duality icon indicating copy to clipboard operation
duality copied to clipboard

Introduce Custom Tileset Data Tags

Open ilexp opened this issue 9 years ago • 20 comments

Summary

Right now, there is no builtin way that allows users to specify additional data layers for a Tileset. It would be nice if there was a convenient was to specify data tags for groups of tiles.

Workaround

  • Users can already specify additional data in an array and index it using the tile index.
  • This, however, is not very usable.

Analysis

  • Add an additional TilesetEditorMode where any number of custom data layers can be specified.
  • Each layer, when selected, allows the user to specify a tag object in the Object Inspector.
  • For each layer, the tileset view allows to specify all the tiles that will be tagged.
  • The TilesetCompiler will generate one output layer for each input layer.
  • A Tileset should specify API for conveniently retrieving tags for a tile, or all tiles of a certain tag layer.
  • Note: It might be a good idea to specify that target API first in this case.

ilexp avatar Aug 09 '16 17:08 ilexp

Picking this one up. I already had to workaround this limitation in pathfindax while specifiying the movement speed of a tile.

The layers will have a string as key and will be stored in a dictionary. The API will provide functionality to lookup a certain layer and then the tile index can be used to get the data in that layer for that particular tile.

In order to add data to a tile the user firsts needs to select a layer and tile. Then the actual data can be filled in in the property editor. I believe for selecting the tiles l can reuse a bit of the autotile logic. The first version will just be strings but we could expand on this by adding a data type field to the layer

Rick-van-Dam avatar Jan 08 '18 14:01 Rick-van-Dam

Picking this one up. I already had to workaround this limitation in pathfindax while specifiying the movement speed of a tile.

I'll assign you 👍 Be aware that the first posts analysis is just a draft. It may have missed some aspects of the issue, so when in doubt, let's talk and figure stuff out.

The layers will have a string as key and will be stored in a dictionary. The API will provide functionality to lookup a certain layer and then the tile index can be used to get the data in that layer for that particular tile.

Sounds good! Would be interested in seeing an API draft as soon as you're confident with your design to see where things are going.

In order to add data to a tile the user firsts needs to select a layer and tile. Then the actual data can be filled in in the property editor. I believe for selecting the tiles l can reuse a bit of the autotile logic. The first version will just be strings but we could expand on this by adding a data type field to the layer

A "select layer and tile, then edit" approach is a good fit for usage scenarios where each tile can carry highly specific data, but it doesn't match well in scenarios where you'd for example just want to tag all tiles with a gravel road surface so you can play the right footstep sounds. In those cases, a "select layer and value, then tag tiles" would be preferable.

How about an approach where in the Tileset editor you'd select a layer first, then, maybe in the Object Inspector(?) select the value you want to "paint", and then click all the tiles you want to apply that value to? Clicking twice could erase the tag from the tile again, and in the Tileset view you could highlight all the tiles that currently have the same tag value, vs. no tag value, vs. a different tag value.

  • Painting the same tag value is super easy.
  • Painting highly specific tag values is about the same as in "select layer and tile", except that the steps of tile selection and entering value are reversed.
  • Still not too hard to implement, as no new editor UI is needed, just the existing one used in a certain way.

What do you think?

ilexp avatar Jan 09 '18 17:01 ilexp

Hmm sounds like a good idea to add some way of copying values to other tiles or editing multiple tiles at the same time. Could be implemented on top of a basic implementation I think.

Currently focusing on getting a minimal tile data tag system going. After that I could see if I can implement some extra nice to have functionality and flesh out a nice API.

Rick-van-Dam avatar Jan 09 '18 17:01 Rick-van-Dam

Hmm sounds like a good idea to add some way of copying values to other tiles or editing multiple tiles at the same time. Could be implemented on top of a basic implementation I think.

You mean "tile" as in "a tile in a tileset", right? Sort of an ambiguous term in this issue. When in doubt, maybe we could use "tile info" for the item from a tileset and "tile" for the one in an actual tilemap, matching their struct names in source code.

Currently focusing on getting a minimal tile data tag system going. After that I could see if I can implement some extra nice to have functionality and flesh out a nice API.

Alright, let's see where this is going :)

ilexp avatar Jan 09 '18 18:01 ilexp

