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

feature: added support for locking checkboxes #1242

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

dmportella
Copy link
Collaborator

Warning

Hi @kakaroto what do you think about this? I made it as separate setting because i didnt want to have to deal with merging of settings from stored character settings already etc, and i didnt want to create another setting type, so i opted to add the "lock" setting to the bool option types as a reference to another setting that is not displayed separately. see this ticket as reference for the feature - #1241

Important

Code is not optimized yet just wanted to get you to give an eyeball at it

Feature

Padlock for options, adds a padlock to certain settings that tells beyond 20 to ignore the reverting of the settings after an action was taken, example SNEAK ATTACK

Sneak Attack

Some users suggested that they would like to use the sneak attack action and apply cunning strike to it, without having to have the sneak attack option turned on for all attacks.

Padlock

For Cunning strike there is not a padlock next to it that will tell the extension NOT to turn off cunning strike when it is used. this will work for normal attacks and sneak attack action.

Note

Padlock will override the reverting of the setting on all uses including when cunning strike is set by a hotkey

image
image
image

Copy link
Owner

@kakaroto kakaroto left a comment

Choose a reason for hiding this comment

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

I'm loving the feature. We've received often the request to make a way to not reset some options after each use, and we've also got the request to clear the sneak attack after each use... I've never thought of a simple way to do it.. I was leaning towards making those into dropdowns instead of toggles, but I love your approach and I think it works really well!

I'd say, add the "(apply to next roll)" text on the cunning strike label, since that should have been there, and add a lock option for all of the other options that have "apply to next roll" in them.
Would also love to have the lock option added to the rogue-sneak-attack option (but with default: true so we don't change the expected behavior.

@dmportella dmportella changed the title WIP feature: added support for locking checkboxes feature: added support for locking checkboxes Mar 27, 2025
@dmportella
Copy link
Collaborator Author

Hey @kakaroto ,

Thanks so much for the suggestions! I'm really glad you loved the idea - that means a lot. 🙌

I went ahead and made all the changes you suggested. The label for Cunning Strike now includes the "(apply to next roll)" text, as it should’ve from the start. I've also added the lock option to all other settings that include "apply to next roll," just as you requested.

And yes! The rogue-sneak-attack option now has a lock as well, with the default set to true, so the expected behavior remains unchanged.

Let me know if there's anything else you'd like adjusted!

@dmportella
Copy link
Collaborator Author

image

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.

2 participants