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

Holding shift with the eraser erases from all layers #2897

Merged
merged 4 commits into from
Sep 29, 2020

Conversation

aganm
Copy link
Contributor

@aganm aganm commented Sep 23, 2020

To implement shift erase, I had to remove the check for Qt::NoModifier so I don't know if that's gonna collide with another command.

@aganm aganm mentioned this pull request Sep 23, 2020
aganm and others added 3 commits September 25, 2020 15:48
Not really performance sensitive in general, but I didn't like the
O(M*N) complexity of the previous solution.
Previously you had to make sure to hold Shift when starting to erase.

Also made it so that holding other modifiers like Alt and Ctrl don't
prevent the Shift modifiers from working.
@bjorn
Copy link
Member

bjorn commented Sep 29, 2020

@aganm Thanks for this improvement! I made a few tweaks. The behavior changed so that other modifiers are now ignored and that you can toggle the strong mode also while erasing. Let me know if it all looks fine, if so we can merge this.

@aganm
Copy link
Contributor Author

aganm commented Sep 29, 2020

Looks good to me!

@bjorn bjorn merged commit a75e8b9 into mapeditor:master Sep 29, 2020
@bjorn
Copy link
Member

bjorn commented Sep 30, 2020

I found the reason for the NoModifier check: 153405d

It means, we should at least check that Ctrl is not being held, otherwise people trying to use this action will erase instead. :-(

@aganm aganm deleted the shift-erase branch September 30, 2020 14:40
@aganm
Copy link
Contributor Author

aganm commented Oct 1, 2020

That should fix it #2904

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

Successfully merging this pull request may close these issues.

None yet

2 participants