ASSERT when switching to map with object selected #3532

I found the cause of the assert to be an objectgroup with the same id as another layer. This happened because the maps were created outside of the Tiled program.

Is there some sort of TMX validation that Tiled can do upon loading the map to alert the user of this? Or some sort of lint tool?

These externally generated maps also have duplicated object ids (in different groups). Will these be a problem? If so, it would be good if Tiled could flag duplicated object ids as well.

I found the cause of the assert to be an objectgroup with the same id as another layer. This happened because the maps were created outside of the Tiled program.

Hmm, duplicate layer IDs should still not be a reason for hitting an assert, so I still consider this a bug in Tiled.

Is there some sort of TMX validation that Tiled can do upon loading the map to alert the user of this? Or some sort of lint tool?

I don't believe Tiled currently warns about duplicate IDs, but I do think that'd be a good thing to add.

These externally generated maps also have duplicated object ids (in different groups). Will these be a problem? If so, it would be good if Tiled could flag duplicated object ids as well.

For layer IDs, I had not expected it to matter at all. For object IDs, it would make any reference to either of the objects involved in ID collision ambiguous.

I notice that if an object is duplicated a number of times the ids will increment, but they can conflict with existing ids. Is that also a bug?

I notice that if an object is duplicated a number of times the ids will increment, but they can conflict with existing ids. Is that also a bug?

Hmm, that would indeed be a bug. However, it shouldn't happen, because the IDs that get assigned to an object are based on an incrementing "next object ID" variable stored with each map. You should not be able to end up with objects with higher IDs than the "next object ID" value. If you can, it would be great if you could share the reproduction steps.

As I said, the maps were created outside of Tiled. The next*id attributes were not set to accurate values.

If proper operation of the program requires unique ids, then it would be more robust if the next*id values were automatically set at load time by looking at the existing data. This is a trivial operation so it seems to me those values shouldn't be saved at all (and therefore could never be wrong). As it is, any external tool that manipulates TMX files will have to do this bookkeeping work to keep these values accurate.

I tried removing the next*id attributes to see what happens. The documentation says this about nextobjectid:

(defaults to the highest object id in the file + 1)

I'm seeing that without that attribute the default value is always 1.

This is a trivial operation so it seems to me those values shouldn't be saved at all

The values are saved to avoid situations where invalidated object references start pointing to the wrong object. For example, if you create objects 1,2,3, and make an object reference to 2, and then delete 2 and 3, nextobjectid would still be 4 and the next object you create, even after closing and reopening the map will not automatically get pointed to by that object reference - because it's not that object. If nextobjectid is generated from the existing objects, it would be 2, and when you create a new object, that existing property will immediately point to it, which is quite likely to be incorrect.

Personally though, I'm with you: these values have no business being saved with the map, and a better solution to the object reference problem may be to unset references as part of the object deletion step (and Undoing should bring those references back, while creating a new object with the same ID should not).