Tomash667 / carpg

Combination of action rpg with roguelike.
https://carpg.pl
MIT License
16 stars 15 forks source link

Savefile loading does not check for scripted game content #439

Open BottledByte opened 4 years ago

BottledByte commented 4 years ago

While the game has functional implementation of changing the properties of Abilties, Items and Perks and other things via scripts and reflect the changes upon game reload, it does no checking if resource IDs stored in save file really exist within current scripts / loaded resources, at least in case of Abilities, Items and Perks (I do not know if other resources are subject to this).

This is severe problem making modding very difficult. If the scripted resource is missing while loading a save file... game crashes (in best case by assert, but it could be worse). Only protection against this is a fact that content is released only with official versions and that these things are handled by version save file conversion code and requirements.txt.

Crashing the game during load (OR in game when the resource gets accessed for the first time!) is unacceptable behaviour, considering facts that lack of resources is during load is recoverable state (simply refuse to load the file or print warning/error), can be checked during loading easily (check that ID of each resource is known/accesible), and that player should be notified about such trivial error.

There is some checking present, but it happens only during resource loading, but if the resource ID in save file is not referenced in requirements.txt, no warning/error gets printed at all.

I am aware that scripting/modding support is still very young, but this thing should be addressed in nearby future.

BottledByte commented 4 years ago

@Tomash667 After I finish my work on #437 (thanks to which I noticed this) and consulting this with you, I would like to address this issue.

Tomash667 commented 4 years ago

@BottledByte how are you planning to do this? Simple solution will be to use TryGet/Get - get will throw exception that will be cached in LoadGame.