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

Serializing class instances can mutate their state in damaging ways #320

Open
Ruin0x11 opened this issue Apr 29, 2021 · 0 comments
Open
Labels
bug Something isn't working design Concerns the architecture of the engine standardization Concerns conventions that should be strongly followed in mods

Comments

@Ruin0x11
Copy link
Owner

Ruin0x11 commented Apr 29, 2021

To avoid allocation, the :serialize() function on classes will return self as the table to be serialized by binser. However, there are some classes that have properties that should not be serialized, like pointers to data prototypes. When :serialize() is called on those objects, the current design is such that those properties should be set to nil by that method. :deserialize() is supposed to restore those properties to their intended values when a save is loaded. However, currently :deserialize() is not called on the objects after serializing to restore the removed properties if the game is not loaded after saving. This is very bad.

If the problem this was trying to solve was avoiding unnecessary allocations, then I have to wonder if the performance benefit is actually worth it. However, :deserialize() will act as if the table is the deserialized class instance, as in the table is not in a special serialized format that gets copied to a fresh instance of that class afterwards - the data deserialized by binser is the class instance itself.

Because of that, maybe it would be best to just call :deserialize() directly after calling :serialize(), and declare that the programmer contract for ISerializable is such that calling :serialize() and :deserialize() in order must be an idempotent operation.

@Ruin0x11 Ruin0x11 added bug Something isn't working design Concerns the architecture of the engine standardization Concerns conventions that should be strongly followed in mods labels Apr 29, 2021
@Ruin0x11 Ruin0x11 changed the title Serializing objects often mutates their state in damaging ways Serializing class instances can mutate their state in damaging ways Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design Concerns the architecture of the engine standardization Concerns conventions that should be strongly followed in mods
Projects
None yet
Development

No branches or pull requests

1 participant