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

Core: don't log warnings for plando_items and missing lttp options #3606

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions Generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ def roll_settings(weights: dict, plando_options: PlandoOptions = PlandoOptions.b
if "linked_options" in weights:
weights = roll_linked_options(weights)

valid_keys = set()
valid_keys = {"triggers"}
Copy link
Collaborator

@beauxq beauxq Jul 2, 2024

Choose a reason for hiding this comment

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

I'm worried about side effects from putting "triggers" in this set this early (because I was thinking of just adding it to the set right before the warning for loop near the end of the function).

It might be ok, but we should make sure it's tested and thought through with everything that is done with valid_keys before that warning loop.
(For example valid_keys is passed to roll_triggers, and I'm not sure what roll_triggers does with it.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

valid_keys gets rebuilt per player, and roll_triggers just adds the names that it triggers off of (fake names people use for more complex trigger setups) to the set. it should be fine, but more testing is probably a good idea

if "triggers" in weights:
weights = roll_triggers(weights, weights["triggers"], valid_keys)

Expand Down Expand Up @@ -506,15 +506,22 @@ def roll_settings(weights: dict, plando_options: PlandoOptions = PlandoOptions.b
for option_key, option in world_type.options_dataclass.type_hints.items():
handle_option(ret, game_weights, option_key, option, plando_options)
valid_keys.add(option_key)
for option_key in game_weights:
if option_key in {"triggers", *valid_keys}:
continue
logging.warning(f"{option_key} is not a valid option name for {ret.game} and is not present in triggers.")

# TODO remove plando_items after moving it to the options system
valid_keys.add("plando_items")
if PlandoOptions.items in plando_options:
ret.plando_items = game_weights.get("plando_items", [])
# TODO there are still more LTTP options not on the options system
valid_keys |= {"sprite_pool", "sprite", "random_sprite_on_event"}
if ret.game == "A Link to the Past":
roll_alttp_settings(ret, game_weights)

# log a warning for options within a game section that aren't determined as valid
for option_key in game_weights:
if option_key in valid_keys:
continue
logging.warning(f"{option_key} is not a valid option name for {ret.game} and is not present in triggers.")

return ret


Expand Down
Loading