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

TileMap.get_used_cells_by_id() should use index instead of id #814

Closed
tylerecouture opened this issue May 10, 2020 · 3 comments · Fixed by godotengine/godot#38635
Closed
Milestone

Comments

@tylerecouture
Copy link

Describe the project you are working on:
NA

Describe the problem or limitation you are having in your project:
Confusing method in TileMap node

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
The TileMap method get_used_cells_by_id(int id) is confusing because nowhere is it explained what id is. It should probably be get_used_cells_by_index(int index) to be consistent with the rest of the TileMap node. Docs for this method should clarify that too.

For example, the related method get_used_cells() DOES refer to index, not id (whatever that is)

Array get_used_cells() const

Returns a Vector2 array with the positions of all cells containing a tile from the tileset (i.e. a tile index different from -1).

index is also referred to in the setcell() and getcell() methods, as opposed to id

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Self explanatory

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Yes, just update docs to explain what id is reffering to

Is there a reason why this should be core and not an add-on in the asset library?:
Already core.

@tylerecouture
Copy link
Author

Thanks! Can the current 3.2.1 docs be updated for current users? https://docs.godotengine.org/en/stable/classes/class_tilemap.html?highlight=TileMap#class-tilemap-method-get-used-cells-by-id

Just a note that id = index?

tylerecouture added a commit to tylerecouture/godot-docs that referenced this issue May 10, 2020
Added: Note: The tile ``id`` is referred to as the tile ``index`` in other methods. 
godotengine/godot-proposals#814
@Calinou
Copy link
Member

Calinou commented May 12, 2020

@tylerecouture Done in godotengine/godot#38686.

kappa54m pushed a commit to kappa54m/godot that referenced this issue May 16, 2020
@KoBeWi KoBeWi added this to the 4.0 milestone Jun 7, 2020
@potaito
Copy link

potaito commented Mar 16, 2022

I stumbled upon this by searching for the meaning of "index" and "id". Would it make sense to add to the documentation, that "index" is the index defined in the tilemap editor?

edg1000 pushed a commit to edg1000/https-github.com-godotengine-godot that referenced this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants