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

Remove almost all remaining dependencies of TileMapLayer on TileMap #88966

Conversation

groud
Copy link
Member

@groud groud commented Feb 28, 2024

This is probably the last step before we can expose TileMap layers as individual nodes, as this makes TileMapLayer self-sufficient. Instead of TileMap fetching data from its parent TileMap node, the TileMap nodes sets properties on its children layers.

The TileMap has however to call set_as_tile_map_internal_node to adjust the behavior of the layer. It makes some callbacks (navigation and physics mainly) point towards the TileMap node as a source of the interaction (collision) instead of the layer node itself. It also automatically instantiate a navigation map for layers with index >= 1, and sets a few needed properties. Basically it allows keeping compatibility.

This PR should keep full compatibility (and fix self_modulate on TileMap not working, while we are at it).
Bugsquad edit: Fixes #31413
Edit: supersedes #75320

@groud groud added this to the 4.3 milestone Feb 28, 2024
@groud groud requested a review from a team as a code owner February 28, 2024 16:30
@groud groud force-pushed the remove_tilemap_layer_dependencies_to_tilemap branch 2 times, most recently from fddc51a to 54b0f95 Compare February 28, 2024 16:39
scene/2d/tile_map_layer.h Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
@groud groud force-pushed the remove_tilemap_layer_dependencies_to_tilemap branch 2 times, most recently from 985cf64 to 34825ad Compare February 29, 2024 09:58
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
@groud groud force-pushed the remove_tilemap_layer_dependencies_to_tilemap branch from 34825ad to c17e16a Compare February 29, 2024 10:59
@groud
Copy link
Member Author

groud commented Feb 29, 2024

Bugsquad edit: Fixes #31413

Regarding this bugsquad edit, this was a 3.x bug report. Not sure it's fixed there.

@groud groud force-pushed the remove_tilemap_layer_dependencies_to_tilemap branch from c17e16a to 787c784 Compare February 29, 2024 11:23
@KoBeWi
Copy link
Member

KoBeWi commented Feb 29, 2024

It's just old, the issue is still relevant in Godot 4.

@groud
Copy link
Member Author

groud commented Feb 29, 2024

It's just old, the issue is still relevant in Godot 4.

That makes sense, but the question was more "Is it still relevant in 3.x"

@KoBeWi
Copy link
Member

KoBeWi commented Feb 29, 2024

Well, the issue will still exist in 3.x and would require a separate fix. But it's not critical.

@akien-mga akien-mga merged commit 8328d9a into godotengine:master Feb 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

In TileMap, when you change the (self_modulate) property, the color of the tile map does not change.
4 participants