Core: Allow Option References and Math in YAML Option Values #4049
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
github-actions
bot
added
affects: core
…weights so that it can pass unittests
…option so that it can pass unittests
…ted functionality.
Exempt-Medic
added
is: enhancement
beauxq
reviewed
Oct 13, 2024
beauxq
reviewed
Oct 13, 2024
beauxq
reviewed
Oct 13, 2024
Comment on lines
+336
to
+340
| BinOp = Literal["+", "-", "*", "/"] | ||
|
|
||
|
|
||
| _bops: Mapping[BinOp, Callable[[Union[float, int], Union[float,int]], Union[float, int]]] = { | ||
| "+": lambda a, b: a + b, |
There was a problem hiding this comment.
The PR author is already aware of this, but this is a note to other reviewers.
This isn't a good place for this math parsing code, but it's not a lot worse than any other place that's available.
It would be good to put it in its own module as a sub-module to Utils #4032
beauxq
reviewed
Oct 13, 2024
beauxq
reviewed
Oct 13, 2024
beauxq
reviewed
Oct 13, 2024
Re-wrote breaking changes to not break, reverted lufia2ac test change, allowed for float math in binops while correcting data type before it is used by from_any.
use a `name_resolver` function
beauxq
reviewed
Oct 15, 2024
Generate.py
Outdated
| return operand_stack[0] | ||
|
|
||
|
|
||
| def get_choice(option, root, value=None, sub_group=None, record: ChoiceRecord = ChoiceRecord(None, None, None)) -> Any: |
There was a problem hiding this comment.
This is a mutable default argument for record
https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments
I recommend you use ruff with the "B" rules enabled. It will point out issues like this to you.
beauxq
reviewed
Oct 15, 2024
Generate.py
Outdated
Comment on lines
414
to
417
| if len(val_str.split(".")) > 1: | ||
| val = float(val_str) | ||
| else: | ||
| val = int(val_str) |
There was a problem hiding this comment.
Suggested change
| if len(val_str.split(".")) > 1: | |
| val = float(val_str) | |
| else: | |
| val = int(val_str) | |
| val = float(val_str) |
can simplify this since it will be turned into int at the end if needed
Also removes if-else from parse_tokens that was no longer necessary as floats are converted to int later on if needed.
Two references to root changed to choice_group, and added if-else to ensure sub_group does not write back to root.
Exempt-Medic
removed
the
waiting-on: world-maintainer
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.
Added functionality to handle math/option references in yamls through additional functions + new class.
What is this fixing or adding?
Adds the ability to reference other option values and perform mathematical operations in yamls by enclosing the option value
with $(...). For example:
option1: random-range-4-15
option2: $(option1 * 2)
option2 will be assigned the value of double whatever option1 rolls.
This does not support random(-range) inside the operation, only by referencing an option using it.
References can stack (option1 points to option2 which points to option3...) so long as no reference loops are created (option1 points to option2 which points to option1) There is no PRACTICAL limit to how many references are made
How was this tested?
Each operator was tested with multiple values, positive and negative.
Option value resolution was tested with multiple references in a single operation ($(option1 * (option2 + option3)) for example)
Testing was performed on various random(-range-) values, and with OoT and Factorio specifically to ensure their special calls still worked.
If this makes graphical changes, please attach screenshots.
No graphical changes