0
\$\begingroup\$

I want it to change the doors lock state but also the colors of the doors Red = locked Green = unlocked either in editor mode or in runtime. But if the method that change the doors colors is not called in the Update it will not work at all. And calling it in the Update seems wrong.

The editor script :

using System.Collections;
using System.Collections.Generic;
using UnityEditor;
using UnityEngine;

[CustomEditor(typeof(DoorsManager))]
public class DoorsLockManagerEditor : Editor
{
    private SerializedProperty _doors;
    private SerializedProperty _globalLockState;

    private bool shouldOverwrite;

    private void OnEnable()
    {
        _doors = serializedObject.FindProperty("Doors");
        _globalLockState = serializedObject.FindProperty("_globalLockState");
    }

    public override void OnInspectorGUI()
    {
        base.OnInspectorGUI();

        serializedObject.Update();

        shouldOverwrite = false;

        // Begin a change check here
        EditorGUI.BeginChangeCheck();
        EditorGUILayout.PropertyField(_globalLockState);
        if (EditorGUI.EndChangeCheck())
        {
            // overwrite only once if changed
            shouldOverwrite = true;
        }

        for (int i = 0; i < _doors.arraySize; i++)
        {
            var door = _doors.GetArrayElementAtIndex(i);

            // if door == null the script itself has an error since it can't even find the SerializedProperty
            if (door == null)
            {
                EditorGUILayout.HelpBox("There was an error in the editor script!\nPlease check the log", MessageType.Error);
                Debug.LogError("Couldn't get door property", target);
                return;
            }

            if (door.objectReferenceValue == null) continue;

            var serializedDoor = new SerializedObject(door.objectReferenceValue);

            var lockState = serializedDoor.FindProperty("doorLockState");

            serializedDoor.Update();

            if (lockState == null)
            {
                EditorGUILayout.HelpBox("There was an error in the editor script!\nPlease check the log", MessageType.Error);
                Debug.LogError("Couldn't get lockState property", target);
                return;
            }

            // HERE OVERWRITE
            if (shouldOverwrite)
            {
                lockState.boolValue = _globalLockState.boolValue;
            }
            else
            {
                EditorGUILayout.PropertyField(lockState, new GUIContent("Door " + i + " Lockstate"));
            }

            serializedDoor.ApplyModifiedProperties();
        }

        serializedObject.ApplyModifiedProperties();
    }
}

The Doors Manager script :

using System.Collections;
using System.Collections.Generic;
using System.Linq;
using UnityEngine;

[ExecuteAlways]
public class DoorsManager : MonoBehaviour
{
    [HideInInspector]
    public List<HoriDoorManager> Doors = new List<HoriDoorManager>();

    // The global state
    [SerializeField] private bool _globalLockState;

    // During runtime use a property instead
    public bool GlobalLockState
    {
        get { return _globalLockState; }
        set
        {
            _globalLockState = value;

            // apply it to all doors
            foreach (var door in Doors)
            {
                // now you would need it public again
                // or use the public property you had there
                door.doorLockState = _globalLockState;
            }
        }
    }

    private void Awake()
    {
        var doors = GameObject.FindGameObjectsWithTag("Door");
        Doors = new HoriDoorManager[doors.Length].ToList();

        for (int i = 0; i < doors.Length; i++)
        {
            Doors[i] = doors[i].GetComponent<HoriDoorManager>();
        }
    }
}

And the HoriDoorManager script :

using UnityEngine;
using System.Collections;
using System.Collections.Generic;

[ExecuteAlways]
public class HoriDoorManager : MonoBehaviour
{
    public List<DoorHori> doors = new List<DoorHori>();
    public bool doorLockState;

    private void Awake()
    {
        if (transform.parent != null)
        {
            Transform parent = transform.parent;
            var children = parent.GetComponentsInChildren<Transform>();

            if (children != null)
            {
                foreach (Transform door in children)
                {
                    if (door.name == "Door_Left" || door.name == "Door_Right")
                        doors.Add(door.GetComponent<DoorHori>());
                }
            }
            ColorDoors(Color.red, Color.green, doorLockState);
        }
    }

