godotengine / godot

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

TileMap getters for cell `TileData` ignore runtime changes stored in `runtime_tile_data_cache` #89503

Open Atlowell opened 8 months ago

Atlowell commented 8 months ago

Tested versions

4.2.1.stable

System information

Windows 10

Issue description

If you overwrite a tilemaps's navigation data, for example using the _tile_data_runtime_update() function, you can remove navigation from a tilemap tile. This works fine when using the Tilemap navigation. However, when you attempt to use the NavigationRegion2D (linked to the tilemap) and programmatically bake the navigation polygon using bake_navigation_polygon(), these changes are not accounted for.

Steps to reproduce

  1. Create a simple 2d scene with a TileMap and a NavigationRegion2D set to retrieve navigation information from that TileMap.
  2. Attach a script to the TileMap and write the following (as an example):
func _use_tile_data_runtime_update(layer, coords):
    if layer == 0 and coords.x == 1:
        return true
    return false

func _tile_data_runtime_update(layer, coords, tile_data):
    tile_data.set_navigation_polygon(0, null)
    return true
  1. Attach a script to your Root node and write the following:
func _ready():
    $TileMap.notify_runtime_tile_data_update(0)
    $NavigationRegion2D.call_deferred("bake_navigation_polygon")
  1. From the Debug Menu, enable "Visible Navigation"
  2. Run the project. You will see both the tilemap's nav mesh (with column 1 excluded, as expected) and the NavigationRegion2D's nav mesh (without column 1 excluded).

Minimal reproduction project (MRP)

Test Navigation.zip

Atlowell commented 8 months ago

This also seems to happen if you add collision polygons to a TileMap during runtime.

smix8 commented 7 months ago

The baking just reads the TileMap used cells and cell tile data.

So this is a matter of the TileMap / TileSet not returning the expected.

EDIT Looks like this runtime tile data update is added to a special cache only used and forwarded to server updates.

The TileMap getters do not consider this runtime cache so they return the original tile state without it.

This obviously makes the parsing and navmesh baking unable to account for it when not even the TileMap own getters do it.

Atlowell commented 7 months ago

Perhaps then the issue should be re-categorized as a TileMap issue.

Atlowell commented 7 months ago

This is unfortunately not as simple as just returning the runtime_tile_data_cache value if it is available. It seems that this cache is wiped every time _internal_update() is called, and a new copy of tile data is created from the TileSetAtlasSource every time use_tile_data_runtime_update returns true.

A solution to this would thus involve the following:

  1. Update the runtime_tile_data_cache to be a persistent value that stores updated tile data, rather than a transient one that is discarded after its changes are accounted for internally
  2. Update get_cell_tile_data() to grab from the runtime_tile_data_cache first, and default to the tileset if no value is present
  3. Update other getters as necessary, though my suspicion is this isn't needed. get_cell_alternative_tile(), get_cell_atlas_coords(), and get_cell_source_id() already seem to account for tile_data_runtime_update.

Some sample code for 2 follows, though I'm not sure how to accommodate for the p_use_proxies parameter, since that seems to be specific to TileSet.

if (E && E->value.runtime_tile_data_cache) {
    return E->value.runtime_tile_data_cache;
} else {
    // existing code gets put in this else function
}