You mean "tile" as in "a tile in a tileset", right? Sort of an ambiguous term in this issue. When in doubt, maybe we could use "tile info" for the item from a tileset and "tile" for the one in an actual tilemap, matching their struct names in source code. Yup

So I now have a very basic (and dirty) setup where its possible to input custom (string) data in a tileset. Except for calculating the hashcode (so that the apply/revert buttons get enabled properly) I have not touched the whole tilecompiler thing. Is this correct for a feature like data tags?

I have to test some more and start thinking about making it more usable:

  • Clean the code
  • Fix the bug with the dictionary so I can change the list to a dictionary like intended. Wouldnt want to search through a list when its not needed. Also provide a userfriendly API for this. Probably something along the lines of Tileset.GetDataLayer(key) and a GetValue(tileindex) method for the datalayer.
  • Adding support for data types other than a string.
  • Adding tileselection highlighting like in the other modes
  • Adding copy/paint functionality like adam suggested

Rick-van-Dam avatar Jan 09 '18 20:01 Rick-van-Dam

I think I keep it in a list instead of a dictionary. Cant seem to get a dictionary working properly with the UI. Maybe we can use the compile functionality to put the records inside a dictionary?

Rick-van-Dam avatar Jan 10 '18 16:01 Rick-van-Dam

Regarding:

So I now have a very basic (and dirty) setup where its possible to input custom (string) data in a tileset.

and

Adding support for data types other than a string.

and

Cant seem to get a dictionary working properly with the UI.

If you're using the Object Inspector to set the tag value, and the tag value is a property of type object, then it should basically do most of the work for you already. Not saying this is the ideal approach, but it should work. What kind of problems are you facing, and what approach are you using / planning to use?

Except for calculating the hashcode (so that the apply/revert buttons get enabled properly) I have not touched the whole tilecompiler thing. Is this correct for a feature like data tags?

In the trivial case where there is a 1:1 mapping from input tiles to output tiles, this is enough, but the proper way is through the compiler. The main reason being that the compiler will at some point be able to generate new tiles that do not exist in the input data (think about AutoTile variants). Check out the compiler input and output containers, as well as their input and output fields in the tileset class. Ideally, the tags would be in line with both the inputs and outputs.

ilexp avatar Jan 11 '18 19:01 ilexp

If you're using the Object Inspector to set the tag value, and the tag value is a property of type object, then it should basically do most of the work for you already. Not saying this is the ideal approach, but it should work.

Currently using a string as type so Ill change that to object.

What kind of problems are you facing, and what approach are you using / planning to use?

How do I edit the key value of the dictionary in the objectinspector? Would probably need more logic for this to keep them in sync as the key is in both the object and the keyvaluepair. Alternatively I could just use the list as a input to generate a dictionary in the compilation step and keep the UI part simple by using a list.

Rick-van-Dam avatar Jan 12 '18 12:01 Rick-van-Dam

@Barsonax Your last comment looks cut-off - what's the missing part?

ilexp avatar Jan 12 '18 18:01 ilexp

Fixed

Rick-van-Dam avatar Jan 12 '18 19:01 Rick-van-Dam

How do I edit the key value of the dictionary in the objectinspector?

This is not supported right now. You can add and remove keys, but not edit them.

ilexp avatar Jan 13 '18 18:01 ilexp

This is not supported right now. You can add and remove keys, but not edit them.

In that case I will keep it as a list and use that as a input to generate a dictionary

Rick-van-Dam avatar Jan 13 '18 18:01 Rick-van-Dam

I have been thinking about making assigning values more user friendly. Alot of the time you will have a few different 'classes' of data. For example all grass like tiles will have a speed modifier of 0.8. It could happen in the future that you want to change this to be 0.75. Now you have to go through all those tiles again...

The idea is that each layer will have a list of value 'classes'. A user can assign a value class to a tile which will set the value of that tile to be equal to the value of the class. If the value of the class changes all tiles with that class will change as well. Possibly some batch tools could be added to quickly assign these classes to tiles.

It is still possible to assign a value directly to a tile. If that tile had a class assigned that class is removed from that tile if this happens.

Still not sure if this is even possible with the object inspector

Rick-van-Dam avatar Jan 21 '18 21:01 Rick-van-Dam

