Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed save data in Questing Foe to allow for custom foe ids #2535

Merged

Conversation

KABoissonneault
Copy link
Collaborator

After looking into this issue: https://forums.dfworkshop.net/viewtopic.php?t=6357

Normally, we can store any int value in a MobileTypes enum, which occurs when using custom enemies with ids not defined in the enum. This works as long as we don't rely on features that require a field to be defined, such as serialization to and from string.

I found that in fsSerializer, enum values are serialized as strings, based on the matching enum field name.

In the Foe.SaveData_v1 struct, we have foeType as a MobileTypes enum. This shows in the save like this

                    "resourceSpecific": {
                        "spawnCount": 3,
                        "foeType": "Knight",
                        ...
                    },

If a field is not defined for this value (ex: custom enemy ids), this gets saved as "foeType": null, which will default to 0 when loaded back in (hence the rat mentioned in the linked thread).

To work around this issue, I've used an int with the id value. I have considered using a string to retain legibility, but I was not sure if it would still work once people start localizing custom enemies (we only have the Career name to identify custom enemies, and that string is user facing).

With the new save data

                    "resourceSpecific": {
                        "spawnCount": 1,
                        "foeId": 277,
                        "humanoidGender": "Male",
                        "injuredTrigger": true,
                        "restrained": false,
                        "killCount": 1,
                        "displayName": "Morkuchas",
                        "typeName": "Ghoul",
                        "spellQueue": null,
                        "itemQueue": null,
                        "$version": "v2",
                        "$type": "DaggerfallWorkshop.Game.Questing.Foe+SaveData_v2"
                    },

I have found one other place where a MobileTypes is serialized, in ItemData_v1, with the field trappedSoulType. I have not provided a fix for this one, because:

  1. many structs and interface use ItemData_v1 explicitly. Moving to ItemData_v2 would require updating all save structures using items to v2 (big change) and the interfaces (breaks mods)
  2. custom monsters cannot be soul trapped as of now. we can keep this fix for later if we ever implement it

…evious enum-based approach would fail on enum values outside of the predetermined enum fields, due to string-based serialization
SaveData_v1 data = (SaveData_v1)dataIn;
SaveData_v2 data;

if (dataIn is SaveData_v1)
Copy link
Collaborator Author

@KABoissonneault KABoissonneault Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had expected the [fsObject("v2", typeof(SaveData_v1))] attribute on the struct to cause fsSerializer to migrate the struct automatically. This is not the case. I suspect the migration only happens if Foe.SaveData_v2 was the top-level struct being deserialized. In the normal case of loading a save with a pending quest, the save data is nested as a generic object in ResourceSaveData_v1.resourceSpecific.

So, I do the migration manually here

@KABoissonneault KABoissonneault merged commit 0e56431 into Interkarma:master Feb 11, 2024
@KABoissonneault KABoissonneault deleted the fix/quest-create-foe branch February 11, 2024 16:25
@KABoissonneault KABoissonneault mentioned this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants