godotengine / godot

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

Having Multiple TileMaps With Local To Scene Tile Sets Breaks Tile Sets #81959

Open kuroodo opened 11 months ago

kuroodo commented 11 months ago

Godot version

v4.1.1.stable.official [bd6af8e0e]

System information

Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1080 (NVIDIA; 31.0.15.3713) - Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 Threads)

Issue description

If you have more than one tile maps in a scene that share a tile set, and then enable the tile set's "Local to Scene" property, the tile set ends up broken. If you use make unique (recursive) before enabling local to scene, you need to restart the editor every time you add another instance of the tile map in the scene with other tile maps

This issue is also present on 4.2 Dev 5

I have not tested with other resources. This might not be exclusive to tile sets.

Steps to reproduce

For 4.1.1

  1. Open the example project and open main.tscn
  2. Observe that the tile set used for RegularMap in main.tscn, and the one in tile_map.tscn are the same
  3. Select either RegularMap in the Main scene, or the tile map in tile_map.tscn
  4. Select MainTileset.tres in the inspector
  5. In the resource category, enable "Local to Scene" and save
  6. Add tile_map.tscn to the main scene

Notice that RegularMap now appears to be broken. MainTilset.tres has also lost its tiles, and the TileMap scene added to Main is unique and still works.

Continuing:

  1. Delete the project and open a fresh copy
  2. In tile_map.tscn, right click MainTileset.tres in the inspector and select make unique (recursive), then save the scene
  3. Enable local to scene for the tile set and save
  4. Add tile_map.tscn to Main and save
  5. Observe that the tile map is working and you can draw using terrains. RegularMap is undisturbed
  6. Return to tile_map.tscn
  7. Observe that the tile set is broken
  8. Close the editor and reopen the project
  9. Open tile_map.tscn
  10. Observe that the tile set works again
  11. Go to main.tscn
  12. Add another tile_map.tscn to the main scene. This one is fine and you should be able to draw too 13 Return to tile_map.tscn
  13. Observe that the tile set is broken again
  14. Return to main.tscn
  15. Add another tile_map.tscn to the scene
  16. The newly added one is broken. Had you restarted the editor before adding it, it would be fine
  17. Close the editor and reopen the project.

At this point I've sometimes noticed the other tile maps break. You can also try restarting the project multiple times to find that the third tile map that was added is always broken. But the moment you add a fourth one and restart, the third one works again.

Depending on the things you do, it can sometimes lead to loss of the tile set in general, where the only way I've found to restore it is by undoing changes in source control. In the step where you performed "make unique (recursive)", if you then save the resource into your project (right click, save) before continuing, the tileset will likely be broken regardless if you restart the editor

The best way to ensure that things dont break is by restarting the editor every time you add a new instance of tile_map.tscn to the scene

For 4.2:

Since Make Unique (recursive) is not available in 4.2, you cannot follow all the steps exactly. Since make unique recursive is missing, when you make the tile set in tile_map.tscn unique and enable local to scene, it breaks the tile map in main

Minimal reproduction project

TileBug4.1.zip

KoBeWi commented 11 months ago

This is exclusive to TileSets, see #80076 It's the same issue, but indirect.

kuroodo commented 11 months ago

Yeah that seems like thats likely the same issue.

I came across this from porting over an old 3.5 project to 4.x. I have an 'UnlockableTiles' scene with a tile map. When something is triggered, an animation plays which gives the tiles some opacity and changes the collision only for the specific tile map. There can be multiple UnlockableTiles in my level, with each having a different trigger.

In 3.5 this worked fine where I can add an instance of the scene to my level and then draw the autotiles. Since the collision was tied to the TileMap instead of the TileSet, modifying the collision of one didn't affect the other.

But in 4.1.1 the workflow is cumbersome. Since collision is now tied to the tile set, for initial set up I have to make the tile set unique recursive and enable local to scene so that I only modify the collision of a specific instance instead of all. I think I have to make the texture unique too. The tile set cannot be a .tres from the file system no matter what or else it will break every time I enable local to scene. From there, every time I add a new instance of UnlockableTiles to my scene, the first thing I need to do is restart the editor. Failure to restart the editor for each new instance can lead to things breaking, where I would need to either discard changes through source control or add the original tileset and repeat the initial set up again.

KoBeWi commented 11 months ago

Instead of using multiple TileMaps you should use multiple layers and disable collisions for layers using _tile_data_runtime_update().

EDIT: I made a simple example project that I was going to submit as an official demo. It's not exactly what you wanted to do, but it's the same concept. dynamic_tilemap_layers.zip

kuroodo commented 11 months ago

Thank you for the example project. It really helped me understand how the new tile system works and I was able to essentially get a working approach on my project through that.

Putting aside what I was trying to do, the original issue is still valid and could potentially lead to loss of data or the tile set data becoming misconfigured. I'll leave it up to the maintainers if they wish to close this as a duplicate of the other issue or not.

KoBeWi commented 11 months ago

Local to scene has an API for configuring how the local resource is created, so I think we can do better than making the tileset unusable. Even a clear error could work.

ZayLong commented 11 months ago

@KoBeWi But what I want each layer to be offset on the Y axis?

KoBeWi commented 11 months ago

You can change tile origin y position. Note that doing it for the whole TileMap might be rather expensive (runtime updates are better suited for sparse layers). I think for this use-case you can use multiple TileMaps. They don't need unique TileSet.

ZayLong commented 11 months ago

@KoBeWi Thanks, thought so. In your example, why are you using ampersands(&) in all of your inputs? if is_on_floor() and Input.is_action_just_pressed(&"jump"):

KoBeWi commented 11 months ago

"jump" is String, &"jump" is StringName. The input method take StringName, so using it directly avoids conversion. Although it's a constant value, so GDScript might optimize that and there is no difference, idk.