Const correctness of Tileson

[mihailescum]

Hi, I'm using Tileson in a small game engine to parse my maps and game objects, and it works like a charm.

When I load for example a Map, I load it using tson::Tileson::parse and then parse the resulting object to my own class, to load the values in a format I can work with. The process looks like this:

MyMap load_resource() 
{
        tson::Tileson t(std::make_unique<tson::Json11>());
        std::unique_ptr<tson::Map> tson_map = t.parse(_resource_path);

        MyMap myMap;
        myMap.parse(*tson_map);
        return myMap;
}

Since I use Tileson purely for loading, I expect the MyMap::parse function to not modify the tson_map, i.e. I would like to pass in a const reference:

void MyMap::parse(const tson::Map &tson_map);

The problem I run into, is that several member function of tson::Map (and other classes, such as tson::Tile) are not declared as const, when I believe they should be, because in my opinion they should not modify tson_map. (Or a const version should have been provided, as for the case of tson::Map::getLayers) If they indeed should *not* be const`, then please let me know that my assumption is incorrect.

For example, trying to compile

void MyMap::parse(const tson::Map &tson_map)
{
        const auto &tson_layers = tson_map.getLayers();
        // Do more stuff
}

raises an error:

the object has type qualifiers that are not compatible with the member function "tson::Map::getLayers" C/C++(1086)
tilemap.cpp(24, 41): object type is: const tson::Map.

Now, assuming that the functions do indeed not alter the original tson::Map object, I can work around this by const-casting, like so:

void MyMap::parse(const tson::Map &tson_map)
{
        const auto &tson_layers = const_cast<tson::Map &>(tson_map).getLayers();
        // Do more stuff
}

But this feels hacky and can lead to undesired results. I have to check every non-const function if it actually modifies the object, and everything could potentially break with a different version of Tiled. Hence actual const-correctness would help me a lot.

[SSBMTonberry]

Thank you for addressing this, @mihailescum :slightly_smiling_face:

You have a valid point, and I am aware that there are certain parts of Tileson that does not have const qualifiers where they seemlingly should. This goes back to many years ago, so I don't exactly remember the reasoning why I went with it, but it may (and I may be wrong) have something to do with hassle related to resolving Tiled properties. I'm certain there are several things that can be done to make Tileson satisfy const correctness without having to apply any hacky code internally in the library, so I'll put this issue on the roadmap to have a proper review on code relevant to this topic! :+1: