Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested class propertytype not saved in custom class properties #3230

Closed
yuxuanchiadm opened this issue Jan 9, 2022 · 4 comments
Closed

Nested class propertytype not saved in custom class properties #3230

yuxuanchiadm opened this issue Jan 9, 2022 · 4 comments
Assignees

Comments

@yuxuanchiadm
Copy link

Hello everyone!

I've found that when nesting a class inside another class. The propertytype of the inner class is not saved to tmx file. So the type of inner class is implicit. Which may cause confusion while loading map in to game engine.

Here is my configuration:

"propertyTypes": [
    {
        "id": 10,
        "members": [
            {
                "name": "field1",
                "type": "bool",
                "value": false
            },
            {
                "name": "field2",
                "type": "float",
                "value": 0
            }
        ],
        "name": "Class0",
        "type": "class"
    },
    {
        "id": 11,
        "members": [
            {
                "name": "field3",
                "type": "bool",
                "value": false
            },
            {
                "name": "field4",
                "propertyType": "Class0",
                "type": "class",
                "value": {
                }
            }
        ],
        "name": "Class1",
        "type": "class"
    }
]

And object after saving it to tmx file

<object id="45" x="188" y="468">
 <properties>
  <property name="myClass" type="class" propertytype="Class1">
   <properties>
    <property name="field3" type="bool" value="true"/>
    <property name="field4" type="class">
     <properties>
      <property name="field1" type="bool" value="true"/>
      <property name="field2" type="float" value="100"/>
     </properties>
    </property>
   </properties>
  </property>
 </properties>
 <point/>
</object>

The field4 is missing propertytype property,

Thanks!

@yuxuanchiadm
Copy link
Author

And also the custom enum type is missing type property in tmx file. So it defaults to string type. Idk if that's intended?

@bjorn
Copy link
Member

bjorn commented Jan 10, 2022

The field4 is missing propertytype property,

Yeah, this information is currently lost as part of the conversion of the data to a format suitable for exporting. I didn't consider it a problem because the information is redundant (or implicit, as you say), given that it is already available as part of the class definition, which was referenced by the parent property.

I'm not personally sure whether this would actually cause confusion. If it turns out this information is actually necessary or when it is simply impractical for it to be missing, I can consider some solution to include it.

The same reasoning affects the JSON format, where a class value becomes a plain JSON object, where there is simply no way to store any further meta-information about nested class members.

And also the custom enum type is missing type property in tmx file. So it defaults to string type. Idk if that's intended?

Yes, the type attribute is always left out for string properties, since properties are strings by default. You can set an enum to be stored as a number, in which case there will be type="int". The actual enum type is stored in propertytype, as long as the property is not nested in a class.

@bjorn bjorn added the documentation Issue related to the Tiled Manual. label Jan 10, 2022
@bjorn bjorn added this to Tiled 1.8 Jan 10, 2022
@bjorn bjorn moved this to Todo in Tiled 1.8 Jan 10, 2022
@yuxuanchiadm
Copy link
Author

yuxuanchiadm commented Jan 10, 2022

I'm not personally sure whether this would actually cause confusion. If it turns out this information is actually necessary or when it is simply impractical for it to be missing, I can consider some solution to include it.

Consider this case:
In the developing pipeline we need to transform some map meta to the actual data we want to load in to our game engine.
Like transform the resource path to engine's internal asset identifier etc. So we want to iterating over all class properties we are interested in. With propertytype present. We can easily identify which property to transform. Otherwise we need to somehow figure out the structure from the top level property type. It just inconvenient.

The actual enum type is stored in propertytype, as long as the property is not nested in a class.

It will be helpful to also include it in nested class imo.

@bjorn bjorn removed the documentation Issue related to the Tiled Manual. label Jan 19, 2022
@bjorn bjorn self-assigned this Jan 19, 2022
@bjorn bjorn moved this from Todo to In Progress in Tiled 1.8 Jan 19, 2022
@bjorn bjorn closed this as completed in 911d2ef Jan 19, 2022
Repository owner moved this from In Progress to Done in Tiled 1.8 Jan 19, 2022
@bjorn
Copy link
Member

bjorn commented Jan 19, 2022

@yuxuanchiadm Thanks for the additional explanation! It makes sense and I think there is no harm done in including the propertytype attribute in the TMX format, so rather than documenting why it isn't there, I've made a change so that this information is included.

For the JSON and Lua formats I've decided to keep things as-is for now, since storing the additional type information on class members there means we could no longer use their native object representation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

No branches or pull requests

2 participants