Skip to content

Add overlay to TileMapLayerEditor specifically for missing tile warning textures - #123779

Open
radioactivejackal1414 wants to merge 1 commit into
godotengine:masterfrom
radioactivejackal1414:tile-warning-textures
Open

radioactivejackal1414 wants to merge 1 commit into
godotengine:masterfrom
radioactivejackal1414:tile-warning-textures

Conversation

@radioactivejackal1414

Copy link
Copy Markdown
Contributor

What problem(s) does this PR solve?

I added a new Control called tile_warning_overlay which is a child of custom_overlay, so that the warnings have their own overlay which is always set to a Linear filter.

Additional information

Related: #123692

@gabcont

gabcont commented Sep 28, 2026

Copy link
Copy Markdown

I just tested your solution and it works as expected, the split of custom_overlay control node into two specific nodes is, at least for me, the right move.

However there are some issues I have:

  1. Why is tile_warning_overlay a child of custom_overlay? That hierarchy does not makes sense to me, I would think they are siblings, which I think is a better way of structuring this.
  2. There is some duplication of code at the start of both overlays' _draw functions, it is not much but that is called every frame. However, I dont see a way to optimize it.
  3. I think custom_overlay is opaque naming that hurts readibiity of this file. I would suggest preview_overlay, although it also handles grid and other visuals, so maybe tile_placing_overlay or similar, as it really only handles visual elements for previewing tiles and their position.

Overall it looks fine to me, although I strongly suggest the renaming.

@radioactivejackal1414
radioactivejackal1414 force-pushed the tile-warning-textures branch 2 times, most recently from 108e83f to d106459 Compare September 29, 2026 23:30
@radioactivejackal1414

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing this @gabcont !

I've updated this based on your feedback:

  • tile_warning_overlay is now siblings with custom_overlay.
  • I agree that the naming of custom_overlay is not ideal. Since I'm splitting it into 2 controls I might as well give it a better name. I went with tile_grid_overlay since it's used for more than just previews.
  • I don't see a simple solution to avoid the duplicated code (but it bothered me too). I wanted the simplest change and the duplicated code felt like a small inconvenience.
  • I also added a block (if (!tile_warning_overlay) {) that matches the logic for custom_overlay, which I was missing before.
  • I added TEXTURE_REPEAT_ENABLED since it should be there too. The repeating pattern wasn't smooth before:

Before: (TEXTURE_REPEAT_DISABLED)
without_repeat
After: (TEXTURE_REPEAT_ENABLED)
with_repeat

Comment thread modules/tilemap/editor/tile_map_layer_editor.cpp
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.

Missing tile warnings in TileMap dock display with wrong texture filter

3 participants