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

Add support for nested (dictionary-type) properties #489

Closed
LearnCocos2D opened this issue Aug 4, 2013 · 14 comments · Fixed by #3215
Closed

Add support for nested (dictionary-type) properties #489

LearnCocos2D opened this issue Aug 4, 2013 · 14 comments · Fixed by #3215
Assignees
Labels
feature It's a feature, not a bug.

Comments

@LearnCocos2D
Copy link

Currently it is not possible to create your own nested properties (like Position, Size). If a single property needs to encode multiple parameters, it needs to be done by embedding all parameters in the same string:

FollowPath | path=p1;speed=10;repeat=0

Or alternatively each property needs to have its own name:

FollowPath | p1
FollowPathSpeed | 10
FollowPathRepeat | 0

Both solutions are supoptimal for editing, and terrible for reading properties in a generic way. For example the above would require reading each property individually, while the example below could be mapped to a "createFollowPath" method that simply gets the properties dictionary passed in. With technologies like KVC (Key Value Coding, ie setting properties in the object hierarchy by key path like "FollowPath.speed") this would lend itself towards more automatisms.

So what I'd really like is to be able to add "dictionary" type of objects. Ideally with unlimited nesting, which once the nesting functionality is there should come natural anyway. The above would then look like this:

V FollowPath
path | p1
speed | 10
repeat | 0

To make this possible the user should be able to create a "dictionary" property.

This could be done by offering another [+] icon next to it, perhaps [++] that creates a property of type dictionary. Then with the dictionary property selected, or one of its children, any click on [+] or [++] adds the new property to the selected dictionary property instead of the root properties.

@bjorn
Copy link
Member

bjorn commented Aug 7, 2013

Feature makes sense, though I guess it kinda screams for predefining types that automatically have a set of nested properties.

@LearnCocos2D
Copy link
Author

Funny, now that you mention it this is what I happen to have here. ;)

On 07.08.2013, at 20:33, Thorbjørn Lindeijer notifications@github.com wrote:

Feature makes sense, though I guess it kinda screams for predefining types that automatically have a set of nested properties.


Reply to this email directly or view it on GitHub.

@fontmas
Copy link

fontmas commented Sep 27, 2014

@bjorn a sugestion for curstom property window could be a sub tree view.

Example idea:
image

@fontmas
Copy link

fontmas commented Sep 27, 2014

Things to think about and ponder...

Is a nested property could be a nest for others nested properties?

How to could be the format in XML files?

@Chaoseiro
Copy link

I believe that, at least from a XML standpoint, it could be something like this

<properties>
    <property name="a property" value="some value"/>
    <properties name="maybe a name attribute is unnecessary?">
        <property name="a nested property" value="some nested value"/>
        <property name="a nested property 2" value="some nested value 2"/>
    </properties>
    <properties name="nested properties 2">
        <property name="a nested property again" value="some nested value"/>
        <property name="a nested property again 2" value="some nested value 2"/>
        <properties name="we must go deeper">
            <property name="are we there yet" value="false"/>
            <property name="limbo" value="false"/>
            <properties name="insanity">
                <property name="limbo" value="true"/>
            </properties>
        </properties>
    </properties>
   </properties>

or, to keep only property nodes inside properties nodes:

<properties>
    <property name="a property" value="some value"/>
    <property name="nested properties 1" value="irrelevant?">
        <property name="a nested property" value="some nested value"/>
        <property name="a nested property 2" value="some nested value 2"/>
    </property>
    <property name="nested properties 2" value="irrelevant?">
        <property name="a nested property again" value="some nested value"/>
        <property name="a nested property again 2" value="some nested value 2"/>
        <property name="we must go deeper" value="irrelevant?">
            <property name="are we there yet" value="false"/>
            <property name="limbo" value="false"/>
            <property name="insanity" value="irrelevant?">
                <property name="limbo" value="true"/>
            </property >
        </property>
    </property>
   </properties>

@theHacker
Copy link
Contributor

I am missing such a feature for a long time.

For a first step (may be this could be an independent feature) I would like to see Tiled not removing custom XML data in a file. Currently if I write my custom tags inside a Tiled file and edit the file with Tiled and save it, Tiled would remove all tags it does not know.

If Tiled would preserve these tags it would be possible to easily save nested properties within the files. Tiled doesn't even have to understand these very game specific properties. The only thing Tiled would have to do is providing a possibility to view and edit these XML fragments.

Custom tags could be nested inside a special tag (for example <custom-properties>) to prevent clashes with existing tags.

With this solution the Tiled file would not be as bloated as with @Chaoseiro's XML and offers more flexibility because you can use tags and attributes as you seem fit.

<properties>
   <!-- Tiled's simple properties -->
