godotengine / godot

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

get_cell_autotile_coord gives the same information if the tile is at the position 0, 0 in the tileset or the cell doesn't have autotilling #39332

Open BrIt-neY opened 4 years ago

BrIt-neY commented 4 years ago

Godot version: 3.2.1.stable.official

OS/device including version: Windows10

Issue description: The GDscript function get_cell_autotile_coord() returns a zero vector when the cell doesn't have autotilling. See documentation: https://docs.godotengine.org/en/stable/classes/class_tilemap.html#class-tilemap-method-get-cell-autotile-coord What the function returns is a vector2 containing the coordinates of the tile in the tileset. But the first tile in the tileset is at the position (0, 0). So it's exactly the same as if the cell doesn't have autotilling. So the function can return misleading information.

Steps to reproduce: Execute the minimal project linked. You'll see that the cell at the coordinates (0, 0) get the same result with the get_cell_autotile_coord function as the cell (4, 0) even if it's an empty cell - so obviously no autotiling.

Minimal reproduction project: Issue_TileMap.zip

I took the liberty to check the tile_map.cpp - if I may add a small contribution to the project. Maybe the constant INVALID_CELL could be returned is both coordinates of the vector2 to handle this case: image

Calamander commented 4 years ago

Actually the problem is that in several places default empty values for autotile coordinates are set to (0,0) and I'm not sure why. Your correction would make function return (-1,-1) if cell is empty, but if there is a tile, which is not autotile - it will still return (0,0).

Anyway it is a bug.

erictuvesson commented 4 years ago

I agree that this can be seen as miss information. At the same time this I don't see a scenario where you would use get_cell_autotile_coord without get_cell, can you think of one?

The documentation does say that it will return a zero vector

Returns the coordinate (subtile column and row) of the autotile variation in the tileset. Returns a zero vector if the cell doesn't have autotiling.

Calamander commented 4 years ago

I agree that this can be seen as miss information. At the same time this I don't see a scenario where you would use get_cell_autotile_coord without get_cell, can you think of one?

For example if you use few tilemaps to separate different types of objects and one of them have single autotile in tileset. So you could use only get_cell_autotile_coord to determine if cell is empty or have tile and which autotile it is. It may not be common use though..

erictuvesson commented 4 years ago

@groud What do you think of this?

groud commented 4 years ago

I don't mind changing it. While returning something like Vector2(INVALID_CELL, INVALID_CELL), does not make much sense, as the vector is not made of two cell IDs, we might change it to return Vector2(-1, -1).

In any case that does not change the fact the user should first check if the tile is an autotile first, but I agree it might be simpler for the user to simply check if the returned result is Vector2(-1, -1).