godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Allow changing exported variables of scene tiles #7249

Open kingarthur0975 opened 1 year ago

kingarthur0975 commented 1 year ago

Describe the project you are working on

I'm making a Mario-esque platformer with question blocks. The way I have these blocks set up, I've exported a string variable that tells the block what item is inside. I also have an exported integer variable that tells the block how many of that item is inside. I've added the question block into a "scenes collection" tileset so I can easily place them inside my level. image

Describe the problem or limitation you are having in your project

The problem with this is that there isn't any way to change my exported variables on each tile, so every block placed down in the tilemap is stuck having all default values.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

What I think would help fix this would to be able to either select the tiles directly in the tilemap and change variables there, or to be able to change the variables in the tileset, so you can have multiple instances of said scene in the tileset, only with different variables.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Solution 1: changing tile variables in the tilemap

The user right clicks on a scene tile, and if there are any exported variables, a small pop-up menu appears where the user is able to alter the variables of that tile.

solution1

Solution 2: changing variables in tile properties in a scene collection tileset

The user selects a scene tile in a tileset, then edits variables under "tile properties". The user can then create multiple instances of the same scene in the tileset, just with different variables. Tile names may be modified so the user knows what variation of the tile they're placing down in the tilemap.

solution2

Solution 3: Scene tileset overhaul/"Scene Painting" (based on ideas presented in replies)

The user selects a scene in a Scene Collection Tileset to paint it in a tilemap. When the tile is painted, it isn't actually added to the tilemap, but the scene is simply instantiated as a child of the tilemap at the grid position. All of the scene's properties, such as position, scale, and exported variables, can still be fine tuned.

If this enhancement will not be used often, can it be worked around with a few lines of script?

A workaround to this is to manually place the scene as a child in your level scene, but it quickly becomes tedious to instantiate a child and position it every time you want to place one

Is there a reason why this should be core and not an add-on in the asset library?

This should be in Godot's core and not in the asset library because it directly improves upon the tileset/tilemap system.

elvisish commented 1 year ago

I'll add to this since my issue (https://github.com/godotengine/godot-proposals/issues/6672) was closed, currently I've just been using separate inherited scenes in the scene tiles to allow the functionality I need currently, but it obviously require a lot of extra work to set-up, whereas scene tile export variables would allow you to just use a single scene and change the variables per tile.

Beebster-UK commented 1 year ago

Solution (1) would be far more preferable and useful.

elvisish commented 1 year ago

Solution (1) would be far more preferable and useful.

I agree, although having both solutions available would have their uses depending on the requirements of the user and would probably provide the best flexibility.

kingarthur0975 commented 1 year ago

Solution (1) would be far more preferable and useful.

I agree, although having both solutions available would have their uses depending on the requirements of the user and would probably provide the best flexibility.

Exactly what I was thinking, having both options would allow for the most flexibility. If only one solution were to be implemented, though, I would prefer Solution 1.

groud commented 1 year ago

hey, thanks for the proposal. My opinion on the question:

Solution 1:

Being able to set the properties of a scene per-tile would require storing the scene's properties in the TileMap. It would require storing a ton of data assigned per-tile, which the TileMap node cannot do right now and is super complex to implement (it would require storing generic data per-tile, which is tricky).

Also, it would likely poorly if you have a ton of scenes with similar properties, compared to having several fixed scenes.

In any case, I think this kinds of goes against the TileMap design anyway, tiles are supposed to be objects that are the same, and used very often. It's not a generic "scene placer".

I think to help with that kind of scenarios anyway, we should implement a "scene painter" tool that would be able to paint scenes according to a grid, which grid could be borrowed from a TileMap node. That would allow having the list of instantiated scenes in the scene dock, and edit their properties accordingly (the TileMap node would not be responsible for instantiating those scenes, which would simplify things a lot).

Solution 2:

Why not, but I think the simple workaround for that is simply to create inherited scenes. That's more inline with how the TileSet scenes system is designed.

Beebster-UK commented 1 year ago