The idea is that each layer will have a list of value 'classes'. A user can assign a value class to a tile which will set the value of that tile to be equal to the value of the class. If the value of the class changes all tiles with that class will change as well.

If I understand you correctly, that actually was the original idea 😄 From the first posting / the issue description:

  • Each layer, when selected, allows the user to specify a tag object in the Object Inspector.
  • For each layer, the tileset view allows to specify all the tiles that will be tagged.

The tag being the value, and each layer being a class. You'd have a "grass" layer and tag all the tiles that are grass tiles. Then you'd have a "stone" layer and tag all the tiles that are stone. And so on. Since the layers are user-defined, you can have some layers defining one property of a tile, like surface type (grass / stone / etc.) and other layers define other properties. And since every layer only has one value, you have a central place to change that value by selecting the layer.

So if that's what you're going after, I'm 100% with you here.

ilexp avatar Jan 22 '18 19:01 ilexp

Hmm I think I have a different idea with 'layer' than you. Like so:

  • Tileset:
    • List of TilesetDataTagInputs (like a layer specifying movement penalties and another for a different thing). Can be accessed by key.
      • List of DataClass (specifies standard classes of data, this is just here for convenience as its only used in the UI). I havent implemented this yet.
      • List of DataTagTileItems (contains the data tag values for all the tiles)

Lets try to get the naming standarized. I think I use different terms for the same things as you.

Rick-van-Dam avatar Jan 22 '18 19:01 Rick-van-Dam

Do you have a use case where you would need to assign unique tag values to different tiles besides what can be covered with a reasonable amount of layers?

ilexp avatar Jan 22 '18 20:01 ilexp

Except in the situation where all or most values are unique not really I guess. Now I think of it it might be better to only allow assigning values through classes.

Is there a way to make a dropdown in the objectinspector where I can specify the values in code?

Rick-van-Dam avatar Jan 22 '18 20:01 Rick-van-Dam

Except in the situation where all or most values are unique not really I guess. Now I think of it it might be better to only allow assigning values through classes.

Yep, I think we might also gain some usability here for the simpler concept, and also get away with a simpler editor implementation too.

Is there a way to make a dropdown in the objectinspector where I can specify the values in code?

Unfortunately not, you'd need to specify a custom PropertyEditor for that.

Since PropertyEditors are selected by property type, and you probably don't want to create a custom type for this, it might be easiest to create a special PropertyEditor for the "parent" object type, which has the property, and then implement a special case for this specific property in there. As an example, take a look at this one, which is a similar use case.

ilexp avatar Jan 24 '18 17:01 ilexp

Tried adding such a property editor but how do I make it actually uses it? Just setting the type doesnt seem to do it.

EDIT: seems to be a priority issue. Raised the priority now it works. EDIT2: now it seems to be locked to readonly and setting that to false doesnt work??

Also I dont think I can get the object instance from the MemberInfo alone. This would mean there is no way to read the list of DataClass instances. I dont know how propertyeditors work exactly so I might be misunderstanding you here.

Rick-van-Dam avatar Mar 05 '18 12:03 Rick-van-Dam

I'm not in the right context to grasp the big picture of your questions right now, but I'll answer them individually hoping it helps:

Also I dont think I can get the object instance from the MemberInfo alone.

Yep, MemberInfo just describes a type member, not tied to any concrete instance. To get the (Remember: Multi selection!) object instances of a PropertyEditor, use this.GetValue() like here, here or here.

EDIT2: now it seems to be locked to readonly and setting that to false doesnt work??

PropertyEditors are generated recursively. The root editor has a Getter and Setter assigned that retrieve the grids selection directly, and each editor either allows to edit that value directly (primitives, colors, etc.) or generates child editors to edit specific subsets of that value (like properties, fields of an object), which are assigned an anonymous Getter and Setter to retrieve that specific value, or set it. Whenever a PropertyEditor doesn't have a Setter assigned, it is forced to be readonly - for example when there's a property without a setter implemented.

Take a look at how PropertyEditors construct child editors. For example, the GameObjectOverviewPropertyEditor generates the shared GameObject "header" editor, and a list of Component editors.

ilexp avatar Mar 05 '18 17:03 ilexp