Merge remote-tracking branch 'upstream/main' into kp/ChestRename

.github/labeler.yml

@@ -27,4 +27,5 @@
     - '!WebHostLib/**'
   - any-glob-to-any-file: # exceptions to the above rules of "stuff that isn't core"
     - 'worlds/generic/**/*.py'
+    - 'worlds/*.py'
     - 'CommonClient.py'

.github/workflows/scan-build.yml

@@ -0,0 +1,65 @@
+name: Native Code Static Analysis
+
+on:
+  push:
+    paths:
+      - '**.c'
+      - '**.cc'
+      - '**.cpp'
+      - '**.cxx'
+      - '**.h'
+      - '**.hh'
+      - '**.hpp'
+      - '**.pyx'
+      - 'setup.py'
+      - 'requirements.txt'
+      - '.github/workflows/scan-build.yml'
+  pull_request:
+    paths:
+      - '**.c'
+      - '**.cc'
+      - '**.cpp'
+      - '**.cxx'
+      - '**.h'
+      - '**.hh'
+      - '**.hpp'
+      - '**.pyx'
+      - 'setup.py'
+      - 'requirements.txt'
+      - '.github/workflows/scan-build.yml'
+
+jobs:
+  scan-build:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+      with:
+        submodules: recursive
+    - name: Install newer Clang
+      run: |
+        wget https://apt.llvm.org/llvm.sh
+        chmod +x ./llvm.sh
+        sudo ./llvm.sh 17
+    - name: Install scan-build command
+      run: |
+        sudo apt install clang-tools-17
+    - name: Get a recent python
+      uses: actions/setup-python@v5
+      with:
+        python-version: '3.11'
+    - name: Install dependencies
+      run: |
+        python -m venv venv
+        source venv/bin/activate
+        python -m pip install --upgrade pip -r requirements.txt
+    - name: scan-build
+      run: |
+        source venv/bin/activate
+        scan-build-17 --status-bugs -o scan-build-reports -disable-checker deadcode.DeadStores python setup.py build -y
+    - name: Store report
+      if: failure()
+      uses: actions/upload-artifact@v4
+      with:
+        name: scan-build-reports
+        path: scan-build-reports

BaseClasses.py

@@ -110,10 +110,14 @@ def __iadd__(self, other: Iterable[Region]):
             return self
 
         def append(self, region: Region):
+            assert region.name not in self.region_cache[region.player], \
+                f"{region.name} already exists in region cache."
             self.region_cache[region.player][region.name] = region
 
         def extend(self, regions: Iterable[Region]):
             for region in regions:
+                assert region.name not in self.region_cache[region.player], \
+                    f"{region.name} already exists in region cache."
                 self.region_cache[region.player][region.name] = region
 
         def add_group(self, new_id: int):
@@ -877,6 +881,8 @@ def __delitem__(self, index: int) -> None:
             del(self.region_manager.location_cache[location.player][location.name])
 
         def insert(self, index: int, value: Location) -> None:
+            assert value.name not in self.region_manager.location_cache[value.player], \
+                f"{value.name} already exists in the location cache."
             self._list.insert(index, value)
             self.region_manager.location_cache[value.player][value.name] = value
 
@@ -887,6 +893,8 @@ def __delitem__(self, index: int) -> None:
             del(self.region_manager.entrance_cache[entrance.player][entrance.name])
 
         def insert(self, index: int, value: Entrance) -> None:
+            assert value.name not in self.region_manager.entrance_cache[value.player], \
+                f"{value.name} already exists in the entrance cache."
             self._list.insert(index, value)
             self.region_manager.entrance_cache[value.player][value.name] = value
 
@@ -1272,12 +1280,12 @@ def create_playthrough(self, create_paths: bool = True) -> None:
             for location in sphere:
                 state.collect(location.item, True, location)
 
-            required_locations -= sphere
-
             collection_spheres.append(sphere)
 
             logging.debug('Calculated final sphere %i, containing %i of %i progress items.', len(collection_spheres),
                           len(sphere), len(required_locations))
+
+            required_locations -= sphere
             if not sphere:
                 raise RuntimeError(f'Not all required items reachable. Unreachable locations: {required_locations}')
 

Generate.py

