Core: Added new roll_triggers functionality and created two helper functions to support it. #3539
+255
−24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and to decide based off of multiple options through And/Or conditions, and can resolve random-range-min-max options. Current style triggers old functionality remains the same, except they can now resolve random-range-min-max options.
github-actions
bot
added
affects: core
and to decide based off of multiple options through And/Or conditions, and can resolve random-range-min-max options. Current style triggers old functionality remains the same, except they can now resolve random-range-min-max options.
…st], added more recognized forms for and/or (AND, And, and, OR, Or, or). Retested, it works.
|
Since you're updating these, you should also update the triggers guide to include examples of all of these new functions |
Generate.py
Outdated
| comparator: str): | ||
| if comparator == "=": | ||
| return yaml_value == trigger_value | ||
| elif comparator == "!=": |
There was a problem hiding this comment.
There's a return, so this can be an if
Edit: There are other instances of this.
Suggested change
| elif comparator == "!=": | |
| if comparator == "!=": |
Generate.py
Outdated
| return yaml_value == trigger_value | ||
| elif comparator == "!=": | ||
| return yaml_value != trigger_value | ||
| if type(yaml_value) is int and type(trigger_value) is int: |
There was a problem hiding this comment.
You should use isinstance(yaml_value, int) and not type()
Edit: There are other instances of this.
Exempt-Medic
added
the
is: enhancement
…able, type) comparisons, and changed elifs after returns to ifs as suggested. Also added changes to Trigger guide to explain the new options.
…options. This defaults to "=" to maintain previous functionality. Also updated the trigger guide to show this extra functionality, and to clarify the function/use of advanced triggers.
…ic/Archipelago.git into AddedTriggerFunctionality
…ues, and str numbers, such as 4 == '4', as well as bool/string such as false vs 'false'
… exist. Changed to consider such a comparison False, as is the current response.
resolution in triggers are properly recorded.
Vertraic
changed the title
…used options without a name lookup to fail.
…ional, and added handling for entries NOT contained in type_hints (less likely to be used now that and/or and comparators are present.)
|
would 100% use this as a sync host. |
…to AddedTriggerFunctionality
|
just an observation from #ap-core-dev, maybe splitting this PR into multiple PRs would be better in the long run and make it less complicated? I think the math comparison operations might be better suited for a separate PR from the multi-condition functionality. Obviously this is only a suggestion, and feel free to ignore me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this fixing or adding?
Keeps the current trigger functionality, while adding the ability to use comparators (<, >) for numerical values, and to and/or multiple conditions together for more complex triggers. This required adding random-range-min-max resolution to roll_triggers. random/random-range-low/high are not included in this resolution.
If the new option "option_advanced" is not included, triggers will work exactly as before, with the exception of resolving random-range-min-max.
For multiple option comparison, And takes priority, so if you look for options: A & B & C | B & D | A & E & F it will logically group them as: (A & B & C) | (B & D) | (A & E & F)
Format is as follows in the trigger options, replace list dots with - :
option_advanced:
the comparator accepts '<', '=', '!=', '>' and defaults to '=' if only two entries are given.
&/0/and/And/AND all represent an AND operation, while |/1/or/Or/OR both represent and OR operation.
option_name/option_result are un-needed when option_advanced is present.
Example:
triggers:
option_advanced:
options:
A Link to the Past:
swordless: true
This says that if crystals needed for gt is more than 2 and less than 6, and crystals needed for ganon is 4 OR if goal is triforce hunt and triforce pieces mode is set to percentage, then swordless will be set to true.
How was this tested?
Unit tests were run to see if anything outside triggers had broken, and I tried old trigger styles to ensure they still worked, and then tried each new feature separately, and in multiple combinations while checking that the resulting settings were as expected, including making sure random-range-min-max permanently resolved to a constant value for that game's generation.
Finally, I ran a generation with every supported game that A: Does not require you to possess a copy of, or B: That I possess a copy of. including both old and new triggers in two of them.
The generation was successful with no errors, and all the game's settings were as expected, including merged values for start_inventory.
If this makes graphical changes, please attach screenshots.
No graphical changes were made.