    void OnTriggerEnter()
    {
        if (doorLockState == false)
        {
            if (doors != null)
            {
                for (int i = 0; i < doors.Count; i++)
                {
                    doors[i].OpenDoor();
                }
            }
        }
    }

    private void Update()
    {
        //ColorDoors(Color.red, Color.green, doorLockState);
    }

    private void ColorDoors(Color red, Color green, bool state)
    {
        List<Transform> children = new List<Transform>();

        for (int i = 0; i < doors.Count; i++)
        {
            foreach (Transform child in doors[i].GetComponentsInChildren<Transform>())
            {
                if (child == doors[i].transform)
                    continue;

                var renderer = child.GetComponent<Renderer>();
                renderer.sharedMaterial.shader = Shader.Find("Unlit/ShieldFX");

                if (state == true)
                {
                    renderer.sharedMaterial.SetColor("_MainColor", red);
                    LockState(true);
                }
                else
                {
                    renderer.sharedMaterial.SetColor("_MainColor", green);
                    LockState(false);
                }
            }
        }
    }

    public bool GetLockState
    {
        get { return doorLockState; }
        set { doorLockState = value; }
    }

    private void LockState(bool state)
    {
        var collider = gameObject.GetComponent<BoxCollider>();

        if (state == false)
        {
            collider.size = new Vector3(2.3f, 2.736307f, 2.5f);
            collider.center = new Vector3(0, 1.378154f, 0);
            collider.transform.localPosition = new Vector3(-1.57f, 0, -2.98f);
            collider.isTrigger = true;
        }
        else
        {
            collider.size = new Vector3(2.3f, 2.736307f, 3);
            collider.center = new Vector3(0, 1.378154f, 0);
            collider.transform.localPosition = new Vector3(-1.57f, 0, -2.98f);
            collider.isTrigger = false;
        }
    }
}

The DoorsManager script is attached to one empty GameObject. The HoriDoorManager is attached to each door.

The problem is in the HoriDoorManager :

private void Update()
        {
            //ColorDoors(Color.red, Color.green, doorLockState);
        }

If I'm not using this line in the Updaye it will not work either in editor mode and runtime. If I will use this line it will work in both editor and runtime.

The question is if it's fine to call this method in the Update all the time ? Or somehow to call this method only when changing the door/s lock state either in editor mode or runtime mode ? And how to do it ?

\$\endgroup\$
  • 1
    \$\begingroup\$ What about calling the method in update seems wrong? If it is a method which needs to be evaluated continuously to react responsively then being called in the update loop seems to be an appropriate solution. Are you concerned about performance? The method could be refactored to be more performant, or changed to a coroutine that is only called every X seconds. Are you sufferring any current performance bottlenecks? \$\endgroup\$ – disc_code22 Nov 11 '19 at 14:46
  • 1
    \$\begingroup\$ One alternative solution could be a refactoring to control this system to be event driven, more like an observer pattern. Regardless, more detail is needed as to how this system fits into your larger architecture to give a more specific solution. \$\endgroup\$ – disc_code22 Nov 11 '19 at 14:50
0
\$\begingroup\$

Your intuition is correct. Looping through all the doors every single frame is a lot of unnecessary work.

You need to have an event driven logic here. A door's lock is not something that changes constantly or by itself (unlike an HP value for example), so there's no need to check lock states constantly. You just need to run the ColorDoors function when a door is locked or unlocked.

Even better, you can modify your script to run the door its on and give it to all the doors instead of having it in a central door manager script. Then, you won't even have to loop through the doors, the doors themselves will take care of their locks.

You can use the event system to send messages to doors if necessary. Just call the function of the door from somewhere else. You don't ever need a list of doors unless you need persistent storage for their states, which is what a save system would be in place for.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.