@@ -462,20 +462,18 @@ def roll_settings(weights: dict, plando_options: PlandoOptions = PlandoOptions.b
         handle_option(ret, game_weights, option_key, option, plando_options)
     if PlandoOptions.items in plando_options:
         ret.plando_items = game_weights.get("plando_items", [])
-    if ret.game == "Minecraft" or ret.game == "Ocarina of Time":
-        # bad hardcoded behavior to make this work for now
-        ret.plando_connections = []
-        if PlandoOptions.connections in plando_options:
-            options = game_weights.get("plando_connections", [])
-            for placement in options:
-                if roll_percentage(get_choice("percentage", placement, 100)):
-                    ret.plando_connections.append(PlandoConnection(
-                        get_choice("entrance", placement),
-                        get_choice("exit", placement),
-                        get_choice("direction", placement)
-                    ))
-    elif ret.game == "A Link to the Past":
+    if ret.game == "A Link to the Past":
         roll_alttp_settings(ret, game_weights, plando_options)
+    if PlandoOptions.connections in plando_options:
+        ret.plando_connections = []
+        options = game_weights.get("plando_connections", [])
+        for placement in options:
+            if roll_percentage(get_choice("percentage", placement, 100)):
+                ret.plando_connections.append(PlandoConnection(
+                    get_choice("entrance", placement),
+                    get_choice("exit", placement),
+                    get_choice("direction", placement, "both")
+                ))
 
     return ret
 
@@ -494,17 +492,6 @@ def roll_alttp_settings(ret: argparse.Namespace, weights, plando_options):
                     raise Exception(f"No text target \"{at}\" found.")
                 ret.plando_texts[at] = str(get_choice_legacy("text", placement))
 
-    ret.plando_connections = []
-    if PlandoOptions.connections in plando_options:
-        options = weights.get("plando_connections", [])
-        for placement in options:
-            if roll_percentage(get_choice_legacy("percentage", placement, 100)):
-                ret.plando_connections.append(PlandoConnection(
-                    get_choice_legacy("entrance", placement),
-                    get_choice_legacy("exit", placement),
-                    get_choice_legacy("direction", placement, "both")
-                ))
-
     ret.sprite_pool = weights.get('sprite_pool', [])
     ret.sprite = get_choice_legacy('sprite', weights, "Link")
     if 'random_sprite_on_event' in weights:

_speedups.pyx

@@ -48,6 +48,7 @@ cdef struct IndexEntry:
     size_t count
 
 
+@cython.auto_pickle(False)
 cdef class LocationStore:
     """Compact store for locations and their items in a MultiServer"""
     # The original implementation uses Dict[int, Dict[int, Tuple(int, int, int]]
@@ -78,18 +79,6 @@ cdef class LocationStore:
         size += sizeof(self._raw_proxies[0]) * self.sender_index_size
         return size
 
-    def __cinit__(self, locations_dict: Dict[int, Dict[int, Sequence[int]]]) -> None:
-        self._mem = None
-        self._keys = None
-        self._items = None
-        self._proxies = None
-        self._len = 0
-        self.entries = NULL
-        self.entry_count = 0
-        self.sender_index = NULL
-        self.sender_index_size = 0
-        self._raw_proxies = NULL
-
     def __init__(self, locations_dict: Dict[int, Dict[int, Sequence[int]]]) -> None:
         self._mem = Pool()
         cdef object key
@@ -281,6 +270,7 @@ cdef class LocationStore:
                        entry.location not in checked])
 
 
+@cython.auto_pickle(False)
 @cython.internal  # unsafe. disable direct import
 cdef class PlayerLocationProxy:
     cdef LocationStore _store

docs/network protocol.md

@@ -327,7 +327,11 @@ Sent to server to inform it of locations that the client has checked. Used to in
 | locations | list\[int\] | The ids of the locations checked by the client. May contain any number of checks, even ones sent before; duplicates do not cause issues with the Archipelago server. |
 
 ### LocationScouts
