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

Inconsistent default values in TileMap.set_cell() and TileMapPattern.set_cell() #80429

Open
sbebular opened this issue Aug 8, 2023 · 5 comments

Comments

@sbebular
Copy link

sbebular commented Aug 8, 2023

Godot version

v4.1.1.stable.official [bd6af8e]

System information

Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Compatibility) - Radeon RX 570 Series (Advanced Micro Devices, Inc.; 31.0.21001.45002) - Intel(R) Core(TM) i3-8100 CPU @ 3.60GHz (4 Threads)

Issue description

TileMap.set_cell() and TileMapPattern.set_cell() have different default values for the alternative_tile parameter. TileMap.set_cell() defaults to 0, allowing a tile to be placed at the given coordinates if no value is provided, while TileMapPattern.set_cell() defaults to -1, which instead erases the cell at the given coordinates.
My expectation was that the behavior of these functions would be consistent, given their similarities and that the documentation for TileMapPattern.set_cell() tells users to refer to TileMap.set_cell(). I am not sure whether the default value for both functions should be 0 or -1, and ask that someone more familiar with TileSets make the decision.

Steps to reproduce

  1. Create a TileMap.
  2. Create a TileSetAtlasSource, add a texture to it, and create a tile.
  3. Add the TileSetAtlasSource to the TileMap.
  4. Create a TileMapPattern.
  5. Use TileMapPattern.set_cell(), using the source_id of the TileSetAtlasSource and the atlas_coords of the created tile. Do not specify an alternative tile.
  6. In the TileMap, set a cell using TileMap.set_cell(). Do not specify an alternative tile.
  7. In the TileMap. set the TileMapPattern at different coordinates using TileMap.set_pattern().
  8. The tile placed using TileMap.set_cell() will appear normally, while the tile(s) placed using TileMap.set_pattern() will not appear.

Minimal reproduction project

N/A

@AThousandShips
Copy link
Member

This is a documentation issue as changing the default value isn't possible without breaking compatibility

@Mickeon
Copy link
Contributor

Mickeon commented Aug 9, 2023

Breaking compatibility when expectations are not being followed seems fairly reasonable. Especially when changing TileMap as a whole is something fairly desired, anyhow:

The class as a whole seems to be under active development and actually quite prone to minor tweaks like these.

@jokoon
Copy link

jokoon commented May 8, 2024

var pattern = TileMapPattern.new()
pattern.set_size(size)
for x in range(size.x):
    for y in range(size.y):
        pattern.set_cell(Vector2i(x,y), 0, Vector2i(5,0))

I am confused, I have the same problem, but I don't see a fix.

In your example, how/where do you "get" the TileSetAtlasSource?

I just specify 0 in set_cell everywhere.

@jokoon
Copy link

jokoon commented May 9, 2024

My test case:

pattern

extends TileMap

func _ready():
    var grid_size = Vector2i(8,8)
    # initializing the grid cells
    for a in range(grid_size.x):
        for b in range(grid_size.y):
            #erase_cell(layer_fg,Vector2i(a,b))
            set_cell(0,Vector2i(a,b),0,Vector2i(1,0))

    #var pat = []
    var size = Vector2i(2,3)
    var pattern = TileMapPattern.new()
    pattern.set_size(size)
    for x in range(size.x):
        for y in range(size.y):
            pattern.set_cell(Vector2i(x,y), 0, Vector2i(7,0))
            #pat.append([a,b])
    #pattern_map[size] = pattern
    #pattern_map[size] = tile_set.add_pattern(pattern)
    #pattern_map[size] = tile_set.get_pattern(layer_fg, pat)
    set_pattern(0, Vector2i(1,1),pattern)
    set_cell(0, Vector2i(3,7), 0, Vector2i(4,0))
    set_cell(0, Vector2i(5,7), 0, Vector2i(5,0))
    set_cell(0, Vector2i(6,7), 0, Vector2i(6,0))
    set_cell(0, Vector2i(7,7), 0, Vector2i(7,0))

My code might have a problem, the issue seems to suggests there is a fix, but I don't see it.

@jokoon
Copy link

jokoon commented May 9, 2024

Ok, I understand, so to clarify, alternative_tile must be specified like so:
pattern.set_cell(Vector2i(x,y), 0, Vector2i(7,0), 0)

The documentation should have a big warning in red, recommending users to set it to 0 in gdscript.

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

No branches or pull requests

4 participants