godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.2k stars 21.21k forks source link

erase_cell() does not erase scene tiles cells #69596

Closed YaikaRace closed 1 year ago

YaikaRace commented 1 year ago

Godot version

4.0.beta7

System information

Windows 10

Issue description

When using erase_cell() on a cell in which the TileMap set a scene from a scene collection the cell is not erased, and when this tile is moved it is impossible to put another tile in the same cell again.

Steps to reproduce

  1. Create a scene and put some sprite
  2. Save the scene and put it in a scene collection in a tilemap.
  3. use set_cel() to put the scene in a cell on runtime
  4. use erase_cell() to delete that scene tile on runtime

Minimal reproduction project

Scene Collection issue.zip

KoBeWi commented 1 year ago

The tile IS erased, it just doesn't remove the created scene instance (and you can't recreate it, because it's cached). See #67330 I think we lack a method to force re-instancing new tile.

YaikaRace commented 1 year ago

There is some way to clear that cache or something similar to be able to re-instantiate a scene in the same cell where was freed a previous scene?

codingvessel commented 1 year ago

So, is there any possibility in gdscript to remove the actual scene from the tilemap?

KoBeWi commented 1 year ago

No. You can queue_free() the node, but it will still sit in the cache. The cache can be cleared by modifying the TileSet resource. You could try doing some dummy operation on the tileset.

This issue was discussed on the chat and it was decided that set_cell() should remove (and free) the cached scene on that tile. Someone has to implement it (I tried and ran into some problems).

YaikaRace commented 1 year ago

I tried doing a dummy operation in the Tileset, but that caused the entire cache to be cleaned and all instantiated scenes to be re-instantiated and duplicated.

Sch1nken commented 1 year ago

Running into a similiar issue but there seem to be some inconsistencies in general: When I do:

erase_cell()  
#or  
clear_layer()  
set_cell()

my old scenes stay. But when I do:

set_cell()  
erase_cell()  
#or  
clear_layer()

It won't create any scenes at all (probably because they only get instantiated during the next process or similiar?).

I might take a crack at removing the scenes as discussed earlier in this issue.

Edit: Whats the right approach here? Say I set_cell() with a Scene and a Node gets created. Then I queue_free() the Node (for whatever reason) without using set_cell/erase_cell to clear it. Should the TileMap periodically do some checkups (to check if the Nodes are still there?).

Or should the TileMap actually listen to the Nodes tree_exiting/tree_exited signals? This seems like the most consistent to me IMHO. So when someones moves or removes the Node the TileMap knows and gives up "ownership" of said Node?

Sch1nken commented 1 year ago

Okay I got a first version running locally. I fixed a crash and so far it seems stable.

The only issue right now is that when a new scene is instantiated the Nodes name swaps between Node and @Node@ when I set/clear cells repeatedly. Edit: This seems to also be related to the cells only actually being added/removed on the next update. And it's only a minor issue.

I'll try to create a pull request later today.

aspersum commented 1 year ago

I'm running into the same issue right now. Clearing the cache is not really a good workaround for my use case, as my scenes need to remember their state and this resets them completely.

I'm sorry to nag, but is this planned for 4.1 ? This seems like a pretty major oversight.

TheKassaK commented 1 year ago

It is not fully fixed on 4.2.

Now the instance is removed when we EraseCell(), but we cant add another tile on the same position.

KoBeWi commented 1 year ago

@TheKassaK Can't reproduce. Please open a new issue with a minimal reproduction project.

TheKassaK commented 1 year ago

@TheKassaK Can't reproduce. Please open a new issue with a minimal reproduction project.

Hey @KoBeWi , the issue is here : https://github.com/godotengine/godot/issues/82597

Linkspeedrunner commented 2 months ago

When I use "erase cell" it does not erase any tiles, but I can place tiles and clean the layer