I think to help with that kind of scenarios anyway, we should implement a "scene painter" tool that would be able to paint scenes according to a grid, which grid could be borrowed from a TileMap node. That would allow having the list of instantiated scenes in the scene dock, and edit their properties accordingly (the TileMap node would not be responsible for instantiating those scenes, which would simplify things a lot).

Thanks for your response. The idea of a scene painter sounds intriguing and would likely fulfil most circumstances. It would also avoid unnecessary tile map refactoring / restructuring. Having a tool to place and align them to the map grid whilst having access to scenes properties would greatly increase the speed of iterating levels.

elvisish commented 1 year ago

There's a scene painter proposal here: https://github.com/godotengine/godot-proposals/issues/6886

I think that's a separate issue, since the Tilemap HAS got the ability to paint scenes, it should also have the ability to set exported variables on them, a scene painter is really a separate concept and not really related to Tilemaps at all.

Beebster-UK commented 1 year ago

My understanding of what @groud was suggesting was a grid based 2D scene painter that would also allow access to the exported properties. I'm assuming this is purely to decouple from the Tilemap structure which from the sounds of it would take some hefty changes to implement.

groud commented 1 year ago

since the Tilemap HAS got the ability to paint scenes, it should also have the ability to set exported variables on them

Well not really. You are not able to set properties on individual tile you place on the TileMap, so why would you have this feature for scenes ? It's quite consistent right now IMO. And it ensures acceptable performance.

While if you need to set individual scenes properties, then I think you should place them without the TIleMap, so that you can have the full control over those properties. I understand it's a bit trickier to do right now, but with a scene painter that would mimic the TileMap editor a bit, that should be rather easy to do.

elvisish commented 1 year ago

Well not really. You are not able to set properties on individual tile you place on the TileMap, so why would you have this feature for scenes ?

Well traditionally tilemaps aren't intended to place scenes at all, this is just a very convenient feature they have now, I don't see why scenes should be treated any less as scenes just because they're in a tilemaps editor. Plus, a scene placer wouldn't have the convenience that tilemap scene placement has (being able to clear all scenes by clearing the tilemap, for instance, or referencing the placed scenes via the tilemap node).

groud commented 1 year ago

Well traditionally tilemaps aren't intended to place scenes at all, this is just a very convenient feature they have now, I don't see why scenes should be treated any less as scenes just because they're in a tilemaps editor.

Because it's super complex to do and would make the API a lot more complex. The TileMap is not a "scene placer", it instantiates and remove scene automatically when you give a cell an ID instead. There's no data associated with a cell besides it's ID.

Such a solution would mean that every time you would want to instantiate a scene on a TileMap, instead of doing set_cell(layer, position, source_id, atlas_coords, alternative_id) you would also need ways to set properties on the given scene. Something like set_cell_scene_property(layer, position, source_id, atlas_coords, alternative_id, property_name, property_value) . It would also need the common getters and so on. This makes the TileMap API a lot more complex for something it was not designed for. I prefer to keep it simple, and scenes on a TileMap should limit to scenes that are placed several times, not just once with a set of very customized properties (which the normal scene editor is for).

Plus, a scene placer wouldn't have the convenience that tilemap scene placement has (being able to clear all scenes by clearing the tilemap, for instance, or referencing the placed scenes via the tilemap node).

A scene painter could solve that too, there's no reason why you would not be able to erase scenes easily too, provided we design it correctly.

groud commented 1 year ago

Thinking about it, maybe a simple solution would simply to enable a special "scene painting" mode directly in the TileMap editor. Basically, you would toggle a painting mode where painting a scene would basically instantiate it as a child of the TileMap node. This would let you edit their properties in the scene dock by selecting them as usual. It also means that set_cell() would not work on them anymore.

That may cover the main use case asked here while needing little code.

kingarthur0975 commented 1 year ago

Thinking about it, maybe a simple solution would simply to enable a special "scene painting" mode directly in the TileMap editor. Basically, you would toggle a painting mode where painting a scene would basically instantiate it as a child of the TileMap node. This would let you edit their properties in the scene dock by selecting them as usual. It also means that set_cell() would not work on them anymore.

That may cover the main use case asked here while needing little code.

