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

Changed TileMap::set_cell alternative_tile default value to 0 #58659

Conversation

IgorKordiukiewicz
Copy link
Contributor

@IgorKordiukiewicz IgorKordiukiewicz commented Mar 1, 2022

@IgorKordiukiewicz IgorKordiukiewicz requested review from a team as code owners March 1, 2022 18:39
@akien-mga akien-mga added this to the 4.0 milestone Mar 1, 2022
@akien-mga akien-mga requested a review from groud March 1, 2022 18:43
@vnen
Copy link
Member

vnen commented Mar 1, 2022

You need to change the default value also where the method is bound:

ClassDB::bind_method(D_METHOD("set_cell", "layer", "coords", "source_id", "atlas_coords", "alternative_tile"), &TileMap::set_cell, DEFVAL(TileSet::INVALID_SOURCE), DEFVAL(TileSetSource::INVALID_ATLAS_COORDS), DEFVAL(TileSetSource::INVALID_TILE_ALTERNATIVE));

@IgorKordiukiewicz IgorKordiukiewicz force-pushed the set_tile_alternative_tile_default_param branch from 85a777d to a387bc3 Compare March 1, 2022 20:10
@IgorKordiukiewicz
Copy link
Contributor Author

Updated

<description>
Sets the tile indentifiers for the cell on layer [code]layer[/code] at coordinates [code]coords[/code]. Each tile of the [TileSet] is identified using three parts:
- The source identifier [code]source_id[/code] identifies a [TileSetSource] identifier. See [method TileSet.set_source_id],
- The atlas coordinates identifier [code]atlas_coords[/code] identifies a tile coordinates in the atlas (if the source is a [TileSetAtlasSource]. For [TileSetScenesCollectionSource] it should be 0),
- The alternative tile identifier [code]alternative_tile[/code] identifies a tile alternative the source is a [TileSetAtlasSource], and the scene for a [TileSetScenesCollectionSource].
- The alternative tile identifier [code]alternative_tile[/code] identifies a tile alternative the source is a [TileSetAtlasSource], and the scene for a [TileSetScenesCollectionSource]. Passing -1 will clear the tile.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this precision should be here. You have to set all three args to -1, Vector2i(-1,-1) and -1 to clear a tile, not only the last one.

@groud
Copy link
Member

groud commented Mar 2, 2022

I am not strictly against the change, but it annoys me that you would have to call set_cell(coords, -1, Vector2i(-1,-1), -1) instead of set_cell(coords) to erase a tile.

If we want to change that argument default value we should likely add an erase_cell(coords) method to keep erasing a cell simple.

@IgorKordiukiewicz IgorKordiukiewicz force-pushed the set_tile_alternative_tile_default_param branch from a387bc3 to d3b03ef Compare March 2, 2022 10:07
@IgorKordiukiewicz
Copy link
Contributor Author

Updated

@IgorKordiukiewicz IgorKordiukiewicz force-pushed the set_tile_alternative_tile_default_param branch from d3b03ef to 466644f Compare March 2, 2022 10:17
Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

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

LGTM.

doc/classes/TileMap.xml Outdated Show resolved Hide resolved
@IgorKordiukiewicz IgorKordiukiewicz force-pushed the set_tile_alternative_tile_default_param branch from 466644f to 8f49150 Compare March 2, 2022 11:14
@akien-mga akien-mga merged commit e6f00a0 into godotengine:master Mar 2, 2022
@akien-mga
Copy link
Member

Thanks!

@IgorKordiukiewicz IgorKordiukiewicz deleted the set_tile_alternative_tile_default_param branch March 3, 2022 22:15
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.

New TileSet editor: set_tile only writes NULL
4 participants