Weird "Race Condition" still persists (no pun intended)

[Cat_Hill]

Hey so I loved this lecture (added something to an previously existing feedback section) but wanted to say it again. I’m sorry to hear many folks didn’t like it, but to people like me this was absolutely wonderful.
EDIT: I’m referring to the lecture on race conditions that comes after the saving system lectures. This is tagged in two spots.

So here’s my “race condition”. Using quotes since I’m not sure that’s what it really is. Within the Save and RestoreState of Mover, it seems to function two different ways for the Player.

1. Reloading the last scene - does what it is supposed to here
2. Restoring state after a portal - does weird stuff

public object CaptureState()
        {
            return new SerializableVector3(transform.position);
        }
      
public void RestoreState(object state)
        {
            SerializableVector3 serializableVector3 = (SerializableVector3)state;
            navMeshAgent.Warp(serializableVector3.ToVector());
            GetComponent<ActionScheduler>().CancelCurrentAction();
        }

On point 2. What is happening is that we are saving the position of the player in Scene X and then restoring that same position that was valid in scene X in scene Y. Since the player exists between scenes, it makes no sense in Scene Y. Now in our simple game, this doesn’t cause much of a problem because the code in Portal fixes it.

But for a moment what’s happening is the player is ending up in a completely intended location after portaling and this may cause some weird unintended effects as the game gets more complex. At the very least I get warnings like this “Failed to create agent because it is not close enough to the NavMesh” popping up.

Are others having this issue? What do you all think? I am hesitant to fix it with some hack within RestoreState and have been living with it since I think the hack would be worse.

[bixarrio]

I hate that the tags are not linked to the lecture, so I have to go look through 90 videos to find the correct one. Each video has the tag shown, but there’s no easy way to get from here to the lecture. Enough ranting, let’s talk.

I haven’t done this course in a long time, so my memory is pretty vague but I recall that in Portal we save the current scene, then load the next scene and restore. As you’ve mentioned, this then places the player in the saved position of the previous scene. We then warp the player to where the portal wants it to be and save again

I think the problem is that when the first warp happens, the NavMeshAgent may see that there is no NavMesh where it’s located and raise the exception. But then we move it to a valid location, so it’s really an ‘invalid’ exception - well it’s valid, but it is in the middle of a ‘process’ that we are performing and expected, but will be handled when we move to a proper spot. @Brian_Trotter may have addressed this several times already (I didn’t do a search this time).

I really want to handle this exception by using an exception handler in the restore, but I’m not sure if Unity is actually raising an exception, or just doing a Debug.LogError - and even then I’m not sure if Debug.LogError is raising an exception or just logging a specific log level. I haven’t tried anything here so I can’t say for sure, here.

Another quick option - which is a hack - would be to set a ‘don’t warp’ flag on the player before saving in the old scene, and unsetting it before saving in the new scene. And then handling the warp in RestoreState based on the flag.

Tbh, I would wait and see what our esteemed TA has to say.

[Cat_Hill]

@bixarrio

I hate that the tags are not linked to the lecture,

TIL you can link two tags. I just linked 13_ss_rpg to think since I think that’s where this interplay between portaling and saving first arises.

@bixarrio

I think the problem is that

Yup. You got it.

@bixarrio

I really want to handle this exception by using an exception handler in the restore, but I’m not sure if Unity is actually raising an exception, or just doing a Debug.LogError - and even then I’m not sure if Debug.LogError is raising an exception or just logging a specific log level. I haven’t tried anything here so I can’t say for sure, here.

There is no raising of exceptions but just looked at docs -Unity - Scripting API: AI.NavMeshAgent.Warp It does return false if it fails to warp. I guess I could throw my own exception and decide to catch and ignore it in the portal. It won’t stop the LogWarning though. Fortunately it’s a warning not an error.

I’m actually surprised how little exceptions are used. I don’t know if not using exceptions is a C# thing or a Unity thing or a Game Developer thing. Exception handling would be the cleanest way to handle this I think.

@bixarrio

Tbh, I would wait and see what our esteemed TA has to say.

Looking forward to it!

[Brian_Trotter]

