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 Coin support in _simplifyDiceTerms #2493

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Conversation

Hoppyhob
Copy link
Contributor

Closes #2281
There was an issue in _simplifyDiceTerms where a Coin was reduced to its operator and number of faces, then assumed to be a die when passed back out. This ended up turning a coin into a die 2 and combining it with any other die 2s in the Dice Terms.
This was fixed by using a c for the faces value if the DiceTerm is Coin and returning a DiceTerm of Coin, instead of Die, if the face value is c.
(Any DiceTerm other than Coin should default to using the number of faces and return as a Die DiceTerm)

Hoppyhob and others added 2 commits October 15, 2023 13:57
Closes foundryvtt#2281
There was an issue in _simplifyDiceTerms where a Coin was reduced to its operator and number of faces, then assumed to be a die when passed back out. This ended up turning a coin into a die 2 and combining it with any other die 2s in the Dice Terms.
This was fixed by using a c for the faces value if the DiceTerm is Coin and returning a DiceTerm of Coin, instead of Die, if the face value is c.
(Any DiceTerm other than Coin should default to using the number of faces and return as a Die DiceTerm)
Copy link
Contributor

@Fyorl Fyorl left a comment

Choose a reason for hiding this comment

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

Nice fix, thanks

@Fyorl Fyorl added this to the D&D5E 2.4.0 milestone Oct 19, 2023
@Fyorl Fyorl merged commit b75c657 into foundryvtt:2.4.x Oct 19, 2023
@Hoppyhob Hoppyhob deleted the 2.4.x_#2281 branch February 3, 2024 22:00
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