Potential Race Conditions / Data Hazards

James_Lafritz · September 24, 2022, 3:57pm

I don’t remember all of the places that I had potential problems in my code as I modified it slightly going through the lectures.
I do know of 2 that I had, I only fixed 1 at the moment the other one will be fixed when the saving wrapper is changed over to Save / Load Screen.

The major one that I had was due to the fact that all of my UI updates from Events, Scriptable Object Game Events to be specific. I did this as part of one of the challenges. My UI only knows about the Core Namespace. It does not know anything about the Health, the Experience, or Base stats. It Listens to events on the Scriptable Object. To ensure that the UI Got properly Updated I used a little trick of making my Start Methods for Health, Experience, and Base Stat be Coroutines. In the start methods I yeild for a frame and then fired the Event to notify all the Observers that the value has changed. Originally I was subscribing to the events in On Enabled and On Disabled.

Base Stat

        private IEnumerator Start()
        {
            _currentLevel = CalculateLevel();
            yield return null;
            if (onExperienceMaxChanged) onExperienceMaxChanged.Invoke(gameObject, _experienceToNextLevel);
            if (onLevelChanged) onLevelChanged.Invoke(gameObject, _currentLevel);
        }

Health.cs

        private IEnumerator Start()
        {
            yield return null;
            
            _max = _baseStats.GetStatValue(Stat.Health);
            if (_value < 0) _value = _max;

            OnHealthChange();
        }

Experience

        private IEnumerator Start()
        {
            yield return null;
            if (onExperiencedChanged) onExperiencedChanged.Invoke(gameObject, Value);
        }

I latter change the UI display to register in the Awake and unregister in the On Destroy because I wanted it to get the updates from the values even if it was not enabled, I saw a potential that the UI may have been Disabled, these values got updated, then the UI gets enabled, the displayed values would not be correct (Wasn’t an Issue as this particular scenario did not happen, but it could). So the above solution is not needed now but I left it there since my Health relied on Base Stats for the Max value that is used after the game is up and running (I wanted to make sure that this value was set properly).

The other Issue is in the way the Loading happens from the Saving Wrapper. I save a game. Go kill a bunch of enemies. Enemies are dead, I get more health, I get experience, I have Leveled up. I press L to load the data in, the visuals are now no longer correct. The enemies that where killed appear to be dead, there is nothing telling the animator to reset or go from dead back to alive. My UI is not displaying correctly, nothing is telling my UI that it needs to be updated. That is because the scene is not actually reloaded when pressing L, since this is just a visualization issue and I was only using L as a Quick Debug feature at the time I am Ok with this until I get to actually properly loading from the UI. There are several ways to fix this, one is in the individual restore code make sure the Value Changed Events get called and to Reset the Animators. The other is in Saving Wrapper to Load Last Scene when pressing L instead of Just calling to the Regular Load Method which I can not change since the Portal Script relies on it.

Saving Wrapper.cs

        private void Update()
        {
            if (Input.GetKeyDown(KeyCode.L))
            {
                Load();
            }

            if (Input.GetKeyDown(KeyCode.S))
            {
                Save();
            }
            
            if (Input.GetKeyDown(KeyCode.Delete))
                Delete();
        }

=>

            if (Input.GetKeyDown(KeyCode.L))
            {
                StartCoroutine(LoadLastScene());
            }

Again since this is currently trow away code for my testing proposes I just did not fix it.

James_Lafritz · September 24, 2022, 4:45pm

In the Hunting for race conditions I did not have the same issue as Sam had, with the nav mesh agent, but that was because I only did the stuff with the nav mesh agent if I had one. It wasn’t my attention to do this to avoid the race condition caused by the restore state, this was because when we made this originally I was planing on modifying it so that I was controlling the player character using the wasd/arrow/left thumb stick for the player instead of click to move, which I hadn’t done yet.

Doing the change here in my Save System Did however fix these other issues I was having. My original saving wrapper code was slightly different.
Mover.cs

        public void RestoreFromJToken(JToken state, int version)
        {
            if (state == null || version < 4) return;
            if (_hasAgent) _navMeshAgent.enabled = false;

            MoverLoadData data = new();
            switch (version) {// Load move data depending on which save version was being used.}

            // do to the way Unity internally accesses the transform component (Uses GetComponent<transform>)
           // caching it before multiple access to its properties is more performant.
            Transform transform1 = transform;
            transform1.position = data.Position;
            transform1.eulerAngles = data.Rotation;

            if (_hasAgent) _navMeshAgent.enabled = true;
        }

Saving System was only Reloading the Scene if it was a different scene. This would have caused me an issue down the road see the above, when implementing the Load Menu. If I applied the Fix above I would have realized that this Race Condition existed. Thanks for saving me from having to hunt this down a great time saver.

        public IEnumerator LoadLastScene(string saveFile)
        {
            int buildIndex = SceneManager.GetActiveScene().buildIndex;
            IDictionary<string, JToken> state = LoadFile(saveFile);
            if (state.ContainsKey("lastSceneBuildIndex"))
            {
                buildIndex = (int)state["lastSceneBuildIndex"];
            }

            if (buildIndex == SceneManager.GetActiveScene().buildIndex)
            {
                RestoreState(state);
                yield break;
            }

            yield return SceneManager.LoadSceneAsync(buildIndex);
            RestoreState(state);
        }

Is now

        public IEnumerator LoadLastScene(string saveFile)
        {
            int buildIndex = SceneManager.GetActiveScene().buildIndex;
            IDictionary<string, JToken> state = LoadFile(saveFile);
            if (state.ContainsKey("lastSceneBuildIndex"))
            {
                buildIndex = (int)state["lastSceneBuildIndex"];
            }

            yield return SceneManager.LoadSceneAsync(buildIndex);
            RestoreState(state);
        }

James_Lafritz · September 24, 2022, 6:56pm

This Change Causes a Race Condition to Happen.

The problem is the way I was registering to to The Experience Changed Game Event in Base Stats.
See the Discussion Properly Registering to Delegates

        private void OnEnable()
        {
            if (onExperienceChanged) onExperienceChanged.RegisterListener(UpdateLevel);
        }

        private void OnDisable()
        {
            if (onExperienceChanged) onExperienceChanged.UnregisterListener(UpdateLevel);
        }

=>

        private IEnumerator Start()
        {
            _currentLevel = CalculateLevel();
            yield return null;
            if (name == "Player") Debug.LogWarning($"{name}-Base Stats Awake()-{_experienceToNextLevel}, {_currentLevel}");
            if (onExperienceMaxChanged) onExperienceMaxChanged.Invoke(gameObject, _experienceToNextLevel);
            if (onLevelChanged) onLevelChanged.Invoke(gameObject, _currentLevel);
            if (onExperienceChanged) onExperienceChanged.RegisterListener(UpdateLevel);
        }

        private void OnDestroy()
        {
            if (onExperienceChanged) onExperienceChanged.UnregisterListener(UpdateLevel);
        }