The architecture of the SavingWrapper has this extremely annoying section in Update()
if(Input.GetKeyDown(KeyCode.L))
{
Load();
}
The problem with this is that most of the things we restore tend to assume from a virgin state. With “Load”, you get issues like dead characters still being dead but following the player around like a puppy.
The solution to this is to replace “Load()” with
StartCoroutine(LoadLastScene());
Which reloads the scene and then runs RestoreState, preventing all manner of duplication bugs and zombies.
Odd… there shouldn’t be an invalid cast there, but let’s make it safe:
public void RestoreState(object state)
{
if(state is Dictionary<string, object> stateDict)
{
foreach(ISaveable saveable in GetComponents<ISaveable>())
{
string typeString = saveable.GetType().ToString();
if(stateDict.ContainsKey(typeString))
{
saveable.RestoreState(stateDict[typeString]);
}
}
}
}
Ah. OK. I had to rewatch a few of the older videos a couple times for this to make sense. I initially didn’t do this because I thought this would re-introduce the bug that Sam fixed here: Fixing A Saving Bug | GameDev.tv But now I see what you are doing is a slight variation and perhaps what Sam intended to do all along.
OK. I think I know what might be going on. So this removed the Invalid cast but it is now only restoring a single dynamic entity. In my code I am attempting to spawn multiple. I traced through the code and your instructions and I can’t quite figure out where or how the unique keys are being generated for each dynamically spawned enemy. Take a look at my debug statements below.
private void Update()
{
if (Input.GetKeyDown(KeyCode.Space))
{
for (int i = 0; i < numberToSpawn; i++)
{
RandomSpawn();
}
}
}
I put some debug statements in the DynamicSaving like so:
public object CaptureState()
{
int i = 1;
Dictionary<string, object> state = new();
foreach (KeyValuePair<DynamicEntity, DynamicCharacterConfiguration> pair in entities)
{
if (pair.Key == null) continue; //Should be impossible
state[pair.Value.CharacterID] = pair.Key.CaptureState();
Debug.Log($"Saving dynamic entity number {i} with key equal to {pair.Value.CharacterID}");
i++;
}
Debug.Log("After save state dict contains: " + state.Count);
return state;
}
My output when saving was
Saving dynamic entity number 1 with key equal to b80501b9-732b-44f9-b49b-9ab53bb600fd
Saving dynamic entity number 2 with key equal to b80501b9-732b-44f9-b49b-9ab53bb600fd
Saving dynamic entity number 3 with key equal to b80501b9-732b-44f9-b49b-9ab53bb600fd
Saving dynamic entity number 4 with key equal to b80501b9-732b-44f9-b49b-9ab53bb600fd
Saving dynamic entity number 5 with key equal to b80501b9-732b-44f9-b49b-9ab53bb600fd
After save state dict contains: 1
I also had these debug statements in the OnValidate of DynamicCharacterConfiguration
private void OnValidate()
{
Debug.Log("On Validate called");
// prevent prefabs from being assigned that do not have a DynamicEntity on them.
if (characterPrefab != null && !characterPrefab.TryGetComponent(out DynamicEntity dynamicEntity))
{
Debug.Log($"Rejecting {characterPrefab.name}");
characterPrefab = null;
}
//Ensures there is always a characterID
if (string.IsNullOrEmpty(characterID))
{
characterID = System.Guid.NewGuid().ToString();
Debug.Log($"Newly generated ID is {characterID}"); //TODO: Remove after testing
lookUp = null;
}
}
Per my debug statements, OnValidate only gets called once when loading the game but doesn’t get called when I am spawning multiple. In fact every attempt to spawn a new enemy results in every one of them having the same exact characterID. I am imagining the fix is to generate a new ID on each spawned Dynamic entity here:
public DynamicEntity Spawn(Transform owner)
{
if (characterPrefab == null) return null;
GameObject go = Instantiate(characterPrefab, owner);
DynamicEntity entity = go.GetComponent<DynamicEntity>();
// entity.AssignUniqueID(); ???
return entity;
}
But for that to work you need a unique ID for each DynamicEntity but we don’t have one – it only exists on the scriptable object. At this point I figured I must have misunderstood something.
Then finally by just changing what the input key triggers, I think it fixed it. I wonder how many people like me ignored the advice to change how the L key works thinking it may either cause a regression or thinking they already solved it.
No, I made a flaw in my code for saving…
Serves me right for testing this with one Dynamic character instead of 7
What we want is a structure that looks like this:
public List<KeyValuePair<string, JObject>> savedCharacters.
What I did instead was a Dictionary<string, JObject> savedCharacters…
Here’s a replacement for DynamicSaving’s ISaveable and JsonSaveables:
public object CaptureState()
{
List<KeyValuePair<string, object>> state = new();
foreach (KeyValuePair<DynamicEntity,DynamicCharacterConfiguration> pair in entities)
{
if(pair.Key==null) continue; //Should be impossible
state.Add(new KeyValuePair<string, object>(pair.Value.CharacterID, pair.Key.CaptureState()));
}
return state;
}
public void RestoreState(object state)
{
if (state is List<KeyValuePair<string, object>> stateDict)
{
foreach (KeyValuePair<string,object> pair in stateDict)
{
DynamicCharacterConfiguration config = DynamicCharacterConfiguration.GetFromID(pair.Key);
if (config == null) continue;
var character = CreateDynamicEntity(config, transform.position, transform.eulerAngles);
if (character)
{
character.RestoreState(pair.Value);
}
}
}
}
public JToken CaptureAsJToken()
{
JArray state = new();
foreach (KeyValuePair<DynamicEntity,DynamicCharacterConfiguration> pair in entities)
{
if(pair.Key==null) continue; //Should be impossible
JObject entry = new();
entry[pair.Value.CharacterID] = pair.Key.CaptureAsJToken();
state.Add(entry);
}
return state;
}
public void RestoreFromJToken(JToken state)
{
if (state is JArray stateArray)
{
foreach (JToken token in stateArray)
{
if(token is JObject entry)
{
foreach (KeyValuePair<string,JToken> pair in entry)
{
DynamicCharacterConfiguration config = DynamicCharacterConfiguration.GetFromID(pair.Key);
if (config == null) continue;
var character = CreateDynamicEntity(config, transform.position, transform.eulerAngles);
if (character)
{
character.RestoreFromJToken(pair.Value);
}
}
}
}
}
}
Now, instead of looking up the state by the DynamicEntity’s ID, we’re creating pairs of IDs and states. So if we have two characters with the same DynamicEntity, they’ll just both be entries in a list. The list is comprised of a KeyValuePair (or JObject) with the key being the id and the value as the state.
Awesome. It passed a good round of testing. I tried spawning a few, saving, portaling, loading, exiting play mode.
This part below seemed surprising. First reaction - we are swapping the key and value around during CaptureState. And we’re using the same key a number of times.
But then I can see why a Dictionary didn’t work for the output of capture state. Wow. It didn’t make sense why you tried to do CaptureState as a KVP until I saw what’s happening in RestoreState. I would have done this very differently but it seems your way is more efficient than what I would have envisioned.
I was envisioning the key in capture/restore state is a tuple of a GUID and CharacterID and the value is the fully serialized object. But your way doesn’t require a GUID per DynamicEntity.
This is actually similar to an approach I use for Equipable Items with procedurally generated stats… For this, you need two things… the ID of the Equipable Item and a Captured state. Then I instantiate the SO of the item and restorestate on it.
“This” here refers to what you did for what’s in RPG.Dynamic? (as opposed to my initial idea)
Thanks so much for mentioning this. I will be using procedural generation in a number of ways because it is much less work as a designer and I think if done right, it is better for players.
So the captured state contains the procedurally generated stats that modify or enhance the base item. The base item being the thing that is specified by the ID of the Equipable Item. Did I get that right?
Unrelated to items / but related to how OnValidate works
I ran into a curious pair of Debug messages. I seem to recall something of the sort happening in the course too I think for folks on a later version of Unity. It seems whenever I generate a new scriptable object that has an “OnValidate” in it, the OnValidate gets called and runs inside a worker but against what seems to be a phantom object. I say phantom object because that characterID is not persisted anywhere. It doesn’t even show me the associated line of the code in the console window.
[Worker0] Newly generated ID is 7ba6f98c-0d7f-4225-b4c7-302a87755000
Newly generated ID is 2c6cff5a-bcda-4e85-8055-b9e1744c17b3
Before the Scriptable Object is named (when it’s got the default name highlighted ready for you to christen it with a sensible name, it technically doesn’t exist. This doesn’t stop the jobs system from running OnValidate() on it and giving us a very nasty headache for our trouble. Once it has a name, all the information from OnValidate is thrown out the window and a new “real” SO is created. This can often lead to one off null reference errors.
I’ve tried six ways from Tuesday to get around this bug, and I can’t. Unity just says that it doesn’t exist so don’t worry about it. It does, after all, work normally once you give it a name.
Does the DynamicSaving keep track of which scene the objects belong to? If this were to be used as a Core Instance to keep track of game wide things, if you are in a scene, and it spawns some entities, then you proceed to go to a different scene, and the DynamicSaving is trying to reload all the entities, what will happen to them?
A scenario would be a BagManager that extends dynamic saving, and is a singleton that will handle dynamic container spawning.
If an enemy from scene 1 drops a container which gets spawned by the bag manager and is assigned to the entity list, then i proceed to leave the scene, what would happen to those entities when i go to scene 2?
The thing is, a singleton manager will always have the same ID in the saveable entity. I’ve extended the normal dynamic saving to a bunch of other things, I’m taking care of how i will handle “managers” that will be persistent using this system
I’m sorry I don’t understand the concern. I implemented this without using singletons. You can have any many spawn points in as many scenes as you want. For each spawn point just pair up a DynamicSaving component and a SaveableEntity. In my implementation I also have a “Dynamic Spawn Zone” Monobehavior that handles some additional logic and config for me such as randomly distributing the enemies in a zone. So I have a 3-tuple of components for every single spawn zone.
In that case, the Singleton would have to also be responsible for blending the existing information from other scenes (which it would have to cache in RestoreState, and then fold the current scene information into the CaptureState when called.
This will add a significant layer of complexity to your saving setup. I strongly recommend against singletons, especially when they are ISaveable/IJsonSaveables.
The saving system is most efficient when you keep the scope of a given class to it’s single responsibility (The Single Responsibility Principle). As you add more things for a single class to “manage”, more and more opportunities for untestable errors creep into your code. In fact, I go out of my way to never name a class “Manager” for a very specific reason. It’s easy to see the term Manager as part of the name of a class and then when you have a new feature to add, you see Manager in your class list, and you (quite understandably) think, oh, DynamicManager, it can handle this new thing, when in fact it’s generally more appropriate to add a new class for this.
True, i tend to stay away as well, but when it comes to managing player specific things, i like to use them, since they would be present everywhere, in any scene.
I resolved my “issue” by having different versions of my class, one that is specifically for the player, and the rest that would be used individually.
Brian I think I found a bug in this part of the code relative to how it interacts with the guard position on AI Controller. I am working on “Final Moment” on Shops and Abilities and caught it because I just passed the part where we mess with the guard position lazy value.
I believe the fix would look something like this where the transform is still passed in to Spawn() to make sure the parent is set up at the same time as the object is instantiated with the intended position and rotation. Right?
EDIT: Code above works on initial spawn but for some reason it fails when the scene is reloaded. All the dynamic entities want to come back to the parent transform.
Ok I figured out why it fails on restore. The RestoreState method (below) gives it the transform of the parent, which is overwritten by the Mover. But the “bad” guard position stays. It seems now the correct fix is to modify the AIController to implement ISaveable
public void RestoreState(object state)
{
if (state is List<KeyValuePair<string, object>> stateDict)
{
foreach (KeyValuePair<string, object> pair in stateDict)
{
DynamicCharacterConfiguration config = DynamicCharacterConfiguration.GetFromID(pair.Key);
if (config == null) continue;
var character = CreateDynamicEntity(config, transform.position, transform.eulerAngles);
if (character)
{
character.RestoreState(pair.Value);
}
}
}
}
The mover should only be modifying the transform.position property, not the transform itself, and the Mover should have saved the position of the character at the time it was saved… so I’m not sure what’s going on with that.
If maiking AIController as an ISaveable/IJsonSaveble is doing the trick, though, I’d stick with it.
The mover is modifying the transform.position correctly, but what is happening is in this line of the code on RestoreState.
var character = CreateDynamicEntity(config, transform.position, transform.eulerAngles);
The position is being set to the DynamicSaving’s associated gameobject.
Then what happens is this code in AIController.Awake executes and sets the guardPosition to be the same as the DynamicSaving’s transform. This was a change late in the SAB course.
guardPosition = new LazyValue<Vector3>(GetGuardPosition);
guardPosition.ForceInit();
So when the game loads, the Dynamic Entities are in their correct starting locations thanks to the Mover script but they end up drifting towards a guard position that is the location of the Dynamic Saving’s gameobject.
Yeah I think this is the fix. It was in this commit that I think broke it. Sam in the video mentions he’s putting in a hack that he would have preferred if it was implementing ISaveable. So in short, I think the RPG.Dynamic is fine, it’s just exhibiting weird behavior as a result of a late change in SAB. Moving the ForceInit to Start() fixes the issue (but breaks respawn).
