Interkarma / daggerfall-unity

Open source recreation of Daggerfall in the Unity engine
http://www.dfworkshop.net
MIT License
2.7k stars 327 forks source link

Deleting and creating new save can preserve old mod save data #2594

Closed KABoissonneault closed 2 months ago

KABoissonneault commented 6 months ago

Describe the issue

When deleting a save, some mod save data may be preserved and re-used on a new save. This occurs for example when the mod with the save data is currently disabled: it won't show in the list of mods in ModManager.Instance.GetAllModsWithSaveData(), and SaveLoadManager.DeleteSaveFolder won't consider the disabled mods for deleting mod save data.

When you look at the save folder after deletion, all disabled (or removed) mods should be remaining

image

When you create a new save later on, that folder might be reused by DFU, and the old mod save data will still be there on your new save.

image

(Notice the 'Date modified' column)

This will become an issue if the user re-enables the mod, as the mod will have some leftover data from an older save that is not compatible with the current one.

To Reproduce

Steps to reproduce the behavior:

  1. Locate a save containing mod save data (mod_­[mod name].txt)
  2. Disable one or multiple mods with such save data
  3. Delete save from the in-game DFU UI
  4. Re-create new saves until that folder is reused

Should be easy to test if your SAVE0 has old mod data.

Expected behavior

If SaveLoadManager.DeleteSaveFolder fails to delete a folder because it contains leftover files, the folder should be renamed to something else. It's worth keeping files DFU doesn't know, these could be important player notes. But we don't have to keep them as primary SAVE folders, we can just toss them under a backup subfolder (ex: DELETED/SAVE0). This will prevent the folder from being reused, and old save files will not accidentally get slipped into a new playthrough.

John-Leitch commented 6 months ago

I can take this one. Should be pretty straight forward to handle, edge cases included e.g. dupes in DELETED.

KABoissonneault commented 6 months ago

Thanks for tackling these, it does spare me some work. However, I am stopping tests on PRs until I got my changes for v1.1 done to focus, and I won't be approving untested PRs. Others can test them if they want, but I expect you might have a few lingering PRs for some weeks. Put in the work at your own risk :)

John-Leitch commented 6 months ago

No worries. If there are any issues, I'll gladly fix them, and if they can't be accepted, so be it. I was a huge daggerfall fan growing up, so it's worth my time either way.

petchema commented 6 months ago

Adding a link to a thread in the forums about that issue, including Interkarma answer https://forums.dfworkshop.net/viewtopic.php?t=5381_

KABoissonneault commented 6 months ago

Seems like Interkarma's suggestion matches #2596 . Good to get a confirmation. Most other suggestions in the thread fail at covering disabled mods, which is the primary issue here. Having mod save data be automatically handled for enabled mods but require an opt-in explicit operation for disabled mods would be weird

KABoissonneault commented 2 months ago

Fixed in 8607d25aaf12a36fdbb71137b2fed73e8dc38be0