-
Notifications
You must be signed in to change notification settings - Fork 671
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
Options: Add an OptionError to specify bad options caused the failure #2343
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, tested and got the same expected output.
Looks like there are merge conflicts that need to be resolved first, however. Once resolved, I will approve.
# Conflicts: # Generate.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ld be thrown via bad input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look fine, tested and got the expected error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good
…ArchipelagoMW#2343) * Options: Add an OptionError to specify bad options caused the failure * inherit from ValueError instead of RuntimeError since this error should be thrown via bad input --------- Co-authored-by: NewSoupVi <[email protected]>
…ArchipelagoMW#2343) * Options: Add an OptionError to specify bad options caused the failure * inherit from ValueError instead of RuntimeError since this error should be thrown via bad input --------- Co-authored-by: NewSoupVi <[email protected]>
What is this fixing or adding?
Users find it hard to tell if a failure is their fault (from random settings) or just the world making a mistake, so gives slightly more accurate cause of the issue
How was this tested?
made bad options and checked that it printed
If this makes graphical changes, please attach screenshots.