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

Being able to have an array custom property #1493

Open
issy123 opened this issue Mar 16, 2017 · 23 comments · May be fixed by #4002
Open

Being able to have an array custom property #1493

issy123 opened this issue Mar 16, 2017 · 23 comments · May be fixed by #4002
Assignees
Labels
feature It's a feature, not a bug.
Milestone

Comments

@issy123
Copy link

issy123 commented Mar 16, 2017

Request

It would be nice if we could add a custom property of type array.
Mostly an array of strings.

I can't really see use for other datatypes, so just add array as option (which would be an array of strings)

Example

image

Reason

We currently use a comma seperated list to write down which doors a button would open. But this is prone to errors and prefer an array list.

@TurBoss
Copy link

TurBoss commented Mar 16, 2017

you can use json, i think...

@bjorn bjorn added the feature It's a feature, not a bug. label Mar 16, 2017
@bjorn
Copy link
Member

bjorn commented Mar 16, 2017

While you can of course write JSON or comma separated values in a string property, I do think it would indeed make sense to support array properties explicitly, though it should ideally allow an array of any available type. Alternatively, the elements in the array could each be allowed to be of any type.

@issy123
Copy link
Author

issy123 commented Mar 16, 2017

@bjorn What use case would satisfy the need of having any type of array?
I can only see a use case in string arrays.

Also why heterogeneous arrays? If they need heterogeneous arrays let them add custom properties of that type. This would also add complexity to parsing this, because you need to check for each array item what type it is.

It's fine by me adding those other types of arrays but it would create a huge list of options and maybe make them confused or being not user-friendly.

@EJlol
Copy link

EJlol commented Mar 17, 2017

I definitely can see use cases for other types too. A few use cases I can come up with:
A Chest object that contains several items (id / integer)
A Monster spawn area with several recolored monsters (color)
A light object that sends some kind of morse code (boolean)

@bjorn
Copy link
Member

bjorn commented Mar 17, 2017

Also why heterogeneous arrays? If they need heterogeneous arrays let them add custom properties of that type. This would also add complexity to parsing this, because you need to check for each array item what type it is.

There shouldn't be much additional complexity to parsing heterogeneous arrays than to parsing heterogeneous property values. Anyway, the reason I considered it is because it is the normal state of affairs for any dynamically typed language. The type is associated with each value rather than with the array, and this is something many people are used to working with. It's also how JSON arrays work. So actually enforcing that each value be of the same type may be more complicated than just allowing them to have different types.

That said, I do agree the use-case is rare, so I will leave both options open until we can evaluate them while implementing this feature.

It's fine by me adding those other types of arrays but it would create a huge list of options and maybe make them confused or being not user-friendly.

That depends on how it is implemented. Most of the work is probably going to be in adjusting the Properties view to be able to work with arrays and allowing to add/remove elements. Maybe it can be done inline, or maybe it makes more sense to spawn a dialog for this. And all this work is not going to be much harder if any data is supported rather than just supporting strings.

So I'm not talking about adding just a "string array" type of property, but rather adding an "Array" checkbox separate of the type (or adding just an "array" property type, if values will be allowed to be heterogeneous).

@issy123
Copy link
Author

issy123 commented Oct 17, 2017

4 upvotes, looks like they want it.

@bjorn is it going to be implemented?

@bjorn
Copy link
Member

bjorn commented Oct 17, 2017

@issy123 Yes, it's going to be implemented. But currently I'm trying to focus on getting Tiled 1.1 out since there are already plenty of new features since 1.0. But unfortunately there are some things that remain to be done as seen on the Roadmap.

@issy123
Copy link
Author

issy123 commented Oct 17, 2017

Awesome man!
I never knew github has such feature.

I'll be sure to support you on patreon!

@bjorn
Copy link
Member

bjorn commented Oct 17, 2017

Thanks, that's really appreciated!

@styk-tv
Copy link

styk-tv commented Jan 6, 2019

Definitely +1 for the list. It would be really great to be able to past json fragments. Its what I do now with the string property but looking at few lines of text and several squashed properties in one line is difficult to read. Sometimes I lock entire state machine sequence in a tile Type, would be wonderful to have better list visibility on more structured level. Start with multiline json would be great. It is then very easy to encounter that based on Type and consume into Tile capability from a single command (without having to parse each line separately. I also experiment with post-processing where this is done post-save.

@justgook
Copy link

justgook commented Jul 4, 2019

In my opinion you can make just as visual representation in custom props and keep storing data as is.
That will not introduce new problems for parsers, but will allow navigate properties much easier, and as soon as you create name with . in name, or just name like "myArray[]" - it automatically become array, that in save-file is just "myArray[0]"
image

@justgook
Copy link

justgook commented Jul 4, 2019

Btw - that way you also would be able store custom types in same array

@Phlosioneer
Copy link
Contributor

I'd like to start tackling this; with #2712 almost finished, it's part of my use-case to need an array of connected objects.

Looking over this thread, here are my thoughts on the design:

  • Limit arrays to one type. This is useful for two reasons:
    • Lets us re-use the code that Color properties currently use, where they unfold to show an array of ints corresponding to RGB.
    • We can later expand this to any type without losing backwards compatibility.
  • Prevent arrays of arrays. The use case for this is small compared to the additional complexity, and no one here has advocated for this explicitly. It can be useful; but I think it's more important to ship an array property first and worry about the possibility of this later.

@bjorn Any other thoughts before I start working on this?

@bjorn
Copy link
Member

bjorn commented Jan 15, 2020

@Phlosioneer As I wrote previously, I'm open to either typed or untyped arrays. However, I do think untyped will be easier to implement since that's just adding one new property type "array", which could probably use a QVariantList and instantiate a QtVariantProperty for each entry, which will automatically pick the right widget.

Since we can't actually use real typed arrays on the C++ side anyway, limiting the arrays to a specific type may only add complexity. It means we need to store the type along with the array somehow (unless you'd only allow empty arrays to be untyped, and derive the type of an array from its first element). In this case, I'd be inclined to add type restriction later if there is a real demand.

