More property/member consistency
aleokdev opened this issue · 4 comments
Right now, there are some members that shouldn't be public, as they should be protected against mutation; e.g. Map::width/height/tile_width. These should be changed to properties.
Just wondering, but why should these be protected against mutation? (I think the change makes sense for consistency, regardless)
Well, although right now these properties aren't used internally (as far as i know), we should protect them in case that we rely on them later on for anything.
And please, do not worry about writing to TMX files right now. When we do implement writing (if we do) then we can add set_ functions as well.
My thinking right now for when to use properties vs when to use members is how they are being used internally. If the values are only useful from the user side and have no relevance within the crate (e.g. tint color, opacity, etc) these could be exposed as public members. In the case that they do have relevance within the crate and may cause invalid behavior if modified (e.g. width/height on a tile layer) OR we have some members that could be exposed but can't because they're contained within a private structure (e.g. internal data types) then we should use properties as well.
(edit) Maybe tile_x can stay as exposed members, as I doubt we'll rely on them internally.
One thing though; I still don't understand why layers must have a width/height attribute if the map already has them. Is it because of backwards compatibility?
One thing though; I still don't understand why layers must have a width/height attribute if the map already has them. Is it because of backwards compatibility?
Yes, layer size does not have to match the size of the map due to backwards compatibility. This goes way back to Tiled Java though, but I've kept this because I was thinking to possibly support this as a feature again in future versions of Tiled.
Eventually I'd also like to support a different tile size and orientation per layer, but I'm not sure how to support this yet. It might be quite a refactoring. Or, it could be done by introducing another top-level container for multiple maps.