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

More advantage mode coverage #4890

Open
wants to merge 2 commits into
base: 4.2.x
Choose a base branch
from

Conversation

krbz999
Copy link
Contributor

@krbz999 krbz999 commented Dec 16, 2024

A continuation of #4838.

Modes:

  • Add: Adds one source of advantage (1) or disadvantage (-1).
  • Upgrade: Makes it impossible to have disadvantage (0, 1). Does nothing.
  • Downgrade: Makes it impossible to have advantage (-1, 0). Does nothing.
  • Multiply: Does nothing.
  • Override: Forces one advantage mode regardless of other sources, downgrade, or upgrades. Does nothing.
  • Custom: is custom.

@krbz999 krbz999 mentioned this pull request Dec 16, 2024
@roth-michael
Copy link
Contributor

roth-michael commented Dec 16, 2024

I don't know how I feel about what I'm about to suggest, but might it make sense to do a ui warn notification if someone uses upgrade/downgrade/multiply/override for the time being, informing them that Add is the only functional change mode currently? On the one hand, I kind of hate that. On the other, it would definitely ensure that it gets fixed since it'd pop up every time data prep happens.
Probably not worth it, but I dunno. If nothing else maybe a console.warn

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.

3 participants