I think it's fine not to support arrays of arrays, unless it would work without additional complexity.

@bjorn bjorn added this to the Tiled 1.5 milestone May 13, 2020
@bjorn bjorn modified the milestones: Tiled 1.5, Tiled 1.6 Sep 29, 2020
@UliAbo
Copy link

UliAbo commented Mar 13, 2022

Just simple arrays with strings (not arrays of arrays) would already be very helpful - is this something we could expect soon? I guess not as it's on no roadmap / milestone listed or do I understand something here wrongly?

@bjorn
Copy link
Member

bjorn commented Mar 14, 2022

@UliAbo It's currently on the Custom Property Enhancements milestone, but indeed this is more of a collection and not a timeline. Custom properties are not the focus of the next release, but I would like to handle arrays in the near future.

@joereynolds
Copy link

joereynolds commented Oct 10, 2023

Just adding my two cents but array property types would be very useful for me.
I'm currently storing more complex data as JSON in a string
image

Whilst this works. It's fiddly to edit and completely bypassess all the niceties of Tiled's other properties. I.e. I can't use a color type or an object inside the JSON. In those cases, my vote would be for heterogenous arrays, not just string types.

@eishiya
Copy link
Contributor

eishiya commented Oct 10, 2023

I'm currently storing more complex data as JSON in a string

I'm curious, why do you not use custom types in Tiled for this? JSON seems unnecessary here, you can have e.g.
component[0]: property of type "animatable", component[1]: property of type "plays_sound_on_event" with sub-properties "sound" and "event", component[2] of type "dies_on_event" with sub-property "event", and so on.

Untyped arrays would certainly be useful for lists of components though, so I support the request for those.

@joereynolds
Copy link

I'll be honest, the only reason is because I haven't started looking into custom properties yet and this was a quick way to get me up and running.

I'll take the suggestion on board and have a look though, thanks!

@ironpowertga
Copy link

A first possible implementation of arrays, which shouldn't have too much impact, would be when the user copies and pastes a property, to check whether this property already exists. If so, we add a number that we increment each time we copy. PropertyName0 property PropertyName1 etc

@eishiya
Copy link
Contributor

eishiya commented Jan 20, 2024

A first possible implementation of arrays, which shouldn't have too much impact, would be when the user copies and pastes a property, to check whether this property already exists. If so, we add a number that we increment each time we copy. PropertyName0 property PropertyName1 etc

This would interfere with modifying properties through copy+pasting and would only be an array in name only, something you can already do in Tiled without any additional features in Tiled. To aid in making "propertyName[N]"-style faux arrays, one could write a Tiled script that lets you put a name in and the number of values to add (and perhaps a starting index, to help with adding more indices to an existing "array".

I don't think there's a need for Tiled to "officially" support faux arrays and muddy the waters for later when real arrays are added.

@MarkOates
Copy link

bumpy bump. 👍 +1 in support of adding an array type as a custom property.

(I ran into this limitation when wanting to make a list of candles 🕯️ that will light up when a lever is activated.)

Consequently, after reading through the issue, I'm in support of the untyped arrays. Same as:

However, I do think untyped will be easier to implement since that's just adding one new property type "array", which could probably use a QVariantList and instantiate a QtVariantProperty for each entry, which will automatically pick the right widget.

The end user's importer would likely be the place to validate the consistent types if that's what they want. Untyped arrays already exist in the objects, for instance.

@GabrielBigardi
Copy link

GabrielBigardi commented May 8, 2024

Bump.

I was searching for this feature and it was said 3 years ago that it was planned for the upcoming release:
https://discourse.mapeditor.org/t/array-custom-property-type/4933

Please add it soon, it would be very helpful for lots of people, including me.

@bjorn bjorn modified the milestones: Tiled 1.11, Tiled 1.12 Jun 21, 2024
@bjorn bjorn linked a pull request Jul 10, 2024 that will close this issue
4 tasks
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
Status: Todo
Development

Successfully merging a pull request may close this issue.