Fix logic for Kak Granny Buy Blue Potion

[fenhl]

The logic previously required actually having obtained the trade quest item even if that item comes later in the sequence than the odd mushroom. The actual in-game flag is also set if adult trade shuffle is off and the starting trade item is the odd potion or later.

See discussion in #dev-bug-reports starting here for context.

Now includes a fix contributed by @mracsys to handle adult trade items being plando'd.

[mracsys]

I have the plando fix I discussed implemented. Using that and the spoiler from the seed that prompted the bug report, I get an unbeatable world with this logic change during plando fill validation:

Kak Granny Buy Blue Potion in world 1 is not reachable without Bomb Bag in world 1!
Traceback (most recent call last):
  File "/home/mracsys/git/OoT-Randomizer-Fork/OoTRandomizer.py", line 58, in start
    main(settings)
  File "/home/mracsys/git/OoT-Randomizer-Fork/Main.py", line 46, in main
    spoiler = generate(settings)
  File "/home/mracsys/git/OoT-Randomizer-Fork/Main.py", line 115, in generate
    place_items(worlds)
  File "/home/mracsys/git/OoT-Randomizer-Fork/Main.py", line 172, in place_items
    distribute_items_restrictive(worlds)
  File "/home/mracsys/git/OoT-Randomizer-Fork/Fill.py", line 85, in distribute_items_restrictive
    worlds[0].settings.distribution.fill(worlds, [shop_locations, song_locations, fill_locations], [shopitempool, dungeon_items, songitempool, progitempool, prioitempool, restitempool])
  File "/home/mracsys/git/OoT-Randomizer-Fork/Plandomizer.py", line 1221, in fill
    world_dist.fill(worlds, location_pools, item_pools)
  File "/home/mracsys/git/OoT-Randomizer-Fork/Plandomizer.py", line 927, in fill
    raise FillError('%s in world %d is not reachable without %s in world %d!' % (location.name, self.id + 1, item.name, player_id + 1))
Fill.FillError: Kak Granny Buy Blue Potion in world 1 is not reachable without Bomb Bag in world 1!

[mracsys]

Search is a pain to debug with the move to solver IDs instead of item names. It'll take a bit for me to tease out.

[mracsys]

This logic fix is not working for plando because the rule parser is pre-calculating world attributes. This will also fail without plando as the parser runs before the item pool is generated and this selected_adult_trade_item is updated from an empty string.

It's not straightforward to remove that parser optimization. I think the right path is to move the check to the world constructor instead of in Item Pool. Plando checks could run at the same time instead of during fill.

Unparsed rule string for Kak Granny Buy Blue Potion for reference: "state.has_all_of((Progressive_Wallet,)) and age == 'adult' and state.has_any_of((Odd_Potion_Access,))"

[fenhl]

@mracsys how does this look?

[mracsys]

Verified that these changes work with the bug report spoiler. Still pending checks for earlier trade sequence item and full trade shuffle.

Spoiler for the bug report seed shows the trade item no longer required as expected:

"Path to Volvagia":     {
        "Sheik at Colossus":           "Zeldas Lullaby",
        "Deku Tree Map Chest":         "Progressive Wallet",
        "Kak Granny Buy Blue Potion":  "Bomb Bag",
        "Colossus Great Fairy Reward": "Megaton Hammer"
      }

Rule string ends up as "state.has_all_of((Progressive_Wallet,)) and age == 'adult'"

[mracsys]

To confirm, any of the following result in the following rule as expected: "state.has_all_of((Progressive_Wallet,)) and age == 'adult' and state.has_any_of((Odd_Potion_Access,))"

• Possible trade sequence items are up to and including Odd Mushroom
• Plando'd item is up to and including Odd Mushroom
• Full trade shuffle is enabled

This is testing on my branch with the above review comments incorporated.

[mracsys]

For reproducibility: OoTR_1517552_UZ14LWEPET_Spoilers.txt

[mracsys]

The following scenarios were also verified by plando'ing Light Arrows on Kak Granny Buy Blue Potion:
granny_logic_check.txt

• Full trade shuffle, all items through Odd Potion are unshuffled. Playthrough correctly collects Pocket Egg from Anju and works through the trade sequence
• Full trade shuffle, Cojiro is shuffled and Odd Mushroom is unshuffled. Cojiro is collected and WOTH, and unshuffled Pocket Egg is ignored.
• Full trade shuffle, Odd Mushroom is shuffled. Odd Mushroom is collected and WOTH.
• Single item trade shuffle, no items selected to be candidate items. Vanilla trade sequence up to Granny is collected.
• Single item trade shuffle, only items up to Odd Mushroom permitted (verifying opposite case of the bug report spoiler). Cojiro was selected, collected, and is WOTH as expected.

Seems good to go.

[r0bd0g]

Logic Review

You should confirm this! (edit: Kirox did), but I can't think of any reason why the OoT Devs would not have left it possible to buy the blue potion as child? The Odd Potion Access event already confirms adult for actually showing the trade item, so probably the is_adult check needs to be removed from the logic for the blue potion check. This could come up in S7 if Interiors are shuffled.

[cjohnson57]

Thanks to everyone for their great work on this! Especially in such a short timespan, you guys rock

[r0bd0g]

This PR still has an issue per my previous comment.

[fenhl]

#2133

[cjohnson57]

This PR still has an issue per my previous comment.

Ah, I incorrectly thought the edit meant there was no issue

[r0bd0g]

Oh I see. No I just meant that Kirox confirmed the check is purchasable as child.