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

Change -h Tool Command Flag to Always Impact Secondary Brush #2560

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Zeranny
Copy link
Contributor

@Zeranny Zeranny commented Jan 31, 2024

Overview

I was asked recently what the -h flag did in various tool util commands as the existing description of "Whether the offhand should be considered or not" is unclear to others and myself.
Upon using it, and from the feedback of others trying to use it too, it felt inconsistent in when it would change the primary or secondary brush.

From reading the code it appears the intent is for tool util commands to change the last used brush when a primary & secondary are set, with the flag inverting which one it impacts.

I think this isn't very intuitive and believe it would make more sense to just have the flag set to change the secondary brush, otherwise the primary.

Description

This changes the -h flag to always change the value for the secondary brush, if one is set. If no flag is used, or no secondary brush is set, then it will just change the primary brush.

I have changed the help page info to make this clear.

Additionally I have added the same flag functionality to the /size / /br size command.

Lastly, some minor tidying of the unused editSession import, and the inconsistent tool/brushTool/bt naming.

Hopefully this change makes sense, I'd be curious to hear if myself and those I spoke to are in the minority when it comes to thinking the existing method is unclear and hard to use.

Submitter Checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter Checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
    Options
  2. Ensure that the pull request title represents the desired changelog entry.
    Options
  3. New public fields and methods are annotated with @since TODO.
    Options
  4. I read and followed the contribution guidelines.
    Options

@Zeranny Zeranny requested a review from a team as a code owner January 31, 2024 20:22
@SirYwell
Copy link
Member

This changes the -h flag to always change the value for the secondary brush, if one is set. If no flag is used, or no secondary brush is set, then it will just change the primary brush.

I think this should rather be a warning, otherwise the user might modify something they didn't want to modify. Would that make sense?

@Zeranny
Copy link
Contributor Author

Zeranny commented Mar 4, 2024

This changes the -h flag to always change the value for the secondary brush, if one is set. If no flag is used, or no secondary brush is set, then it will just change the primary brush.

I think this should rather be a warning, otherwise the user might modify something they didn't want to modify. Would that make sense?

I think adding a message that states wether it was the primary or secondary that the mask, size, or whatever has been applied to could help, but this change instead requires the user to explicitely declare that they want to modify the secondary which removes any ambiguity.

Unless I misunderstood your concern?

@SirYwell
Copy link
Member

I think adding a message that states wether it was the primary or secondary that the mask, size, or whatever has been applied to could help, but this change instead requires the user to explicitely declare that they want to modify the secondary which removes any ambiguity.

Unless I misunderstood your concern?

My concern is that explicitly wanting to change a secondary brush changes the primary brush. But I also see how only preventing that could be confusing. I guess a message clarifying which brush was changed would be enough then.

Copy link

Please take a moment and address the merge conflicts of your pull request. Thanks!

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.

None yet

3 participants