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.

Sign up for GitHub

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

Jump to bottom

fix(OseCharacterCreator): #424 - don't print on close, don't print Failure() on rolls #448

Merged
merged 1 commit into from
May 30, 2023
Merged

fix(OseCharacterCreator): #424 - don't print on close, don't print Failure() on rolls #448

merged 1 commit into from
May 30, 2023

Conversation

Stew-rt
Copy link
Contributor

@Stew-rt Stew-rt commented May 29, 2023 •
edited
Loading

fix for issue #424

  • sets FormApplication option, submitOnClose to false.
  • migrated Gather scores to _onSubmit override
  • Added default to data.roll.type check in helpers-dice.js, setting isSuccess AND isFailure to false.
  • set data object in call to OseDice.Roll to not provide type Object.

I'd quite like to re-work some of this more. But this resolves the reported issue.

Would like to tidy a bit and add two features:

  1. Count re-rolls for manually rolled stats (and present totals in final form)
  2. Provide a system option for different rolling standards, such as point buy and roll 4d6, deduct the lowest.

@Stew-rt
For issue vttred#445
e19689f 
* sets FormApplication option, submitOnClose to false.
* migrated Gather scores to _onSubmit override
* Added default to data.roll.type check in helpers-dice.js, setting isSuccess AND isFailure to false.
* set data object in call to OseDice.Roll to not provide type Object.
@Stew-rt Stew-rt changed the title For issue #445 fix(OseCharacterCreator): #445 - don't print on close, don't print Failure() on rolls May 29, 2023
@Stew-rt Stew-rt changed the title fix(OseCharacterCreator): #445 - don't print on close, don't print Failure() on rolls fix(OseCharacterCreator): #424 - don't print on close, don't print Failure() on rolls May 29, 2023
@anthonyronda
Copy link
Member

Thanks for your patience, I'll be beginning your review now

As for the point buy and drop lowest, once we have more modularity to support that in UFT, that will be the place to do it. Feel free to ask in the discord server what our plans are with that.

anthonyronda
anthonyronda approved these changes May 29, 2023
View reviewed changes
Copy link
Member

@anthonyronda anthonyronda left a comment

Choose a reason for hiding this comment

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

Very clean first PR! I have no notes, everything appears to be in working order

LGTM

@anthonyronda anthonyronda merged commit d14e45e into vttred:main May 30, 2023
@anthonyronda
Copy link
Member

@all-contributors please add @Stew-rt for code 🎉

@allcontributors allcontributors bot mentioned this pull request May 30, 2023
Merged
@allcontributors
Copy link
Contributor

@anthonyronda

I've put up a pull request to add @Stew-rt! 🎉

@anthonyronda
Copy link
Member

@Stew-rt please look at #59 and provide your preferred attribution details!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anthonyronda anthonyronda anthonyronda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Stew-rt @anthonyronda