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

Pokémon R/B: Fix incompatible option combination #2356

Merged
merged 3 commits into from
Oct 30, 2023

Conversation

Alchav
Copy link
Contributor

@Alchav Alchav commented Oct 24, 2023

What is this fixing or adding?

You can set fossil_check_item_types to no_key_items but then turn key_items_only on, resulting in no non-key-items to fill the fossil checks. This skips processing the fossil_check_item_types rules at all if key_items_only is on

How was this tested?

Generating with same seed # of a previously failing generation

@ScootyPuffJr1 ScootyPuffJr1 added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Oct 24, 2023
Co-authored-by: Fabian Dill <[email protected]>
Comment on lines 470 to 471
if not self.multiworld.key_items_only[self.player]:
if self.multiworld.fossil_check_item_types[self.player] == "key_items":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if not self.multiworld.key_items_only[self.player]:
if self.multiworld.fossil_check_item_types[self.player] == "key_items":
if not self.multiworld.key_items_only[self.player]:
rule = None
if self.multiworld.fossil_check_item_types[self.player] == "key_items":

Need to establish rule before using it. None isn't necessarily the correct thing to choose here though, I'm just not super sure what is without reviewing the code further.

Copy link
Member

@NewSoupVi NewSoupVi Oct 30, 2023

Choose a reason for hiding this comment

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

Another option would be to actually have an "else:" case, in which case declaring it before becomes unnecessary. Technically, even the current code is only wrong because a case was hit that wasn't covered, although that is exactly the reason why you usually shouldn't do this unless you have an "else:"

(For the record: Block scope is not a thing in Python.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh right, I forgot python is nice like that, letting you have it if every case leads you to setting rule as something

@Berserker66 Berserker66 merged commit aa56383 into ArchipelagoMW:main Oct 30, 2023
12 checks passed
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants