LttP: fix that collect can bypass requirements for ganon ped goal (#1771)

LttP: more pep8

Utils.py

@@ -39,7 +39,7 @@ class Version(typing.NamedTuple):
     build: int
 
 
-__version__ = "0.4.0"
+__version__ = "0.4.1"
 version_tuple = tuplize_version(__version__)
 
 is_linux = sys.platform.startswith("linux")

worlds/alttp/Bosses.py

@@ -6,6 +6,7 @@ from Fill import FillError
 from .Options import LTTPBosses as Bosses
 from .StateHelpers import can_shoot_arrows, can_extend_magic, can_get_good_bee, has_sword, has_beam_sword, has_melee_weapon, has_fire_source
 
+
 def BossFactory(boss: str, player: int) -> Optional[Boss]:
     if boss in boss_table:
         enemizer_name, defeat_rule = boss_table[boss]

worlds/alttp/Client.py

@@ -10,7 +10,7 @@ import Utils
 from NetUtils import ClientStatus, color
 from worlds.AutoSNIClient import SNIClient
 
-from worlds.alttp import Shops, Regions
+from . import Shops, Regions
 from .Rom import ROM_PLAYER_LIMIT
 
 snes_logger = logging.getLogger("SNES")
@@ -270,7 +270,8 @@ location_table_uw = {"Blind's Hideout - Top": (0x11d, 0x10),
                      'Ganons Tower - Pre-Moldorm Chest': (0x3d, 0x40),
                      'Ganons Tower - Validation Chest': (0x4d, 0x10)}
 
-boss_locations = {Regions.lookup_name_to_id[name] for name in {'Eastern Palace - Boss',
+collect_ignore_locations = {Regions.lookup_name_to_id[name] for name in {
+    'Eastern Palace - Boss',
     'Desert Palace - Boss',
     'Tower of Hera - Boss',
     'Palace of Darkness - Boss',
@@ -280,7 +281,9 @@ boss_locations = {Regions.lookup_name_to_id[name] for name in {'Eastern Palace -
     'Ice Palace - Boss',
     'Misery Mire - Boss',
     'Turtle Rock - Boss',
-                                                               'Sahasrahla'}}
+    'Sahasrahla',
+    'Master Sword Pedestal',  # can circumvent ganon pedestal's goal's pendant collection
+}}
 
 location_table_uw_id = {Regions.lookup_name_to_id[name]: data for name, data in location_table_uw.items()}
 
@@ -322,8 +325,15 @@ location_table_misc = {'Bottle Merchant': (0x3c9, 0x2),
 location_table_misc_id = {Regions.lookup_name_to_id[name]: data for name, data in location_table_misc.items()}
 
 
+def should_collect(ctx, location_id: int) -> bool:
+    return ctx.allow_collect and location_id not in collect_ignore_locations and location_id in ctx.checked_locations \
+            and location_id not in ctx.locations_checked and location_id in ctx.locations_info \
+            and ctx.locations_info[location_id].player != ctx.slot
+
+
 async def track_locations(ctx, roomid, roomdata) -> bool:
     from SNIClient import snes_read, snes_buffered_write, snes_flush_writes
+    location_id: int
     new_locations = []
 
     def new_check(location_id):
@@ -340,11 +350,10 @@ async def track_locations(ctx, roomid, roomdata) -> bool:
         shop_data_changed = False
         shop_data = list(shop_data)
         for cnt, b in enumerate(shop_data):
-            location = Shops.SHOP_ID_START + cnt
+            location_id = Shops.SHOP_ID_START + cnt
-            if int(b) and location not in ctx.locations_checked:
+            if int(b) and location_id not in ctx.locations_checked:
-                new_check(location)
+                new_check(location_id)
-            if ctx.allow_collect and location in ctx.checked_locations and location not in ctx.locations_checked \
+            if should_collect(ctx, location_id):
-                    and location in ctx.locations_info and ctx.locations_info[location].player != ctx.slot:
                 if not int(b):
                     shop_data[cnt] += 1
                     shop_data_changed = True
@@ -371,9 +380,7 @@ async def track_locations(ctx, roomid, roomdata) -> bool:
             uw_unchecked[location_id] = (roomid, mask)
             uw_begin = min(uw_begin, roomid)
             uw_end = max(uw_end, roomid + 1)
-        if ctx.allow_collect and location_id not in boss_locations and location_id in ctx.checked_locations \
+        if should_collect(ctx, location_id):
-                and location_id not in ctx.locations_checked and location_id in ctx.locations_info \
-                and ctx.locations_info[location_id].player != ctx.slot:
             uw_begin = min(uw_begin, roomid)
             uw_end = max(uw_end, roomid + 1)
             uw_checked[location_id] = (roomid, mask)
@@ -404,8 +411,7 @@ async def track_locations(ctx, roomid, roomdata) -> bool:
             ow_unchecked[location_id] = screenid
             ow_begin = min(ow_begin, screenid)
             ow_end = max(ow_end, screenid + 1)
-            if ctx.allow_collect and location_id in ctx.checked_locations and location_id in ctx.locations_info \
+            if should_collect(ctx, location_id):
-                    and ctx.locations_info[location_id].player != ctx.slot:
                 ow_checked[location_id] = screenid
 
     if ow_begin < ow_end:
@@ -428,9 +434,7 @@ async def track_locations(ctx, roomid, roomdata) -> bool:
             for location_id, mask in location_table_npc_id.items():
                 if npc_value & mask != 0 and location_id not in ctx.locations_checked:
                     new_check(location_id)
-                if ctx.allow_collect and location_id not in boss_locations and location_id in ctx.checked_locations \
+                if should_collect(ctx, location_id):
-                        and location_id not in ctx.locations_checked and location_id in ctx.locations_info \
-                        and ctx.locations_info[location_id].player != ctx.slot:
                     npc_value |= mask
                     npc_value_changed = True
             if npc_value_changed:
@@ -446,8 +450,7 @@ async def track_locations(ctx, roomid, roomdata) -> bool:
                 assert (0x3c6 <= offset <= 0x3c9)
                 if misc_data[offset - 0x3c6] & mask != 0 and location_id not in ctx.locations_checked:
                     new_check(location_id)
-                if ctx.allow_collect and location_id in ctx.checked_locations and location_id not in ctx.locations_checked \
+                if should_collect(ctx, location_id):
-                        and location_id in ctx.locations_info and ctx.locations_info[location_id].player != ctx.slot:
                     misc_data_changed = True
                     misc_data[offset - 0x3c6] |= mask
             if misc_data_changed:

worlds/alttp/Dungeons.py

@@ -1,15 +1,17 @@
 import typing
 
 from BaseClasses import Dungeon
-from worlds.alttp.Bosses import BossFactory
 from Fill import fill_restrictive
-from worlds.alttp.Items import ItemFactory
+
-from worlds.alttp.Regions import lookup_boss_drops
+from .Bosses import BossFactory
-from worlds.alttp.Options import smallkey_shuffle
+from .Items import ItemFactory
+from .Regions import lookup_boss_drops
+from .Options import smallkey_shuffle
 
 if typing.TYPE_CHECKING:
     from .SubClasses import ALttPLocation
 
+
 def create_dungeons(world, player):
     def make_dungeon(name, default_boss, dungeon_regions, big_key, small_keys, dungeon_items):
         dungeon = Dungeon(name, dungeon_regions, big_key,

worlds/alttp/EntranceShuffle.py

@@ -1,7 +1,9 @@
 # ToDo: With shuffle_ganon option, prevent gtower from linking to an exit only location through a 2 entrance cave.
 from collections import defaultdict
-from worlds.alttp.OverworldGlitchRules import overworld_glitch_connections
+
-from worlds.alttp.UnderworldGlitchRules import underworld_glitch_connections
+from .OverworldGlitchRules import overworld_glitch_connections
+from .UnderworldGlitchRules import underworld_glitch_connections
+
 
 def link_entrances(world, player):
     connect_two_way(world, 'Links House', 'Links House Exit', player) # unshuffled. For now

worlds/alttp/InvertedRegions.py

@@ -1,6 +1,7 @@
 import collections
-from worlds.alttp.Regions import create_lw_region, create_dw_region, create_cave_region, create_dungeon_region
+
-from worlds.alttp.SubClasses import LTTPRegionType
+from .Regions import create_lw_region, create_dw_region, create_cave_region, create_dungeon_region
+from .SubClasses import LTTPRegionType
 
 
 def create_inverted_regions(world, player):

worlds/alttp/ItemPool.py

@@ -2,14 +2,15 @@ from collections import namedtuple
 import logging
 
 from BaseClasses import ItemClassification
-from worlds.alttp.SubClasses import ALttPLocation, LTTPRegion, LTTPRegionType
-from worlds.alttp.Shops import TakeAny, total_shop_slots, set_up_shops, shuffle_shops, create_dynamic_shop_locations
-from worlds.alttp.Bosses import place_bosses
-from worlds.alttp.Dungeons import get_dungeon_item_pool_player
-from worlds.alttp.EntranceShuffle import connect_entrance
 from Fill import FillError
-from worlds.alttp.Items import ItemFactory, GetBeemizerItem
+
-from worlds.alttp.Options import smallkey_shuffle, compass_shuffle, bigkey_shuffle, map_shuffle, LTTPBosses
+from .SubClasses import ALttPLocation, LTTPRegion, LTTPRegionType
+from .Shops import TakeAny, total_shop_slots, set_up_shops, shuffle_shops, create_dynamic_shop_locations
+from .Bosses import place_bosses
+from .Dungeons import get_dungeon_item_pool_player
+from .EntranceShuffle import connect_entrance
+from .Items import ItemFactory, GetBeemizerItem
+from .Options import smallkey_shuffle, compass_shuffle, bigkey_shuffle, map_shuffle, LTTPBosses
 from .StateHelpers import has_triforce_pieces, has_melee_weapon
 
 # This file sets the item pools for various modes. Timed modes and triforce hunt are enforced first, and then extra items are specified per mode to fill in the remaining space.

worlds/alttp/Items.py

@@ -2,6 +2,7 @@ import typing
 
 from BaseClasses import ItemClassification as IC
 
+
 def GetBeemizerItem(world, player: int, item):
     item_name = item if isinstance(item, str) else item.name
 

worlds/alttp/OverworldGlitchRules.py

@@ -6,18 +6,21 @@ from BaseClasses import Entrance
 
 from .StateHelpers import can_lift_heavy_rocks, can_boots_clip_lw, can_boots_clip_dw, can_get_glitched_speed_dw
 
+
 def get_sword_required_superbunny_mirror_regions():
     """
     Cave regions that superbunny can get through - but only with a sword.
     """
     yield 'Spiral Cave (Top)'
 
+
 def get_boots_required_superbunny_mirror_regions():
     """
     Cave regions that superbunny can get through - but only with boots.
     """
     yield 'Two Brothers House'
 
+
 def get_boots_required_superbunny_mirror_locations():
     """
     Cave locations that superbunny can access - but only with boots.
@@ -207,7 +210,6 @@ def get_mirror_offset_spots_lw(player):
     yield ('Death Mountain Offset Mirror (Houlihan Exit)', 'Death Mountain', 'Hyrule Castle Ledge', lambda state: state.has('Magic Mirror', player) and can_boots_clip_dw(state, player) and state.has('Moon Pearl', player))
 
 
-
 def get_invalid_bunny_revival_dungeons():
     """
     Dungeon regions that can't be bunny revived from without superbunny state.
@@ -300,6 +302,7 @@ def create_no_logic_connections(player, world, connections):
         parent.exits.append(connection)
         connection.connect(target)
 
+
 def create_owg_connections(player, world, connections):
     for entrance, parent_region, target_region, *rule_override in connections:
         parent = world.get_region(parent_region, player)
@@ -308,6 +311,7 @@ def create_owg_connections(player, world, connections):
         parent.exits.append(connection)
         connection.connect(target)
 
+
 def set_owg_connection_rules(player, world, connections, default_rule):
     for entrance, _, _, *rule_override in connections:
         connection = world.get_entrance(entrance, player)

worlds/alttp/Rom.py

@@ -21,22 +21,22 @@ import bsdiff4
 from typing import Optional, List
 
 from BaseClasses import CollectionState, Region, Location, MultiWorld
-from worlds.alttp.Shops import ShopType, ShopPriceType
+from Utils import local_path, user_path, int16_as_bytes, int32_as_bytes, snes_to_pc, is_frozen, parse_yaml, read_snes_rom
-from worlds.alttp.Dungeons import dungeon_music_addresses
+
-from worlds.alttp.Regions import location_table, old_location_address_to_new_location_address
+from .Shops import ShopType, ShopPriceType
-from worlds.alttp.Text import MultiByteTextMapper, text_addresses, Credits, TextTable
+from .Dungeons import dungeon_music_addresses
-from worlds.alttp.Text import Uncle_texts, Ganon1_texts, TavernMan_texts, Sahasrahla2_texts, Triforce_texts, \
+from .Regions import old_location_address_to_new_location_address
+from .Text import MultiByteTextMapper, text_addresses, Credits, TextTable
+from .Text import Uncle_texts, Ganon1_texts, TavernMan_texts, Sahasrahla2_texts, Triforce_texts, \
     Blind_texts, \
     BombShop2_texts, junk_texts
-
+from .Text import KingsReturn_texts, Sanctuary_texts, Kakariko_texts, Blacksmiths_texts, \
-from worlds.alttp.Text import KingsReturn_texts, Sanctuary_texts, Kakariko_texts, Blacksmiths_texts, \
     DeathMountain_texts, \
     LostWoods_texts, WishingWell_texts, DesertPalace_texts, MountainTower_texts, LinksHouse_texts, Lumberjacks_texts, \
     SickKid_texts, FluteBoy_texts, Zora_texts, MagicShop_texts, Sahasrahla_names
-from Utils import local_path, user_path, int16_as_bytes, int32_as_bytes, snes_to_pc, is_frozen, parse_yaml, read_snes_rom
+from .Items import ItemFactory, item_table, item_name_groups, progression_items
-from worlds.alttp.Items import ItemFactory, item_table, item_name_groups, progression_items
+from .EntranceShuffle import door_addresses
-from worlds.alttp.EntranceShuffle import door_addresses
+from .Options import smallkey_shuffle
-from worlds.alttp.Options import smallkey_shuffle
 
 try:
     from maseya import z3pr

worlds/alttp/Shops.py

@@ -3,12 +3,14 @@ from enum import unique, IntEnum
 from typing import List, Optional, Set, NamedTuple, Dict
 import logging
 
-from worlds.alttp.SubClasses import ALttPLocation
-from worlds.alttp.EntranceShuffle import door_addresses
-from worlds.alttp.Items import item_name_groups, item_table, ItemFactory, trap_replaceable, GetBeemizerItem
-from worlds.alttp.Options import smallkey_shuffle
 from Utils import int16_as_bytes
 
+from .SubClasses import ALttPLocation
+from .EntranceShuffle import door_addresses
+from .Items import item_name_groups, item_table, ItemFactory, trap_replaceable, GetBeemizerItem
+from .Options import smallkey_shuffle
+
+
 logger = logging.getLogger("Shops")
 
 

worlds/alttp/StateHelpers.py

@@ -1,50 +1,62 @@
 from .SubClasses import LTTPRegion
 from BaseClasses import CollectionState
 
+
 def is_not_bunny(state: CollectionState, region: LTTPRegion, player: int) -> bool:
     if state.has('Moon Pearl', player):
         return True
 
     return region.is_light_world if state.multiworld.mode[player] != 'inverted' else region.is_dark_world
 
+
 def can_bomb_clip(state: CollectionState, region: LTTPRegion, player: int) -> bool:
     return is_not_bunny(state, region, player) and state.has('Pegasus Boots', player)
 
+
 def can_buy_unlimited(state: CollectionState, item: str, player: int) -> bool:
     return any(shop.region.player == player and shop.has_unlimited(item) and shop.region.can_reach(state) for
                 shop in state.multiworld.shops)
 
+
 def can_buy(state: CollectionState, item: str, player: int) -> bool:
     return any(shop.region.player == player and shop.has(item) and shop.region.can_reach(state) for
                 shop in state.multiworld.shops)
 
+
 def can_shoot_arrows(state: CollectionState, player: int) -> bool:
     if state.multiworld.retro_bow[player]:
         return (state.has('Bow', player) or state.has('Silver Bow', player)) and can_buy(state, 'Single Arrow', player)
     return state.has('Bow', player) or state.has('Silver Bow', player)
 
+
 def has_triforce_pieces(state: CollectionState, player: int) -> bool:
     count = state.multiworld.treasure_hunt_count[player]
     return state.item_count('Triforce Piece', player) + state.item_count('Power Star', player) >= count
 
+
 def has_crystals(state: CollectionState, count: int, player: int) -> bool:
     found = state.count_group("Crystals", player)
     return found >= count
 
+
 def can_lift_rocks(state: CollectionState, player: int):
     return state.has('Power Glove', player) or state.has('Titans Mitts', player)
 
+
 def can_lift_heavy_rocks(state: CollectionState, player: int) -> bool:
     return state.has('Titans Mitts', player)
 
+
 def bottle_count(state: CollectionState, player: int) -> int:
     return min(state.multiworld.difficulty_requirements[player].progressive_bottle_limit,
                 state.count_group("Bottles", player))
 
+
 def has_hearts(state: CollectionState, player: int, count: int) -> int:
     # Warning: This only considers items that are marked as advancement items
     return heart_count(state, player) >= count
 
+
 def heart_count(state: CollectionState, player: int) -> int:
     # Warning: This only considers items that are marked as advancement items
     diff = state.multiworld.difficulty_requirements[player]
@@ -53,6 +65,7 @@ def heart_count(state: CollectionState, player: int) -> int:
             + min(state.item_count('Piece of Heart', player), diff.heart_piece_limit) // 4 \
             + 3  # starting hearts
 
+
 def can_extend_magic(state: CollectionState, player: int, smallmagic: int = 16,
                         fullrefill: bool = False):  # This reflects the total magic Link has, not the total extra he has.
     basemagic = 8
@@ -69,6 +82,7 @@ def can_extend_magic(state: CollectionState, player: int, smallmagic: int = 16,
             basemagic = basemagic + basemagic * bottle_count(state, player)
     return basemagic >= smallmagic
 
+
 def can_kill_most_things(state: CollectionState, player: int, enemies: int = 5) -> bool:
     return (has_melee_weapon(state, player)
             or state.has('Cane of Somaria', player)
@@ -77,6 +91,7 @@ def can_kill_most_things(state: CollectionState, player: int, enemies: int = 5)
             or state.has('Fire Rod', player)
             or (state.has('Bombs (10)', player) and enemies < 6))
 
+
 def can_get_good_bee(state: CollectionState, player: int) -> bool:
     cave = state.multiworld.get_region('Good Bee Cave', player)
     return (
@@ -87,49 +102,59 @@ def can_get_good_bee(state: CollectionState, player: int) -> bool:
             is_not_bunny(state, cave, player)
     )
 
+
 def can_retrieve_tablet(state: CollectionState, player: int) -> bool:
     return state.has('Book of Mudora', player) and (has_beam_sword(state, player) or
                                                     (state.multiworld.swordless[player] and
                                                     state.has("Hammer", player)))
 
+
 def has_sword(state: CollectionState, player: int) -> bool:
     return state.has('Fighter Sword', player) \
             or state.has('Master Sword', player) \
             or state.has('Tempered Sword', player) \
             or state.has('Golden Sword', player)
 
+
 def has_beam_sword(state: CollectionState, player: int) -> bool:
     return state.has('Master Sword', player) or state.has('Tempered Sword', player) or state.has('Golden Sword',
                                                                                                 player)
 
+
 def has_melee_weapon(state: CollectionState, player: int) -> bool:
     return has_sword(state, player) or state.has('Hammer', player)
 
+
 def has_fire_source(state: CollectionState, player: int) -> bool:
     return state.has('Fire Rod', player) or state.has('Lamp', player)
 
+
 def can_melt_things(state: CollectionState, player: int) -> bool:
     return state.has('Fire Rod', player) or \
             (state.has('Bombos', player) and
             (state.multiworld.swordless[player] or
                 has_sword(state, player)))
 
+
 def has_misery_mire_medallion(state: CollectionState, player: int) -> bool:
     return state.has(state.multiworld.required_medallions[player][0], player)
 
 def has_turtle_rock_medallion(state: CollectionState, player: int) -> bool:
     return state.has(state.multiworld.required_medallions[player][1], player)
 
+
 def can_boots_clip_lw(state: CollectionState, player: int) -> bool:
     if state.multiworld.mode[player] == 'inverted':
         return state.has('Pegasus Boots', player) and state.has('Moon Pearl', player)
     return state.has('Pegasus Boots', player)
 
+
 def can_boots_clip_dw(state: CollectionState, player: int) -> bool:
     if state.multiworld.mode[player] != 'inverted':
         return state.has('Pegasus Boots', player) and state.has('Moon Pearl', player)
     return state.has('Pegasus Boots', player)
 
+
 def can_get_glitched_speed_dw(state: CollectionState, player: int) -> bool:
     rules = [state.has('Pegasus Boots', player), any([state.has('Hookshot', player), has_sword(state, player)])]
     if state.multiworld.mode[player] != 'inverted':

worlds/alttp/SubClasses.py

@@ -4,6 +4,7 @@ from enum import IntEnum
 
 from BaseClasses import Location, Item, ItemClassification, Region, MultiWorld
 
+
 class ALttPLocation(Location):
     game: str = "A Link to the Past"
     crystal: bool

worlds/alttp/UnderworldGlitchRules.py

@@ -1,9 +1,8 @@
-
 from BaseClasses import Entrance
-from .SubClasses import LTTPRegion
 from worlds.generic.Rules import set_rule, add_rule
 from .StateHelpers import can_bomb_clip, has_sword, has_beam_sword, has_fire_source, can_melt_things, has_misery_mire_medallion
 
+
 # We actually need the logic to properly "mark" these regions as Light or Dark world. 
 # Therefore we need to make these connections during the normal link_entrances stage, rather than during set_rules.
 def underworld_glitch_connections(world, player):

worlds/alttp/__init__.py

@@ -3,7 +3,6 @@ import os
 import random
 import threading
 import typing
-from collections import OrderedDict
 
 import Utils
 from BaseClasses import Item, CollectionState, Tutorial, MultiWorld
@@ -122,7 +121,7 @@ class ALTTPWorld(World):
     dungeons on your quest to rescue the descendents of the seven wise men and defeat the evil
     Ganon!
     """
-    game: str = "A Link to the Past"
+    game = "A Link to the Past"
     option_definitions = alttp_options
     topology_present = True
     item_name_groups = item_name_groups
@@ -202,7 +201,7 @@ class ALTTPWorld(World):
     location_name_to_id = lookup_name_to_id
 
     data_version = 8
-    required_client_version = (0, 3, 2)
+    required_client_version = (0, 4, 1)
     web = ALTTPWeb()
 
     pedestal_credit_texts: typing.Dict[int, str] = \