-Sent to the server to inform it of locations the client has seen, but not checked. Useful in cases in which the item may appear in the game world, such as 'ledge items' in A Link to the Past. The server will always respond with a [LocationInfo](#LocationInfo) packet with the items located in the scouted location.
+Sent to the server to retrieve the items that are on a specified list of locations. The server will respond with a [LocationInfo](#LocationInfo) packet containing the items located in the scouted locations.
+Fully remote clients without a patch file may use this to "place" items onto their in-game locations, most commonly to display their names or item classifications before/upon pickup.
+
+LocationScouts can also be used to inform the server of locations the client has seen, but not checked. This creates a hint as if the player had run `!hint_location` on a location, but without deducting hint points.
+This is useful in cases where an item appears in the game world, such as 'ledge items' in _A Link to the Past_. To do this, set the `create_as_hint` parameter to a non-zero value.
 
 #### Arguments
 | Name | Type | Notes |

docs/options api.md

@@ -24,8 +24,11 @@ display as `Value1` on the webhost.
 (i.e. `alias_value_1 = option_value1`) which will allow users to use either `value_1` or `value1` in their yaml
 files, and both will resolve as `value1`. This should be used when changing options around, i.e. changing a Toggle to a
 Choice, and defining `alias_true = option_full`.
-- All options support `random` as a generic option. `random` chooses from any of the available values for that option,
-and is reserved by AP. You can set this as your default value, but you cannot define your own `option_random`.
+- All options with a fixed set of possible values (i.e. those which inherit from `Toggle`, `(Text)Choice` or
+`(Named/Special)Range`) support `random` as a generic option. `random` chooses from any of the available values for that
+option, and is reserved by AP. You can set this as your default value, but you cannot define your own `option_random`.
+However, you can override `from_text` and handle `text == "random"` to customize its behavior or
+implement it for additional option types.
 
 As an example, suppose we want an option that lets the user start their game with a sword in their inventory, an option
 to let the player choose the difficulty, and an option to choose how much health the final boss has. Let's create our

test/general/test_helpers.py

@@ -29,8 +29,8 @@ def test_region_helpers(self) -> None:
                 "event_loc": None,
             },
             "TestRegion2": {
-                "loc_1": 321,
-                "loc_2": 654,
+                "loc_3": 321,
+                "loc_4": 654,
             }
         }
 

worlds/AutoWorld.py

@@ -15,7 +15,7 @@
 
 if TYPE_CHECKING:
     import random
-    from BaseClasses import MultiWorld, Item, Location, Tutorial
+    from BaseClasses import MultiWorld, Item, Location, Tutorial, Region, Entrance
     from . import GamesPackage
     from settings import Group
 
@@ -458,6 +458,16 @@ def remove(self, state: "CollectionState", item: "Item") -> bool:
     def create_filler(self) -> "Item":
         return self.create_item(self.get_filler_item_name())
 
+    # convenience methods
+    def get_location(self, location_name: str) -> "Location":
+        return self.multiworld.get_location(location_name, self.player)
+
+    def get_entrance(self, entrance_name: str) -> "Entrance":
+        return self.multiworld.get_entrance(entrance_name, self.player)
+
+    def get_region(self, region_name: str) -> "Region":
+        return self.multiworld.get_region(region_name, self.player)
+
     @classmethod
     def get_data_package_data(cls) -> "GamesPackage":
         sorted_item_name_groups = {

worlds/alttp/Options.py

@@ -43,8 +43,7 @@ class Goal(Choice):
     Triforce Hunt: Collect Triforce pieces spread throughout the worlds, then turn them in to Murahadala in front of Hyrule Castle
     Local Triforce Hunt: Collect Triforce pieces spread throughout your world, then turn them in to Murahadala in front of Hyrule Castle
     Ganon Triforce Hunt: Collect Triforce pieces spread throughout the worlds, then kill Ganon
-    Local Ganon Triforce Hunt: Collect Triforce pieces spread throughout your world, then kill Ganon
-    Ice Rod Hunt: You start with everything except Ice Rod. Find the Ice rod, then kill Trinexx at Turtle rock."""
+    Local Ganon Triforce Hunt: Collect Triforce pieces spread throughout your world, then kill Ganon"""
     display_name = "Goal"
     default = 0
     option_ganon = 0
@@ -211,13 +210,12 @@ class map_shuffle(DungeonItem):
     display_name = "Map Shuffle"
 
 
-class key_drop_shuffle(Toggle):
+class key_drop_shuffle(DefaultOnToggle):
     """Shuffle keys found in pots and dropped from killed enemies,
     respects the small key and big key shuffle options."""
     display_name = "Key Drop Shuffle"
 
 
-
 class DungeonCounters(Choice):
     """On: Always display amount of items checked in a dungeon. Pickup: Show when compass is picked up.
     Default: Show when compass is picked up if the compass itself is shuffled. Off: Never show item count in dungeons."""

worlds/alttp/Rules.py

@@ -397,9 +397,9 @@ def global_rules(world, player):
         set_rule(world.get_entrance('Blind Fight', player), lambda state: state._lttp_has_key('Small Key (Thieves Town)', player, 3) and can_use_bombs(state, player))
 
     set_rule(world.get_location('Thieves\' Town - Big Chest', player),
-             lambda state: (state._lttp_has_key('Small Key (Thieves Town)', player, 3)) and state.has('Hammer', player))
+             lambda state: ((state._lttp_has_key('Small Key (Thieves Town)', player, 3)) or (location_item_name(state, 'Thieves\' Town - Big Chest', player) == ("Small Key (Thieves Town)", player)) and state._lttp_has_key('Small Key (Thieves Town)', player, 2)) and state.has('Hammer', player))
     if world.accessibility[player] != 'locations':
