Skip to content

Added Fix Invalid Cell and Clear All action to GridMap #37136

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simpuid
Copy link
Contributor

@simpuid simpuid commented Mar 18, 2020

Added option to fix invalid cells in GridMap just like TileMap.
Also added an option to Clear all cells of GridMap, this fixes/resolve #37124
Both actions are working properly, even with undo-redo.

@akien-mga
Copy link
Member

What if you want to swap the MeshLibrary to another, compatible one (i.e. different meshes but with the same logical use at the same IDs)? I do this a lot with TileMaps (see e.g. https://github.com/KOBUGE-Games/jetpaca where different TileSets use the exact same layout).

@simpuid
Copy link
Contributor Author

simpuid commented Mar 18, 2020

Then that would be a problem. Maybe we can clear the internal data only when mesh library is empty?

@Alastor01
Copy link

What if you want to swap the MeshLibrary to another, compatible one (i.e. different meshes but with the same logical use at the same IDs)? I do this a lot with TileMaps (see e.g. https://github.com/KOBUGE-Games/jetpaca where different TileSets use the exact same layout).

Would it be possible to have two options? As there can be two scenarios:

A) Updating to a compatible library, like you said

B) Changing library to a different one with incompatibles IDs - in this case not removing old references would make a mess of a map (an example would be having library with new tiles or missing ones)

…dMap

Changes made:
* Added condition to `GridMap::set_mesh_library` to update only if new library is different from old one
* Added `GridMap::fix_invalid_cells` taking inspiration from `TileMap::fix_invalide_tiles`
* Added menu entry to clear cells in `GridMapEditor`
* Added menu entry to fix tiles in `GridMapEditor`
@simpuid
Copy link
Contributor Author

simpuid commented Mar 19, 2020

@Alastor01 that would cause problem when user will undo the mesh library change. The cell data will remain empty even after undoing it.
One of the solution is to adapt it like TileMap :
TileMap has function to fix invalid tiles.
gridmap
I have implemented the same functionality and also added an option to clear all cells.
gridmap_new
Things are working properly even undo-redo.
@akien-mga what are your opinions? Should I open a new PR for that or force push to this one?

@Alastor01
Copy link

@simpuid That's interesting, haven't used 2D part of the engine for ages.

This looks great, thank you!

@simpuid simpuid changed the title GridMap clears it's internal data when mesh library changes Added Fix Invalid Cell and Clear All action to GridMap Mar 21, 2020
@akien-mga
Copy link
Member

Sorry for the delay reviewing this. The feature seems to make sense, it should maybe be synced with how this is handled in the TileMap editor in 4.0 to be consistent (which is also able to display invalid cells in the viewport).

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.

GridMap Mesh Library "Clear" doesn't actually clear the grid-map
5 participants