Added TileMap::set_cell_region() #42194
Conversation
Calinou
added
cherrypick:3.x
dariomnz
force-pushed
the
master
branch
2 times, most recently
from
3cd7b29 to
f303d22
Compare
doc/classes/TileMap.xml
Outdated
| <argument index="6" name="autotile_coord" type="Vector2" default="Vector2( 0, 0 )"> | ||
| </argument> | ||
| <description> | ||
| Sets the tiles in the given region. |
There was a problem hiding this comment.
Suggestion to avoid repeating the function name:
Assigns the tile of id tile to all cells in the rectangular region defined by start and end.
| Sets the tiles in the given region. | ||
| An index of [code]-1[/code] clears the region. | ||
| Optionally, the tile can also be flipped, transposed, or given autotile coordinates. The autotile coordinate refers to the column and row of the subtile. | ||
| [b]Note:[/b] Data such as navigation polygons and collision shapes are not immediately updated for performance reasons. |
There was a problem hiding this comment.
Do you know exactly when they update? If on the next physics step for instance, it'd be useful to mention it. The more precise the reference, the better.
There was a problem hiding this comment.
It's the same as the method set_cell but for a region. This method use a call_defered for the update_dirty_quadrants. I think it is, but I'm not pretty sure that this update is in the next physics step.
There was a problem hiding this comment.
When something is deferred, it's always called on the idle frame. Not sure if it's even worth mentioning.
scene/2d/tile_map.cpp
Outdated
| @@ -893,6 +893,34 @@ void TileMap::set_cell(int p_x, int p_y, int p_tile, bool p_flip_x, bool p_flip_ | |||
| used_size_cache_dirty = true; | |||
| } | |||
|
|
|||
| void TileMap::set_cell_region(const Vector2 &p_start, const Vector2 &p_end, int p_tile, bool p_flip_x, bool p_flip_y, bool p_transpose, Vector2 p_autotile_coord) { | |||
There was a problem hiding this comment.
Instead of P_start and p_end, you should use a single Rect2i. See https://docs.godotengine.org/en/latest/classes/class_rect2i.html
There was a problem hiding this comment.
And to simplify the implementation, you could simply use p_rect = p_rect.abs(), as it automatically does what your are doing in the lines after.
|
Great to see another proposal to adress the set_cell performance issues :) Just for the sake of completeness, there are two more pending PR's for this topic.
Back to your PR, this function has the same issue as the previous two = flip and transpose is always the same. |
|
@zzz-assault Yes, the same TILE_ID is intended. This is designed for when you have to fill a tilemap with the same kind of tiles, and then for example create rooms by removing the tiles. You can also make rectangles that have the same tiles to use this method. |
|
Understood, as said this seems to be a very narrow usecase. Allowing flexible tile per set cell within region will cover all usecases (incl. yours). In case of new function I'll prefer the flexible solutions. Also setting a huge number of same tiles as border or background seems to be very costy (performance). If it's only cosmetic background you could simply use a sprite and with region enabled (texture with only this bg tile and repeat activated) and expand region to the size of the tilemap or max map you need. Then add in Tilemap only rooms. This is some trick I used in the past and reduces liading time and required memory by a huge amount. |
|
Given the discussion on godotengine/godot-proposals#1538 and above, it's not clear whether this would actually:
So after discussing with @groud we suggest going back to the drawing board for now and continuing the discussion in godotengine/godot-proposals#1538 to really narrow down on a consensual change that would address a well defined (and benchmarked) problem. |
akien-mga
added
archived
and removed
cherrypick:3.x
Implementation of the new feature that was proposed in godotengine/godot-proposals#1538