</properties>
<custom-properies>
  <a-property>some value</a-property>
  <nested-properties-1 foo="some" bar="value" attributes-as-you="wish">
    <a-nested-property>some nested value</a-nested-property>
    <a-nested-property-2>some nested value 2</a-nested-property-2>
  </nested-properties-1>
  <nested-properties-2><!-- here: no "irrelevant?" -->
    <a-nested-property>some nested value</a-nested-property>
    <a-nested-property-2>some nested value 2</a-nested-property-2>
    <we-must-go-deeper><!-- here: no "irrelevant?" -->
        <are-we-there-yet value="false"/>
        <limbo value="false"/>
        <insanity limbo="true" />
    </we-must-go-deeper>
  </nested-properties-2>
</custom-properies>

@bjorn
Copy link
Member

bjorn commented Jun 19, 2016

@theHacker I don't really like that approach, because there is no place to keep arbitrary XML data around between loading and saving, and it would be non-trivial to add this. Tiled would have to keep around an XML DOM representation of the original file, and update it with changes made in Tiled before saving it back.

It would also be hard for 3rd party libraries to add support for that, as opposed to the slightly more verbose version proposed by @Chaoseiro.

@kheftel-old
Copy link

With the new multiline properties, you could put arbitrary JSON or xml data in a string property as a workaround.

@jjppof
Copy link

jjppof commented Mar 26, 2021

With the new multiline properties, you could put arbitrary JSON or xml data in a string property as a workaround.

That would be awesome. At least a JSON type would help a lot. Maybe a third party to help on textfield highlight and indentation.

@bjorn bjorn added this to Tiled 1.8 Dec 2, 2021
@bjorn bjorn self-assigned this Dec 2, 2021
@bjorn bjorn moved this to In Progress in Tiled 1.8 Dec 2, 2021
@bjorn
Copy link
Member

bjorn commented Dec 17, 2021

I've been doing some work on this in the past months, and I've now opened a pull request (#3215), which makes autobuilds with this feature visible, for those who'd like to give it a try. Any feedback would be very appreciated!

I did not get around to writing any documentation yet, but the gist is that the custom classes are currently stored in the project (alongside the custom enums that were added a while back), so make sure you have saved a project before using this.

Repository owner moved this from In Progress to Done in Tiled 1.8 Dec 23, 2021
@bjorn
Copy link
Member

bjorn commented Dec 23, 2021

I've decided to merge the pull request because I'd like to include this feature in a Tiled 1.8 beta release in its current state, and hopefully also to get more feedback that way. However, I'm reopening this issue because until the final release there remain a few things to be done:

  • Avoid a possible crash when the project file is changed to include a circular reference in the property types. At the moment circular references are only prevented from being created in the UI.
  • Add a warning dialog when removing a property type or one of its values / members, because this change can't be undone and might affect existing properties so it can be rather destructive.
  • Make enum and class property values accessible and creatable through the scripting API.
  • For enums stored as flags, limit the number of values to 32, since more flags can't be stored in the 32-bit integer that is currently used.
  • Implement import & export for property types.
  • Raise a warning when a custom property type is not recognized while loading a file.

@bjorn bjorn reopened this Dec 23, 2021
@hexus
Copy link

hexus commented Dec 23, 2021

Looks awesome. Very eager to try this out when the beta's released. 🎉

@bjorn bjorn moved this from Done to In Progress in Tiled 1.8 Dec 23, 2021
@jjppof
Copy link

jjppof commented Dec 23, 2021

Awesome feature!!

bjorn added a commit that referenced this issue Jan 13, 2022
It was possible to cause a crash with a crafted property types JSON
document that contained a circular reference. Now such references are
automatically removed on load, with an error reported.

Also added a test for this case.

Part of finishing issue #489
bjorn added a commit that referenced this issue Jan 13, 2022
Not sure if this is the best place, and unfortunately here the error
message can't provide any context. But it's better than silent failure.

Part of finishing issue #489
bjorn added a commit that referenced this issue Jan 13, 2022
Since these changes are immediately saved and can't currently be undone,
it's probably a good idea to have a confirmation dialog.

Part of finishing issue #489
bjorn added a commit that referenced this issue Jan 13, 2022
bjorn added a commit that referenced this issue Jan 17, 2022
When importing, the imported types are added to the existing types. When
an imported type has the same name as an existing type, the existing
type is replaced.

Also added autotest for the merging of property types.

Part of finishing issue #489
bjorn added a commit that referenced this issue Jan 17, 2022
Check for existing names when renaming classes and when renaming or
adding members.

Not checking these on enum values for now. While it does not make sense
to have multiple entries with the same name, it also doesn't really
cause any problems. Also, preventing entries with the same name might be
annoying while trying to reorder the values of an enum, for which no
specific actions currently exist.

Part of finishing issue #489
bjorn added a commit that referenced this issue Jan 18, 2022
Allows creating values of custom enums and classes from scripts.

Also added "@SInCE Tiled 1.8" where applicable.

Part of finishing issue #489
@bjorn
Copy link
Member

bjorn commented Jan 18, 2022

Closing again, since there are no tasks on the list anymore. I may publish another pre-release later this week, or at least on the string-freeze. I could still use more feedback, though! For those who want to test it, note that more recent builds than the 1.8 beta are also available.

@bjorn bjorn closed this as completed Jan 18, 2022
Repository owner moved this from In Progress to Done in Tiled 1.8 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants