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

Giving tiles a property dictionary #12634

Closed
leoddd opened this issue Nov 4, 2017 · 43 comments
Closed

Giving tiles a property dictionary #12634

leoddd opened this issue Nov 4, 2017 · 43 comments

Comments

@leoddd
Copy link
Contributor

leoddd commented Nov 4, 2017

One very, very basic TileSet functionality that I believe to be missing is a simple dictionary to hold miscellaneous data, which other nodes in the scene can then read to be aware of how exactly they should treat the tile.

I've personally seen this feature in almost every single tilemap implementation I've seen and think it's pretty essential.

To give you an idea of what exactly I mean, we can use Tiled as an example:
propertydict
This tile, I would like to hurt any character that touches it. Of course, I'm not expecting the tile itself to run this code or be aware of anything, but for my characters to be able to see a tile they are colliding with and check its property dictionary for an "ontouch" value and do whatever that value tells them, in this case run a routine to hurt the player.
Another example I would actually like to use is, can you see that ice block on the left of that image? I gave it a property of "friction" with the value 0.3, which I would like to be able to read from within the game to adjust my character physics while walking over this tile. (This friction value isn't related to the built-in physics engine, it's just for my own KinematicBody physics handling.)
These applications are I think the most important reasons to implement something like this, just because I think it's actually impossible to get this sort of effect in Godot in any way at the moment, because these rely on the tile's own collision shape. The only workaround would be to completely strip the tiles' collision shapes and manually place TileMap independent staticbodies that can handle this interaction, which is... not optimal. Error-prone, inaccurate, cumbersome, all the bad things.

Similarly, I may want to run a script on level-load that places an object, such as a particle emitter, over specific tiles. It would be very helpful if I had some way to communicate to that script what type of particle emitter should be placed, and that one should be placed at all via these tile properties.
I realize that this example in particular could be done manually by hand, but so could a lot of things that a game engine is there to alleviate!
Another issue with doing it by hand is the much higher chance to make mistakes or to forget placing something.

I've seen people recommend to check the tile number in the TileSet resource and act on that, but I personally am not very happy with how inflexible this suggestion is. I may have different TileSets with different tiles in the same tile number slot, because a dozen tiles used in one TileSet would make no sense to even have in the other, so if I were to go by tile numbers then I would have to insert a lot of empty tiles just to make it fit.
Not to mention how hard to manage such a system would be in a bigger game, as well as its unintuitiveness.

The TileMap node and its lack of features is easily one of Godot's biggest 2D weaknesses, which is why I find myself having to resort to external tools (Tiled) to design my maps, and while it's absolutely no shame for an editor feature in an all-purpose editor like this to lose out to a specialized tool developed for exactly this use in terms of comfort and usability, it's not a great feeling to be stuck without a way to actually implement any sort of game logic inside the game based on a TileMap, something that even Tiled thought of.

Thanks in advance to anyone who may consider this idea, it'd really make me very happy if this were to actually be implemented eventually, especially since I personally feel like it's actually not that complex of a change!

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Nov 4, 2017

What this makes me wonder is if there'd be any good way to have tiles inherit properties from other tiles, or perhaps from the TileMap node itself. It'd be nice to, for example, have the TileMap node set default properties for all tiles within it, and then have individual tiles override those defaults if they need to.

Imagine making a Minecraft clone with this feature -- as an example, all tiles might need to have a walk_speed setting that defines how fast the player moves over it, but only some will need to change it from its default. It would be nice to have the TileMap implement the default value, and then only have it overridden for specific tiles.

@leoddd
Copy link
Contributor Author

leoddd commented Nov 4, 2017

I don't think that would be very needed. Default settings could be handled through anything ranging from level settings, other nodes, assumptions when the per-tile setting is missing etc.

On top of that, the TileMap node and the TileSet resource are definitely very different in function, there wouldn't be much sense in terms of organization for the TileMap node, which simply handles populating the scene with tiles, to hold any further information about these tiles or the TileSet.

At most I could imagine seeing that in TileSet, but even there I don't think it's terribly useful other than for very minor convenience.

@Zylann
Copy link
Contributor

Zylann commented Nov 5, 2017

If tiles were a resource, it would have been easy to just add a script of yours on them, in which you can put whatever you like :p

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Nov 5, 2017

First off, I didn't actually mean to confuse TileMaps and TileSets. But that's my bad for mixing the two.

Second, I'm not sure how it would be only a minor convenience. If you only have a few tiles, sure; but after a while, after you've added a lot of tiles to a TileSet, having a way to set the default for all of them rather than going through one by one would then be considered a possibly major convenience. Perhaps I'm not considering the reality of how TileSets are actually used? I'd still like to hear why you would consider it only a minor inconvenience, though, as perhaps I'm missing something.

Third, you mentioned using other nodes or other methods of creating a default properties list. While I won't deny that there is other ways, I'm not sure why you mention those options -- doesn't using one of those options contradict the point of custom properties being an option in the first place?

@Zylann
Copy link
Contributor

Zylann commented Nov 5, 2017

I just had an idea:
You could make a tool script that you attach to the TileSet resource (yeah, you can do that, every Object in Godot can have a script :) )
In this script, you would expose a variable number of properties, one per tile, through usage of get_property_list. You can even have global properties if you like.
This way, for every tile your tileset contains, you will be able to have an inspector to set their properties. You could even go further and add custom editor panels (with EditorPlugin API) to edit these properties in a more convenient way.
Then, in the game, you could retrieve those properties by writing tileset.tile_get_property(tile_id, "walk_sound"), where this function would actually be implemented by your script. WDYT?

@LikeLakers2
Copy link
Contributor

I may be misunderstanding slightly, but I'd like to make sure I get the basic idea. Are we talking something like this sort of setup?:

  • Attach script to TileSet resource (however you do that)
  • Script contains a tile_get_property method that checks if the specified tile has the property you want
    • If it does, it returns the value of that property
    • If it does NOT, it returns a default value, or otherwise null

Is this correct? Because if it is, I can see why it would work as an alternative and I could see it working, but once you get into creating your own editor panel, that to me feels like something that should be handled by whatever built-in editor panel will already be handling custom properties.

@Zylann
Copy link
Contributor

Zylann commented Nov 5, 2017

@LikeLakers2 you can do this without custom panel, just exporting vars in the inspector (with _get_property_list since you may have variable number of tiles). But if you want more, like having a visual selector or something more friendly, you can use the EditorPlugin API to do it, because using only the inspector could be too limited. This is exactly what TileMap does for example, to show a list of tiles on a left panel.

Then, if the scripted feature becomes great and popular enough, you will be able to port it to C++ with no hassle and have it merged into the engine ;)

Not saying it shouldn't be done, just pointing the fact that adding game-specific properties to tiles is currently doable with scripting :)

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Nov 5, 2017

Talked with @Zylann over Discord because I realized that I was having a hard time understanding his replies and figured a chat interface would help that a bit. I think I understand the suggestion for a tool script now (primarily I was confused over what was being suggested), though this still leaves me wondering why the primary suggestion for this is a tool script. Using a tool script would work, but it just feels sorta wrong for this purpose, so I'm going to stick to my suggestion for TileSet having a default properties dictionary that all tiles within that TileSet would inherit and be able to override.

EDIT: As it turns out, this whole debacle may have been a sort of misunderstanding. Whether from my side, Zylann's, or both, I think we both just misunderstood what the other was trying to do. Woops!

@Zylann
Copy link
Contributor

Zylann commented Nov 5, 2017

Just had another idea, maybe the scene-to-tileset exporter could make use of this #11384 ?

@leoddd
Copy link
Contributor Author

leoddd commented Nov 5, 2017

@Zylann I've actually already implemented something similar to what you're suggesting into my workflow as outlined here, but that is an approach that only really works by using a custom importer like that, since you can't set meta data on nodes through the editor interface itself yet.
It's a viable workaround until this exists in the editor in some form, yes.

@LikeLakers2 I was saying that having these default values on TileSet wasn't especially needed because any script that may want to access these custom tile properties could handle the default case itself.

Picture something like my earlier "friction" example, where tiles communicate different friction values to bodies that come in contact with them. Ranges from 0 to 1, we want a default of 1.
A code snippet to grab this value now would simply be something like this, where tile_id is the ID of our tile in the TileSet resource:

var tile_friction
if "friction" in TileSet.get_tile_props(tile_id):
  tile_friction = TileSet.get_tile_props(tile_id).friction
else:
  tile_friction = 1.0

Now every single tile that does not have a friction value explicitly set will assume that it wants to have a friction of 1.
At least, that was my original standpoint. I slept over it and now would agree that having a per-TileSet default value is useful, as for example some TileSets used for ice stages might just be more slippery by default, without now having to set the friction lower on each tile individually.

Also off-topic, but seeing your name surprised me a lot, I remember you from wayyy back on SMW Central! Small world.

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Nov 5, 2017

@leoddd Eyyyy. I barely use SMWC anymore. But anyways.

Sleepiness does a lot to you, eh? If we're in agreement that a per-TileSet default is useful, then I'd also like to make it a point to have a way to figure out if we're getting a default value or one specified by the tile. Perhaps I'm asking for a little much now, but it would allow for any single script to easily use its own override just like different TileSets can have their own defaults. We could perhaps have a function that only ever returns what's explicitly defined in a tile's properties. Something like this (look at the second and fifth lines for the changes):

var tile_friction
if "friction" in TileSet.get_tile_props_list(tile_id):
  tile_friction = TileSet.get_tile_props(tile_id).friction
else:
  tile_friction = 0.8

In this context, get_tile_props_list only returns a list of custom properties that are explicitly defined for a tile, ignoring all defaults (for example, returns {"friction":0.5}). get_tile_props, on the other hand, takes the defaults and merges them with what's explicitly defined for a tile (returns {"friction":0.5, "solid":true, "sound":null}). So in this example, if friction is explicitly overridden by the tile, we will use that value, otherwise we will use a hardcoded value in the script. (There's probably a better way to implement this, but this example gets my point across.)

I'm unsure what the use cases for such a thing would be, and thus unsure how useful this would be, but I'm sure some developer will find a use case, so having some easy way to figure it out rather than forcing the user to work around such a limitation themselves would probably be pretty useful. Plus, I don't imagine it would be too much trouble to implement either.

@leoddd
Copy link
Contributor Author

leoddd commented Nov 8, 2017

I don't know how useful that would be either. I guess that decision would just be left up to whoever would implement it in the end?

