mapeditor / tiled

Flexible level editor
https://www.mapeditor.org/
Other
10.97k stars 1.74k forks source link

Cleaning up empty nested classes #3998

Open hnbdr opened 1 month ago

hnbdr commented 1 month ago

Nested class properties remain even if they do not have a single subproperty. They are visible in the interface and then saved in the json map. They appear if you clear all nested properties manually but forget to delete the nested class itself. They also remain after setProperty(path, undefined) is executed.

It would be convenient if they were cleaned automatically.

bjorn commented 1 month ago

Hmm, this is supposed to work. The clearing of "empty" class values is done in setClassPropertyMemberValue:

https://github.com/mapeditor/tiled/blob/ee52b20b685e04714151505cb2c141fb09b5444f/src/libtiled/properties.cpp#L90-L96

SetProperty::redo calls this function via Document::setPropertyMember. So we need to find out why this isn't working as expected in your situation. Can you mention specific and ideally minimal steps to reproduce this issue?

hnbdr commented 1 month ago

Certainly.

  1. Create a new project.
  2. Create a class ParentClass
  3. Create a NestedClass
  4. In the NestedClass class, create a nestedProperty of any type
  5. In the ParentClass class, create a nestedClass property of type NestedClass
  6. Create a TestLayer on the map
  7. Assign the TestLayer a ParentClass
  8. Change the nestedProperty of TestLayer
  9. Clear the nestedProperty property of TestLayer manually by clicking on "Reset" or run item.setProperty(['nestedClass', 'nestedProperty'], undefined)
  10. The nestedClas property is still there. The text color is dark. The script api shows Tiled::PropertyValue(, 2, NestedClass) with an empty value object. The map file saved as json or xml also has a nestedClass property with an empty value.
hnbdr commented 1 month ago

It looks like the scheme works but only if the nesting depth is greater than one. So the property itself with depth 1 is not cleared, regardless of whether the class is assigned to the object on the map or not. If it is not assigned it makes sense, but if it is assigned then it is redundant to have an empty one.