-        allow_self_locking_items(world.get_location('Thieves\' Town - Big Chest', player), 'Small Key (Thieves Town)')
+        set_always_allow(world.get_location('Thieves\' Town - Big Chest', player), lambda state, item: item.name == 'Small Key (Thieves Town)' and item.player == player)
 
     set_rule(world.get_location('Thieves\' Town - Attic', player), lambda state: state._lttp_has_key('Small Key (Thieves Town)', player, 3))
     set_rule(world.get_location('Thieves\' Town - Spike Switch Pot Key', player),
@@ -420,11 +420,7 @@ def global_rules(world, player):
     set_rule(world.get_location('Ice Palace - Jelly Key Drop', player), lambda state: can_melt_things(state, player))
     set_rule(world.get_location('Ice Palace - Pengator Chest', player), lambda state: can_melt_things(state, player) and state._lttp_has_key('Small Key (Ice Palace)', player))
     set_rule(world.get_entrance('Ice Palace (Second Section)', player), lambda state: can_melt_things(state, player) and state._lttp_has_key('Small Key (Ice Palace)', player) and can_use_bombs(state, player))
-    if not world.enemy_shuffle[player]:
-        # Stalfos Knights can be killed by damaging them repeatedly with boomerang, swords, etc. if bombs are
-        # unavailable. If bombs are available, the pots can be thrown at them, so no other weapons are needed
-        add_rule(world.get_entrance('Ice Palace (Second Section)', player), lambda state: (can_use_bombs(state, player)
-            or state.has('Blue Boomerang', player) or state.has('Red Boomerang', player) or has_sword(state, player) or state.has("Hammer", player)))
+
     set_rule(world.get_entrance('Ice Palace (Main)', player), lambda state: state._lttp_has_key('Small Key (Ice Palace)', player, 2))
     set_rule(world.get_location('Ice Palace - Big Chest', player), lambda state: state.has('Big Key (Ice Palace)', player))
     set_rule(world.get_entrance('Ice Palace (Kholdstare)', player), lambda state: can_lift_rocks(state, player) and state.has('Hammer', player) and state.has('Big Key (Ice Palace)', player) and (state._lttp_has_key('Small Key (Ice Palace)', player, 6) or (state.has('Cane of Somaria', player) and state._lttp_has_key('Small Key (Ice Palace)', player, 5))))

worlds/alttp/test/dungeons/TestAgahnimsTower.py

@@ -23,14 +23,14 @@ def testTower(self):
             ["Castle Tower - Dark Archer Key Drop", True, ['Progressive Sword', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Lamp']],
 
             ["Castle Tower - Circle of Pots Key Drop", False, []],
-            ["Castle Tower - Circle of Pots Key Drop", False, ['Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)']],
+            ["Castle Tower - Circle of Pots Key Drop", False, ['Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)']],
             ["Castle Tower - Circle of Pots Key Drop", False, [], ['Lamp']],
             ["Castle Tower - Circle of Pots Key Drop", False, [], ['Bomb Upgrade (+5)', 'Bomb Upgrade (+10)', 'Bomb Upgrade (50)', 'Progressive Sword', 'Hammer', 'Progressive Bow', 'Fire Rod', 'Ice Rod', 'Cane of Somaria', 'Cane of Byrna']],
-            ["Castle Tower - Circle of Pots Key Drop", True, ['Progressive Sword', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Lamp']],
+            ["Castle Tower - Circle of Pots Key Drop", True, ['Progressive Sword', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Lamp']],
 
             ["Agahnim 1", False, []],
-            ["Agahnim 1", False, ['Small Key (Agahnims Tower)'], ['Small Key (Agahnims Tower)']],
+            ["Agahnim 1", False, ['Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)']],
             ["Agahnim 1", False, [], ['Progressive Sword']],
             ["Agahnim 1", False, [], ['Lamp']],
-            ["Agahnim 1", True, ['Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Lamp', 'Progressive Sword']],
+            ["Agahnim 1", True, ['Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Small Key (Agahnims Tower)', 'Lamp', 'Progressive Sword']],
         ])