After reading through some related issues, I realized that one thing preventing this from being implemented is that there currently is no way to actually edit dictionaries through the Godot Inspector, which I am not sure what the status on that is as of right now (#12015).

@leoddd
Copy link
Contributor Author

leoddd commented Nov 8, 2017

Apparently there was a PR regarding this topic a while back, which was closed due to lack of discussion: #3089

@mhilbrunner
Copy link
Member

mhilbrunner commented Nov 15, 2017

FWIW, I never built a tile-based game not needing tile properties. So very interested in this, as otherwise I think everyone would just need to build it themselves.

Edit: Looking at the PR, it really should be done as a dictionary, and that dictionary would of course have to be editable, so this kind of depends on #12015 being done first.

@vnen
Copy link
Member

vnen commented Nov 28, 2017

The idea I had is similar to #3089, but without a way of setting metadata in-editor this is a bit pointless. If you're doing this via code, is quite trivial to use the TileSet resource metadata to store per-tile information. However, I do agree that a convenience function would be used often enough to warrant a core addition.

@erodozer
Copy link

For 3.1, to make this a bit cleaner, do you think it'd be possible to make an API change to Tilemaps as well. It'd be a lot more intuitive to have various properties attached to the Tile itself, instead of having to keep going through the Tileset and pass the ID around to get different things about the tile. Then the map and tileset can just share references to a tile in memory.

Something more like

Tile {
    int id,
    Dictionary properties,
    Texture texture 
}

so then I can go

tilemap.get_cell(x, y).properties["friction"]

instead of

var tile = tilemap.get_cell(x, y)
tilemap.get_tileset().get_tile_properties(tile)["friction"]

I know it'd require quite a big refactoring of the tilemap stuff as well as the editor to support it, but it's definitely an API improvement and could make it easier to extend the tiling code in the future.

@Zylann
Copy link
Contributor

Zylann commented Dec 12, 2017

@nhydock what you are asking for is a new function get_tile_at(x, y) then, because there is no point in storing dictionary, texture and all this stuff into every cells because it's directly deductible from the ID (Cells are lightweight pointers to a tile type). get_cell should stay as it is because it's lightweight, and you dont need to access the entirety of a cell's data all the time, and adding a new function would avoid breaking compatibility for the sake of a helper, which seems a bit overdone if it's to spare one line though.

@erodozer
Copy link

What I'm asking for is that each cell just has a pointer to a tile object/struct, instead of some index in a tileset and having to do lookups manually. Doing it that way could also decouple the tileset from the tilemap, since the tile reference is aware of what it needs to draw, or what tileset it belongs to. This is how libgdx does it.

@Zylann
Copy link
Contributor

Zylann commented Dec 12, 2017

@nhydock then I would advise that Tile becomes a resource, because creating a new dictionary with all the data everytime one cell is get/set seems quite of an overhead to me (not to mention the refcounts going on when you get an object instead of an int)
Also FYI a Tile contains all this: https://github.com/godotengine/godot/blob/master/scene/resources/tile_set.h#L92

@erodozer
Copy link

A resource is what I wanted. I by no means meant creating a dictionary on every single cell, that'd be stupid and incredibly wasteful. That tiledata struct also seems to be exactly what I wanted exposed directly through the tilemap instead of just providing the int id, just so then additional lookups would be unnecessary.

@leoddd
Copy link
Contributor Author

leoddd commented Dec 12, 2017

I agree that just being able to manipulate the Tile structs directly would be a lot more convenient than what currently exists, but you should make a separate issue for this.

@aaronmzn
Copy link

aaronmzn commented Dec 15, 2017

The main point of this issue is to add custom properties on tiles, right?

First things first, just making sure everyone is aware of this, Tiled main purpose is for making Tilemap levels, not for creating tilesets.

In Tiled you can have multiple Layers for building your level, in this case would be Tile layers, where in each layer you can edit properties and add Custom Properties, this mean every property you add to this layer would apply to all the tiles in this layer.

Now, if you have multiple tiles that hurt the player when touched then create a new layer and add the wanted propertie without adding it one by one for every tile, that would be tedious and inefficient, right?

Well, Godot's TileMap node works sorta the same, every TileMap node you add is a Layer, each layer has its own collision, and you can attach an script and then add variables there for the properties, without getting the tile location or things like that.

  • Something like this, where all the propeties are variables in the TileMap script:
	if is_colliding():
		if get_collider().onTouch == "hurt"
			hurt(get_collider().damage)
  • With groups:
	if is_colliding():
		var tile = get_collider()
		if tile.is_in_group("dangerous"):
			hurt(tile.damage)
  • Or even with dictionaries:

on Tilemap script:

	var prop = {
		onTouch = "hurt",
		damage = 10
		}

on Character script:

	var tile = get_collider()
	if tile.prop.onTouch == "hurt":
		player_life -= tile.prop.damage

@erodozer
Copy link

@aaronmzn The very first line of the issue says that OP is looking for properties on TileSets. Making multiple layers is a hacky work around for this problem. For something like a top down, grid-locked RPG, utilizing Godot's 2D physics and colliders is incredibly excessive when all I would actually need to do is check a boolean on a tile in the tileset. It's also a pain in the butt having to manage multiple tilemaps for something like procedural room generation in a roguelike and then have the character check between them all to understand what stuff they can do.

This isn't demanding we have props on each cell in a tilemap, we want props on each tile definition in a tileset, so then each cell can have inherited traits. It's an incredibly common use case, and would be incredibly helpful to have out of the box.

@leoddd
Copy link
Contributor Author

leoddd commented Dec 15, 2017

Also, the layer properties in Tiled aren't inherited by every tile on that layer in Tiled. Because Tiled doesn't manage any of that, it just exposes the data.
It may be that some .tmx implementation out there parses layer properties as properties for every tile in that layer, but that's neither mandatory nor "standard".

And yeah, I'm aware there are workarounds but they're all kinds of unintuitive and messy compared to just saving the data in the TileSet, where it can be reused easily across TileMaps and even projects.

@aaronmzn
Copy link

I haven't used Tiled since ver 0.14.2 and I just saw that since 1.0 you can open, edit, and save tileset separated from the tilemap, thats a cool feature but quite new for me.

  • If you have a TileMap node for bouncing blocks and one for the instakill blocks. its up to you to be careful with not placing a tile above a tile from another layer, because the caracter could either touch a bouncing tile or could touch a insta-kill tile or both.

  • On godot if you want a layer to be above the others you have to put it last, and if you want a tiled background that has no body/shape, then put it first, above all of them, then you put the player wherever you like, behind the foreground.

  • You can play around with the TileMap node's scene position so you can create the level you like, you should create a new scene with a Node2D as a parent and the TileMap nodes as childs, the player would be a child too so you set a spawner, and call this scene level_0-1 or something then you just play this scene and there it is.

@nhydock I was suggesting the most logical solution from how TileMaps and Godot works. I understand what you guys mean and it makes sense, seems like you guys want all the tiles in a single TileMap node and for a procedural generated levels seems a good reason. But while there is no way of doing that, instead you could use TileMap as Layers and script, when you have a single tilemap and you use get_collider() you'll be able to get the properties inside the script attached to the TileMap node. TileMap Layers on the other hand are the basics of Tilemaps. Again, as I said, godot is the same with the TileMap node as layers, you can workaround with this in the meantime.

And yes, sadly, on godot the only way to check if a tile has a certain property is by colliding with it, there is no overlap detection in KinematicBody for topdown games like you would do on Area2D with signals and setting trigger/disabled to true has no utility on this rather for Area2D detection, KinematicBody or even every physics body should have an overlap detection function and neither bodies should collide unless you tell them too wich collision layer/group will collide with, same with overlapping.

@leoddd Yes, that's what I meant but I formulated my words bad I guess, but you got me.

Rather than adding properties to every single tile you consider it as dangerous, just add a new TileMap where you put all the dangerous slopes and blocks in this layer and then add the property with a script, and access them with the code I writed before

Trust me, this is not messy, give a it test, try the dictionary method, it's easier and clean in comparison of what you guys think, and Tilemap layers are the basics of tilemaps, there is nothing abnormal. instead getting the properties from the tileset, you get it from the tilemap layer, like the old Tiled.

	if is_colliding():	# I forgot this line the last post, its important.
		var tile = get_collider()
		if tile.prop.onTouch == "hurt":
			player_life -= tile.prop.damage

@leoddd
Copy link
Contributor Author

leoddd commented Dec 16, 2017

Alright, but you're missing that I'm not asking for advice on how to handle this. This isn't the Q&A site.
I implemented a system to work around this using a Tiled importer plugin before even making this issue, but it is still a thing that should exist in-engine. The workaround I'm using in my importer only functions because you can do things through code that you can't through the editor.

A property field for each tile index in TileSets would be an ideal way to make this possible and easy to use in-editor.

@Eoin-ONeill-Yokai
Copy link
Contributor

I know this is a bit of a bump, but couldn't this be solved by simply letting users add child nodes to "tile" nodes? Even without a resource, you could do something like so:

- Example Tileset
| - grass
  | - encounter-data ( type: NoEncounter )
| - enc-tall-grass
  | - encounter-data (type: Encounter, rate: 0.5, monsters: some-resource-file) 
| - stone-tile
  | - collision-data

This way, you use the node system itself to determine what type of metadata a tile has?

@Zylann
Copy link
Contributor

Zylann commented Sep 7, 2018

The whole process of converting a scene into a tileset is called "Convert", as there was no proper user-friendly TileSet editor so the scene editor was used until now. If we end up adding nodes to the engine whose sole purpose is to be used by this converter and only in the editor, then why bother converting? And why not have a proper editor in the first place?

@mhilbrunner
Copy link
Member

Some related discussion: #15583

@avril-gh
Copy link
Contributor

avril-gh commented Sep 13, 2018

... my 3 cents.
theres a generally known saying that sometime a wise ones trying to solve simple things with most complex possible solution. (its about this thread and godot tiles in general)

I could see it like this :

  1. i have a node (any) - and i think about it as a 'tile', (im saving it to disk as a scene).
    then,
  2. I convert it to tileset resource (which is a simple list of scenes i pick).
    then,
  3. tile editor show me all these scenes in a form of 'tiles' that i can click.
  4. Like usual i pick some 'tile' within tile editor and put on grid.

The 'TileSet' node itself could be super simple, having fiew functions like get_tile_at( x, y, z)
yet it could be infinitely powerful and flexible - using what godot allready have - nodes and its inheritance.
Need anything fancy ? build it into particular 'tile' (scene) then use by get_tile_at( x, y, z).my_shiny_stuff(whoah)

Im allready using this way of creating tiled-levels for long time, but since theres no editor for it, i use tiled editor to create levels, then game loads level_xyz.json and creates whole level from it in the fly.

You can see me 'moaning' about slow instancing performance here #16769 ...and thats exacly what i been doing there, - building tiled level by instancing tiles which are scenes. ...And its been long time a go.
It works great, and yes godot can handle so many scenes even on mobile.

Theres just no editor to use real-scenes as 'tiles' and thats how TileSet node and its editor should be.
A simplicity but infinite flexibility, extensibility and power. (well, maybe to the edge of performance lol)

love ❤️ the godot for its nodes and scenes inheritance.

@HummusSamurai
Copy link
Contributor

@avril-gh I'd love to have a sample code for your workflow actually, it seems very powerful.

Do you have a small demo online somewhere?

@avril-gh
Copy link
Contributor

I'd love to have a sample code for your workflow actually, it seems very powerful. Do you have a small demo online somewhere?

@HummusSamurai actualy i thought about attaching some simple demonstration.
Since theres interest in it, i will perhaps do, so stay tuned. Will post it here.

@xDGameStudios
Copy link
Contributor

My post got closed as a duplicate of this one! I just wanted to add some ideias.

Apart from the dictionary idea, tiles could have a resource slot... resources are easier to edit, are memory friendly and more performant than using dictionaries. This way we could create a custom Resource for instance TerrainType with some variables: friction, damage, movement cost, walkable, etc. Then create different kinds of instances of that resource for each terrain/tile type: lava, water, ice, rocks, etc.
This way changing a property would be as simple as change only one resource file. And as they are data references and loaded only once.

The only drop back from the previous ideia is if someone wants tiles to have exclusive variables like, HP and when that specific tile HP drops to 0 the tile breaks... this wouldn't work with resources as they all would share the same resource reference.

I have one question!! what is the purpose of using scripts in the TileSet editor?! or even the tileset_script, variables.. if I export variables within them... I'm not able to access them elsewhere.
Can I?

@OvermindDL1
Copy link

For note, I have my tilemaps use an ECS style setup. I have pure primitive data arrays where the index matches to a rotating out index of tile positions (since mine are 0,0 centered, it's simple), or dictionaries for uncommon data. I'm effectively treating each tile as an entity in the ECS setup. I then have 'systems' that operate over those data sets for fast mutation that can trivially be multi-threaded, then a final join. It's a bit horrifyingly ugly in GDScript unlike C++ or Rust or so (quite the swap there!) but it works really well and is actually very efficient even in GDScript.

@xDGameStudios
Copy link
Contributor

For note, I have my tilemaps use an ECS style setup. I have pure primitive data arrays where the index matches to a rotating out index of tile positions (since mine are 0,0 centered, it's simple), or dictionaries for uncommon data. I'm effectively treating each tile as an entity in the ECS setup. I then have 'systems' that operate over those data sets for fast mutation that can trivially be multi-threaded, then a final join. It's a bit horrifyingly ugly in GDScript unlike C++ or Rust or so (quite the swap there!) but it works really well and is actually very efficient even in GDScript.

May I ask you how do you add the data to each tile?! I get each tile is an entity... but how do you add the data to it?! Manually, load from a JSON file, hard-coded into the GDScript file?! The ideia was how to do it without... hard coding it inside the code file. When working with a large team with programmers and designers you don't want designer messing with code, you know. Well just asking

@OvermindDL1
Copy link

I don't take tiles as entities. I encode their position into an integer packed to the positive near 0 (swizzling and so forth the coordinate positions, it's simple bit work), then just index into arrays or dictionaries using that value as per proper ECS standards (The 'Entity' in ECS is ALWAYS an integer, just an integer, if something does otherwise then its probably not ECS). I do load it from a JSON file, it's pretty simple, though GDScript makes it irritating enough that I'm leaning to using C++/Rust even for simple mockups again...

@Lucrecious
Copy link

Lucrecious commented Nov 13, 2018

My take:
I don't see the issue with just having a KeyValue Array for every tile index in a TileSet - as in a dictionary per tile id and possibly one for the TileSet default.

I looked at tile_set.cpp, and adding an extra property for custom meta properties would probably be under 50 lines of code in total. That would be including the inspector property to visually manage it, the storage under the TileSet class and the binding to gdscript.

I think it's pretty clear that the need for meta properties per tiles is real, and it could be as simple as the Godot developers want it to be.

tileset.get_meta_properties(id) can get the dictionary. If space is an issue, a Dictionary can be restricted to only be created when the user actually needs a tile to contain a meta property.

I don't know, in my eyes it's weird this hasn't been done yet. Not only is this a simple pull request (again, under 50 new lines of code so merging is easy, I could probably add this feature in a few hours) and would benefit pretty much everyone making 2D tile-based games.

@Lucrecious
Copy link

#23714

@willnationsdev
Copy link
Contributor

@Lucrecious @xDGameStudios I think that both approaches could be used here, to support both Resource/Resource script approaches and the looser Dictionary approach.

  1. Define a TileProperties class that extends Resource and add a Ref object to the TileData struct. Then expose it from the TileSet alongside the other tile_* properties in the TileSetContextEditor. Users can create their own TileProperties object to associate with each tile. Perhaps we could also set a TileProperties default script in the TileSet so that it creates new tiles with a new instance of this TileProperties object already assigned (@LikeLakers2).

  2. Expose the tileset_script's properties to the TileSetContextEditor, that way exported properties can be edited from the Inspector.

  3. For convenience, create a Dictionary called meta_properties that lets people set properties for the overall TileSet more dynamically.

This strategy gives Resource support both for TileSets and their underlying TileData structs (for maximum power/flexibility) but also grants the more user-friendly Dictionary access as desired.

@willnationsdev
Copy link
Contributor

FYI, I will be ready to launch a PR for this tomorrow. In my fork's "tileset" branch, I have added a Dictionary "meta_properties" to the TileSet class and made it so that all script variables and the meta properties Dictionary are both visible from the TileSetContextEditor's Inspector. This means that you can expand the TileSet sub-inspector from the TileMap Inspector to view all of the TileSet properties or you can do so by opening the TileSet editor and expanding the "TileSet" group of properties which display the same information.

In addition, a TileProperties class has been added to TileData and exposed in the "Selected Tile" group of the TileSetContextEditor. The TileProperties script is also accessible from here, so that it can be easily re-assigned without having to separately open the TileProperties resource in its own Inspector.

Finally, a default TileProperties script can be assigned to the TileSet so that it forces all tiles in the TileSet to use said script. It assigns the script to all existing tiles as well (that way a common set of properties may exist on all tiles in the TileSet). This keeps you from having to individually set the script on every TileProperties resource used by the tiles in the TileSet.

I plan to re-post the above information + pictures in the PR. Let me know what you guys think.

@xDGameStudios
Copy link
Contributor

It looks like it fails to build in Travis CI!!
never the less these are great news... I was working around this limitations for quite sometime now!
Hope to see this merged soon ;)

@willnationsdev
Copy link
Contributor

@xDGameStudios I'll be fixing it up in an hour or two when I get off work. No worries!

Also, if you'd like, you should be able to cherry-pick the commit once its ready (i.e. once I submit the PR) so that you don't have to wait for it. :-)

@akien-mga
Copy link
Member

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.