RandyGaul / cute_headers

Collection of cross-platform one-file C/C++ libraries with no dependencies, primarily used for games
4.24k stars 264 forks source link

cute_tiled: About the behaviour when dealing with unimplemented fields #324

Closed g-lujan closed 1 year ago

g-lujan commented 1 year ago

I was taking a look on the behaviour when a field is not implemented yet and found something that I'm not sure if it's a issue or the intended behavior. As an example, the field "text" is not implemented on objects and cute_tiled deals with it this way:

// function cute_tiled_read_object
case 7758770083360183834U: // text
   CUTE_TILED_WARNING("Text field of Tiled objects is not yet supported.");
   while (cute_tiled_peak(m) != '}') cute_tiled_next(m);
   cute_tiled_expect(m, '}');
   break;

Why does it go until the next "}"? What if the text field is in the middle of the object, like:

"objects":[
   {
     "class":"",
     "height":31.5,
     "id":1,
     "name":"",
     "rotation":0,
     "visible":true,
     "text":"test",
     "width":96,
     "x":416,
     "y":192
  }],

It will ignore all fields below, causing problems when trying to parse. Shouldn't it be something like:

case 7758770083360183834U: // text
   CUTE_TILED_WARNING("Text field of Tiled objects is not yet supported.");
   while (cute_tiled_peak(m) != ',' && cute_tiled_peak(m) != '}') cute_tiled_next(m);
   if (cute_tiled_peak(m) == '}')   continue;
   break;

This way, we also consider the case of the field having other fields below it. Or is the expected behaviour to really cause explicit errors when trying to use an unimplemented field?

RandyGaul commented 1 year ago

This looks like a good change! I don’t use very many features of Tiled so little things like this get missed. Please add in a pull request with your modification!

g-lujan commented 1 year ago

@RandyGaul just submitted a PR with this change and a similar one for the new "class" field. Maybe the same can be applied to other fields, but I still need to take a closer look.

RandyGaul commented 1 year ago

My recommendation would be to just add fixes for things that we come across as problems. It keeps changes simple. Like you said, maybe there are some other similar issues, especially if the text field can appear somewhere else, but we can address those when they actually pop up as a problem.