I’ll address this after work (or maybe at lunch).

[Brian_Trotter]

@bixarrio

I hate that the tags are not linked to the lecture, so I have to go look through 90 videos to find the correct one. Each video has the tag shown, but there’s no easy way to get from here to the lecture. Enough ranting, let’s talk.

There is a structure to the lecture. You’ll note that each tag has a number_xx_yyy
From Right to left:
yyy = course. In this case rpg is the rpg series, inv is the inventory, etc. The course codes get murky after a bit, but once you know the course code, you’ve got to the right page to start.
zz = initials representing the topics, in this case xx is cs or “Character Stats”
The number is nominally the lecture number (sometimes adjusted if we insert a lecture, could have an a appendix, etc)
13_cs_rpg becomes the 13th lecture in Character Stats in the RPG course or…Levelling Up

@Cat_Hill

But for a moment what’s happening is the player is ending up in a completely intended location after portaling and this may cause some weird unintended effects as the game gets more complex. At the very least I get warnings like this “Failed to create agent because it is not close enough to the NavMesh” popping up.

Leading cause, methinks of the Failed To Create Agent Warning.
There is a simple fix for this. In my scenes, I put the character in each scene in a location that I know is on the NavMesh.
Once you get the game to the stage where you have a starting menu to create or load a new game (See: Shops and Abilities), where you place the character only really matters in that first scene. After that, the location will always be driven by the Loading system. Just make sure it’s on a Navmesh and things will be golden.
Don’t fret over those Failed to Create Agent messages unless they are spamming you to 999 in a couple seconds in red. Then your character really IS in a place it shouldn’t be.

@Cat_Hill

I am hesitant to fix it with some hack within RestoreState and have been living with it since I think the hack would be worse.

No hack in RestoreState is going to fix the character not starting on the NavMesh when the new scene loads. There are plenty of hacks that could break your game, though.

@bixarrio

I really want to handle this exception by using an exception handler in the restore, but I’m not sure if Unity is actually raising an exception, or just doing a Debug.LogError - and even then I’m not sure if Debug.LogError is raising an exception or just logging a specific log level. I haven’t tried anything here so I can’t say for sure, here.

It’s not truly throwing an exception, it’s just using LogError instead of LogWarning, sort of like DontDestroyOnLoad doesn’t throw an exception, just shows a LogWarning instead of a LogWarning.

In terms of exception handlers… Ugh… Hey Google: Show me a way to address issues in 20 lines of code that I could have handled with an if(thing==null) return; instead. (Sometimes exception handlers are handy, others, they’re just a very verbose way of doing what you can do faster with an if statement and a Debug.Log.

[bixarrio]

@Brian_Trotter

There is a structure to the lecture. You’ll note that each tag has a number_xx_yyy

Yeah, I know the structure and I can find the lectures, but - like you mentioned - sometimes lectures get inserted and I count down to lecture 34 (because they are not numbered, I have to count them) and then I find myself at tag 31 or something. I just wish there was an easier way. Clicking the tag takes me to some other posts for that lecture, which may also help to track down the specific video quicker. I just meant that the video has a link to the posts related to it, and it would’ve been nice if the inverse was true, too. I just mentioned it, don’t want to hijack the post with my own complaints.

@Brian_Trotter

In terms of exception handlers… Ugh… Hey Google: Show me a way to address issues in 20 lines of code that I could have handled with an if(thing==null) return; instead. (Sometimes exception handlers are handy, others, they’re just a very verbose way of doing what you can do faster with an if statement and a Debug.Log.

[Cat_Hill]

@Brian_Trotter

There is a simple fix for this. In my scenes, I put the character in each scene in a location that I know is on the NavMesh.

Don’t you mean you have to put the character in a location that you know is on the next scene’s navmesh But you have to do that in the current scene’s location.

So this discussion just sparked an interesting solution. I can set a special location when portaling to let the save/restore system know the character is “portaling” and to not try to set the character location upon restore. This feels the least hacky to me but I’ll hold off implementing until I get farther in the course.

@Brian_Trotter

In terms of exception handlers… Ugh… Hey Google: Show me a way to address issues in 20 lines of code that I could have handled with an if(thing==null) return; instead. (Sometimes exception handlers are handy, others, they’re just a very verbose way of doing what you can do faster with an if statement and a Debug.Log.

Well I had to laugh but sometimes exceptions really are the best way to handle it.

Objects sometimes run into the issues that they can’t possibly have context for how to fix. And unlike silently returning a boolean, the writer of the library can give a lot of transparency to its users with exceptions. Not to get too philosophical but I feel that if you’re creating a major library or API, I think the responsible thing to do is to make sure your code throws exceptions.

---

Anyway - thanks everyone for the humor and help here!

[Brian_Trotter]

@Cat_Hill

Don’t you mean you have to put the character in a location that you know is on the next scene’s navmesh But you have to do that in the current scene’s location.

Oops, I just saw the logic there…

Alright, 3 years after this annoying bug (with annoying solutions and an annoying TA saying “it don’t hurt nothing, just live with it”), I believe I have the solution, and I tested it multiple times to be sure…

First, we need a new SerializableVector3. To be overkill, I’ll call it SerializableVector3WithScene

        [System.Serializable]
        public struct SerializableVector3WithScene
        {
            public float x;
            public float y;
            public float z;
            public int sceneIndex;

            public SerializableVector3WithScene(Vector3 position)
            {
                x = position.x;
                y = position.y;
                z = position.z;
                sceneIndex = SceneManager.GetActiveScene().buildIndex;
            }

            public Vector3 ToVector3()
            {
                return new Vector3(x, y, z);
            }
        }

When creating a new SerializableVector3WithScene, it captures the scene build index automagically.
Now in Mover, instead of saving a SerializableVector3, we’ll use this new struct:

        public object CaptureState()
        {
            return new SerializableVector3WithScene(transform.position);
        }

Now here’s where the magic happens. We’re going to make some significant changes to RestoreState, to enquire about the current scene, but we also need to make the script sdrawkcab compatable in case you’re loading an old scene.
We’re going to check to see if we’re using a SerializableVector3WithScene, if we are, then we compare the scene with the current scene. If they’re different, then we return doing nothing.

I’ve changed the way we handle the position, rather than position being a SerializableVector3, I make it a Vector converted from whichever struct it is. Then we simply use the position once it’s properly converted, or, if for some reason state is neither of these things, then we exit gracefully.

        public void RestoreState(object state)
        {
            int sceneIndex = SceneManager.GetActiveScene().buildIndex;
            Vector3 position;
            if (state is SerializableVector3WithScene serializableVector3WithScene)
            {
                if (sceneIndex != serializableVector3WithScene.sceneIndex) return;
                position = serializableVector3WithScene.ToVector3();
            }
            else if (state is SerializableVector3 serializableVector3)
            {
                position = serializableVector3.ToVector();
            }
            else return;
            navMeshAgent.Warp(position);
            GetComponent<ActionScheduler>().CancelCurrentAction();
        }

[Brian_Trotter]

Bonus: JsonSavingSystem Edition
With the JsonSavingSystem, it’s even easier, no special struct required.
We can just add the scene BuildIndex to the JObject when capturing, and then check it when restoring.

        public JToken CaptureAsJToken()
        {
            JObject state = new JObject(); //We no longer need to map a JObject to an IDictionary
            state["x"] = transform.position.x;
            state["y"] = transform.position.y;
            state["z"] = transform.position.z;
            state["sceneIndex"] = SceneManager.GetActiveScene().buildIndex;
            return state;
        }

        public void RestoreFromJToken(JToken state)
        {
            if (state is JObject stateDict)
            {
                if (stateDict.ContainsKey("sceneIndex") && stateDict["sceneIndex"].ToObject<int>() !=
                    SceneManager.GetActiveScene().buildIndex)
                {
                    return;
                }
                navMeshAgent.Warp(stateDict.ToVector3());
                GetComponent<ActionScheduler>().CancelCurrentAction();
            }
        }
    }

Note that even though our state has the extra “sceneIndex” field, the helper function ToVector3() doesn’t care, because it’s only checking for “x”, “y”, and “z”

[Cat_Hill]

really nice solution! Love it.