Saving and loading a custom object with `store_var` and `get_var` has inconsistent behaviour between the editor and builds

[marc-jones]

Godot version

3.3.2-stable

System information

Linux - Fedora 32 5.11.22-100.fc32.x86_64

Issue description

store_var and get_var functions of the File object with the allow_objects / full_objects bools set to true behaves differently between running in the editor and running in a build.

In the editor everything works as it should, but in builds (with debug) an error is displayed when the object is read:

ERROR: instance_create: Script inherits from native type 'Reference', so it can't be instanced in object of type 'Node'.

I've tested it with Linux, HTML5, and Android builds and it's consistent across all of them.

This reddit commenter found that the script export mode affected the behaviour.

Steps to reproduce

The minimal project creates a save file the first time it is run, then reads from that save file each subsequent time it is run. When running the project in the editor the correct behaviour occurs. The first time you run the project, "null" is displayed on the screen because the save file doesn't exist. All subsequent times the project is run, "foobar" is read from the save file created the first time the project was run and displayed on screen.

I then delete the save file with the bash command:

rm ~/.local/share/godot/app_userdata/godot-save-load/test.save

When running the build of the project (which has debug), the first time it's run "null" is also displayed. Subsequent times, however, no text is displayed (because the save object is null and therefore doesn't have the text property) and the following message is displayed in the terminal:

ERROR: instance_create: Script inherits from native type 'Reference', so it can't be instanced in object of type 'Node'.

Interestingly, I can induce the error in the editor. If I delete the save file and run the build to create the save, then run the project in the editor, the same error as above is displayed. Equally, if I delete the save file and run the project in the editor to create the save file, then run the build, "foobar" is displayed.

Minimal reproduction project

https://github.com/marc-jones/godot-save-load

[Xrayez]

This reddit commenter found that the script export mode affected the behaviour.

This is likely due to the fact that GDScript exports compiled/encrypted files using gdc or gde extensions. That's why exporting as text works because the extension is gd for text files.

Pack files contain gdc scripts, not gd. So if you somehow make it so that you load gdc in exported games, this could also work. But something tells me that there should be a better way to solve your problem, because Object serialization entails security issues as well.

See also my previous fix #33266, and get inspired to solve your case perhaps.

[marc-jones]

Ah, okay, so you recommend that I should convert the GameSave.gd object to a dictionary with inst2dict and save the dictionary to disk, rather than save the GameSave.gd object directly?

[Xrayez]

Yeah, if you only care about the state (for saves), you should use inst2dict or other serialization methods such as var2str/to_json (when inst2dict is not enough). Unless you want to expose GDScript script editing to users for modding support.

[marc-jones]

Thanks, I've updated my reproduction project with the fix and it works well. Many thanks for your help!

[Calinou]

We should probably update the documentation to mention this. Any ideas which class we should add this information to? In the File class or Object class?

[Xrayez]

Documenting this in the File class would make sense to me because that's where the problem originated. Script is also an Object, but it's not immediately obvious.

Serializing Object is probably fine depending on what you want to achieve... I'd say the issue is probably specific to GDScript. I think it should also be enough to mention that GDScript files are exported using gdc extension by default (or gde if encrypted), which can lead to parse/load errors in exported projects (since script files load/preload other scripts ending with gd extension, and not gdc/gde), so serializing GDScript itself is not recommended, but an object instance created by GDScript (inst2dict).

[marc-jones]

I think my problem was a misunderstanding of the limitations / consequences of the full_object bool in get_var and store_var on the File object. The documentation mentions that there are security issues with using it, but didn't mention anything about compiled / encrypted scripts not being read correctly when full_objects is true. Given my current understanding of how it works, I can't really see a good use case for setting full_objects to true, so perhaps adding one would be useful?

Another thing I think should be added to the documentation on these functions is that only properties which are exported are saved when full objects are saved, as discussed in this reddit thread.

[Xrayez]

Yeah, I'd say serialization in Godot should be taken with care, and if you want your code to be robust with Godot, prefer locality over using features like full_objects, deep, recursive etc. They are probably not that well tested in the first place, so serialization hasn't matured to the point of being robust. And probably because Godot's development pace is still quite fast for those features to be properly stabilized.

P.S. I still stumble upon those limitations myself, but I've learnt to workaround them by now... I've really tried to improve some of the serialization features in the past, but due to lack of review, those enhancements are in Limbo now, see for instance #33137, #33241. So, you can judge for yourself the priorities of Godot development. I guess the vast majority who use Godot don't actually employ serialization features, so that's why core developers don't really pay much attention to those in the first place. 😛

[kleonc]

Another thing I think should be added to the documentation on these functions is that only properties which are exported are saved when full objects are saved, as discussed in this reddit thread.

It's kinda addressed by #49470.

[marc-jones]

Interesting, thanks for the advice!

[snailrhymer]

It's kinda addressed by 49470

When I came across it, I read the note added as saying that setting PROPERTY_USAGE_STORAGE would be sufficient to ensure serialisation. Seeing that it's marked as default in the GlobalScope docs, it took me some trial and error to realise I needed to use export to ensure serialisation.

I think there's still room for more clarity, possibly by explicitly mentioning export in the note?