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

Add multi select paint and fill to GridMap #101942

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

Conversation

jamesmintram
Copy link
Contributor

@jamesmintram jamesmintram commented Jan 22, 2025

Part of godotengine/godot-proposals#10992

This PR adds support for multi select in the GridMap editor, doing so has a few effects:

  • Painting - a random tile from the selection will be used
  • Fill - area filled with random tiles from selection
  • Clicking - repeatedly clicking the same cell will choose another tile randomly
  • Existing functionality should remain unaffected

In addition to the new functionality I carried out some tidying around naming. I found it hard to follow mesh_palette_library and mesh_palette given one variable name is a subset of the other. I renamed them:

mesh_library and mesh_palette

I also made naming more explicit throughout as it was easy to get mixed up between various "library index" vs "palette index" vs "library id" as they are all ints.

The selection behaviour has been generalized around a "collected" of selected tiles from the palette, which also supports a single item and zero items being selected (ie maintains existing behaviour)

@mk56-spn

This comment was marked as resolved.

@AThousandShips AThousandShips added this to the 4.x milestone Jan 23, 2025
@jamesmintram jamesmintram force-pushed the jamesrm/gridmap-multiselect branch 4 times, most recently from 90be446 to 6ab917f Compare January 23, 2025 23:58
@jamesmintram jamesmintram changed the title [WIP] Add multi select paint and fill to GridMap Add multi select paint and fill to GridMap Jan 24, 2025
@jamesmintram jamesmintram marked this pull request as ready for review January 24, 2025 00:04
@jamesmintram jamesmintram requested a review from a team as a code owner January 24, 2025 00:04
@jamesmintram jamesmintram force-pushed the jamesrm/gridmap-multiselect branch from 6ab917f to 5cda9d1 Compare January 24, 2025 00:09
@jamesmintram
Copy link
Contributor Author

@Nodragem this is reading for review/testing

cc'ing @ryevdokimov as you approved previous commits in this area

@Nodragem
Copy link
Contributor

Nodragem commented Jan 24, 2025

Sounds great! can't wait to have a look at it.

@jamesmintram I had a quick look, it seems that the feature is working as intended on my side, very nice!

However, there are, historically, actions that revert the multi-selection to 1 item being selected. While these "side-effects" were not important before, it can become frustrating now that multi-selection is being used.

I noticed for now that interacting with the below buttons/widgets are cancelling the multi-selection:
image

Would it be easy for you to make sure that multi-selection is preserved after interaction with this widgets/buttons?

@jamesmintram
Copy link
Contributor Author

Sure, thanks for checking that out. I'll get that fixed up today

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.

4 participants