Should the state object be defined inside the actual object? For Save/Load purpose

Question

I have a class Furniture with a subclass of FurnitureState where all the "live" data will be stored. During gameplay when the furniture will be turned off or relocated (Player action) it will change the fields of this state class.

The idea is that i can easily serialize all the states during the save operation by just iterating over all furnitures, items, enemies.

public class Furniture
{
    [System.Serializable]
    public class FurnitureState
    {
        public string furnitureType;
        public int x;
        public int y;
        public int enabled;
    }

    public FurnitureState state;
}

Is this a good approach or should i keep the state classes in separate files with all other states for player, items, enemies?

Is it good practice to have the state class to be serializable or should i use another class which will extract the properties that sould be saved/loaded?

Answer 1

This is mostly a matter of coding style, and different teams will have different style guidelines. It looks fine to me, according to my personal style preferences.

There's nothing wrong with using a serialized type to gather the save/load properties of an object. In fact, I'd consider that a distinct positive, since it means less boilerplate code to write/modify when adding new properties, reducing the surface area where you can accidentally introduce bugs.

One thing I would change is to make your nested type a struct instead of a class. This way you don't pay for an extra garbage collector memory allocation when deserializing it, and no extra pointer indirection when accessing it.

Mutable structs can be dangerous in some circumstances (leading to excess copying, or accidental modification of a copy instead of the original), but when all this type is doing is acting as a serializable container for member variables to save/load, those risks are unlikely to bite you for your use cases.

I'd move it out to its own file if it gets cumbersome to maintain the Furniture file with FurnitureState mixed in. But as long as it's just using a dozen or so lines, I don't see it as causing undue clutter.