This is actually a pretty good idea in my opinion! Having a seperate scene painting mode that instantiates child scenes would solve the issue of having to keep track of a ton of per-tile data. I updated the issue to add a new "solution 3" that describes how I think this should work.

Kakiroi commented 11 months ago

Such a solution would mean that every time you would want to instantiate a scene on a TileMap, instead of doing set_cell(layer, position, source_id, atlas_coords, alternative_id) you would also need ways to set properties on the given scene. Something like set_cell_scene_property(layer, position, source_id, atlas_coords, alternative_id, property_name, property_value) . It would also need the common getters and so on. This makes the TileMap API a lot more complex for something it was not designed for. I prefer to keep it simple, and scenes on a TileMap should limit to scenes that are placed several times, not just once with a set of very customized properties (which the normal scene editor is for).

I get that 4.0 tilemap is going for simplicity, but maybe it's way too simple? As long as users know the implication that modifying exported variable in scene tile will call more complicated api, I don't see why it shouldn't be possible.

Maybe scene tile can have "editable exported variable" bool check that automatically fetches and expands exported variable in the editor? Like "editable children" in scene tree. And when instantiating scene, it can just check whether it has "exported variable edited" bool to either call simple set_cell, or call set_cell_property with it.

mghicks commented 10 months ago

Such a solution would mean that every time you would want to instantiate a scene on a TileMap, instead of doing set_cell(layer, position, source_id, atlas_coords, alternative_id) you would also need ways to set properties on the given scene. Something like set_cell_scene_property(layer, position, source_id, atlas_coords, alternative_id, property_name, property_value)

If set_cell returned the instantiated scene nodepath or reference, I think the details could be left to the caller.

may-bell commented 1 month ago

being able to simply instantiate scenes (and thus be able to modify their export variables) using the tilemap editor would be life-changing. pretty please?

brandonrbridges commented 1 week ago

Jumping on this thread here to say that being able to change exported variables would be amazing.

elvisish commented 1 week ago

What is the alternative currently? Making up dozens (or more) of inherited scenes based on a single base scene, importing them all manually into their own scene file groups? It seems that if scene tiles are going to be available as an option (which is already very impressive) then a lot of the flexibility of using scenes should be also available, otherwise it's not any better (and in some case much worse) than just placing scenes manually and adjusting export variables as needed.

arran-nz commented 1 week ago

@elvisish

Making up dozens (or more) of inherited scenes based on a single base scene, importing them all manually into their own scene file groups

Unfortunately, yes - This is how I'm currently working around this in my project.

ParadoxV5 commented 4 days ago

Adding to the scene tile painter proposal, my project would like to connect a signal from one of my scene tiles to the HUD node outside of the TileMapLayer.


Such a solution would mean that every time you would want to instantiate a scene on a TileMap, instead of doing set_cell(layer, position, source_id, atlas_coords, alternative_id) you would also need ways to set properties on the given scene. Something like set_cell_scene_property(layer, position, source_id, atlas_coords, alternative_id, property_name, property_value) . It would also need the common getters and so on. This makes the TileMap API a lot more complex for something it was not designed for.

Scene tiles become child nodes of the TileMapLayer during runtime. Because of this, get_cell cannot find an instantiated scene tile the last time I checked (back when Godot was still TileMaps).

IntangibleMatter commented 4 days ago

Adding to the scene tile painter proposal, my project would like to connect a signal from one of my scene tiles to the HUD node outside of the TileMapLayer.

Scene tiles become child nodes of the TileMapLayer during runtime. Because of this, get_cell cannot find an instantiated scene tile the last time I checked (back when Godot was still TileMaps).

Wouldn't you be able to have the tiles as members of a group, then iterate through that group and connect the signals?

Not saying it's a good solution, but that should work

ParadoxV5 commented 4 days ago

Be it connecting signals or configuring exported variables, we don’t have to work things around in code if the editor can edit a tile scene similar to a regular node tree scene.

Wouldn't you be able to have the tiles as members of a group, then iterate through that group and connect the signals?

This is the same workaround for if the game dynamically creates those scenes. But the signficicance here is that the TileMapLayer statically tiled these scenes and thus they’re suitable for static configurations.