Remove redundant fields in `MapData`

[DanRStevens]

The MapData struct uses vectors, which have a natural in build size. As such, we don't need access to the tilesetCount field in MapHeader struct contained within MapData. Additionally, having it there means we need to remember to update it to keep it in sync. This violates the DRY principle, in that we have two different fields, which might not match, rather than a single authoritative field.

We might solve this by storing needed fields from MapHeader directly in MapData, or perhaps contain a reduced or modified struct, while maintaining MapHeader only for serialization purposes.

Related, this may help normalize access to the mapTileWidth field / MapTileWidth() accessor, which is actually stored as lgMapTileWidth. Converting this to a regular field in MapData could allow access (and method/field capitalization) to match mapTileHeight.

[DanRStevens]

Now that I'm thinking about it. This may also relate to something that was brought up in the past. Should MapData be a struct or a class?

I believe the original plan was to keep things simple and easy to serialize, and structs seemed like the obvious choice. However, the use of vectors takes the MapData away from being directly useful for serialization. Having both vectors, and validation of map data, makes this rather class like.

[Brett208]

Agreed that we have blurred the lines between a data transfer object and a class. What if we remove MapHeader as a member of map data? When saving, we can form and validate the MapHeader. When loading we can use the MapHeader to feed the newly created map class. Maybe as a constructor input.

[DanRStevens]

Agreed.

After taking a look over the MapHeader fields, I think only 2 fields are fundamental and need to be preserved for MapData: mapTileWidth, and mapTileHeight.

Additionally, we don't currently have support for saved game parsing, nor representing the parsed data in memory. As such, there is no current way to infer the bSavedGame field. This is however not a fundamentally required field, as adding support for saved games would add extra fields and vectors which could be used to infer we have saved game data.

The version is perhaps more a feature of the on disk format, with possible format conversions. Hence the version field could simply be generated, or only stored in cases where we have a parser that supports multiple versions.

[Brett208]

We use the the saved game field to allow loading the map portion of a saved game into memory. This allows OP2MapImager to image a saved game's map. This could be limited to recording just in the Map Load code if we want to remove the field from the map. might be nice to know the source of the map data though (from a map file or a saved game).

-Brett

[DanRStevens]

We could leave it in for now, since we don't have a replacement. In the future though, if we add data structures to represent saved game data, that could replace the bSavedGamed (isSavedGame?) field.

[Brett208]

Happy with renaming to isSavedGame. If we make IsSavedGame a get function, then it will allow us to manipulate the internal implementation with hopefully not affecting the public API. For now, it can just default to false unless a SavedGame was loaded to create the map data. Since we cannot save a saved game yet, I think this behavior would be fine.

-Brett

[DanRStevens]

I think that's a very good plan. Let's go with that.