Merge branch 'main' into password-strips

.github/workflows/unittests.yml

@@ -53,7 +53,7 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install pytest "pytest-subtests<0.14.0" pytest-xdist
+        pip install pytest pytest-subtests pytest-xdist
         python ModuleUpdate.py --yes --force --append "WebHostLib/requirements.txt"
         python Launcher.py --update_settings  # make sure host.yaml exists for tests
     - name: Unittests

MultiServer.py

@@ -1915,7 +1915,7 @@ async def process_client_cmd(ctx: Context, client: Client, args: dict):
             hint = ctx.get_hint(client.team, player, location)
             if not hint:
                 return  # Ignored safely
-            if hint.receiving_player != client.slot:
+            if client.slot not in ctx.slot_set(hint.receiving_player):
                 await ctx.send_msgs(client,
                                     [{'cmd': 'InvalidPacket', "type": "arguments", "text": 'UpdateHint: No Permission',
                                       "original_cmd": cmd}])

NetUtils.py

@@ -232,7 +232,7 @@ def _handle_text(self, node: JSONMessagePart):
 
     def _handle_player_id(self, node: JSONMessagePart):
         player = int(node["text"])
-        node["color"] = 'magenta' if player == self.ctx.slot else 'yellow'
+        node["color"] = 'magenta' if self.ctx.slot_concerns_self(player) else 'yellow'
         node["text"] = self.ctx.player_names[player]
         return self._handle_color(node)
 
@@ -410,6 +410,8 @@ def get_checked(self, state: typing.Dict[typing.Tuple[int, int], typing.Set[int]
         checked = state[team, slot]
         if not checked:
             # This optimizes the case where everyone connects to a fresh game at the same time.
+            if slot not in self:
+                raise KeyError(slot)
             return []
         return [location_id for
                 location_id in self[slot] if

Options.py

@@ -754,7 +754,7 @@ def __init__(self, value: int) -> None:
         elif value > self.range_end and value not in self.special_range_names.values():
             raise Exception(f"{value} is higher than maximum {self.range_end} for option {self.__class__.__name__} " +
                             f"and is also not one of the supported named special values: {self.special_range_names}")
-        
+
         # See docstring
         for key in self.special_range_names:
             if key != key.lower():
@@ -1180,7 +1180,7 @@ def __len__(self) -> int:
 class Accessibility(Choice):
     """
     Set rules for reachability of your items/locations.
-    
+
     **Full:** ensure everything can be reached and acquired.
 
     **Minimal:** ensure what is needed to reach your goal can be acquired.
@@ -1198,7 +1198,7 @@ class Accessibility(Choice):
 class ItemsAccessibility(Accessibility):
     """
     Set rules for reachability of your items/locations.
-    
+
     **Full:** ensure everything can be reached and acquired.
 
     **Minimal:** ensure what is needed to reach your goal can be acquired.
@@ -1249,12 +1249,16 @@ class CommonOptions(metaclass=OptionsMetaProperty):
     progression_balancing: ProgressionBalancing
     accessibility: Accessibility
 
-    def as_dict(self, *option_names: str, casing: str = "snake") -> typing.Dict[str, typing.Any]:
+    def as_dict(self,
+                *option_names: str,
+                casing: typing.Literal["snake", "camel", "pascal", "kebab"] = "snake",
+                toggles_as_bools: bool = False) -> typing.Dict[str, typing.Any]:
         """
         Returns a dictionary of [str, Option.value]
 
         :param option_names: names of the options to return
         :param casing: case of the keys to return. Supports `snake`, `camel`, `pascal`, `kebab`
+        :param toggles_as_bools: whether toggle options should be output as bools instead of strings
         """
         assert option_names, "options.as_dict() was used without any option names."
         option_results = {}
@@ -1276,6 +1280,8 @@ def as_dict(self, *option_names: str, casing: str = "snake") -> typing.Dict[str,
                 value = getattr(self, option_name).value
                 if isinstance(value, set):
                     value = sorted(value)
+                elif toggles_as_bools and issubclass(type(self).type_hints[option_name], Toggle):
+                    value = bool(value)
                 option_results[display_name] = value
             else:
                 raise ValueError(f"{option_name} not found in {tuple(type(self).type_hints)}")

README.md

@@ -78,6 +78,7 @@ Currently, the following games are supported:
 * Yacht Dice
 * Faxanadu
 * Saving Princess
+* Castlevania: Circle of the Moon
 
 For setup and instructions check out our [tutorials page](https://archipelago.gg/tutorial/).
 Downloads can be found at [Releases](https://github.com/ArchipelagoMW/Archipelago/releases), including compiled

WebHost.py

@@ -34,7 +34,7 @@ def get_app() -> "Flask":
         app.config.from_file(configpath, yaml.safe_load)
         logging.info(f"Updated config from {configpath}")
     # inside get_app() so it's usable in systems like gunicorn, which do not run WebHost.py, but import it.
-    parser = argparse.ArgumentParser()
+    parser = argparse.ArgumentParser(allow_abbrev=False)
     parser.add_argument('--config_override', default=None,
                         help="Path to yaml config file that overrules config.yaml.")
     args = parser.parse_known_args()[0]

WebHostLib/generate.py

@@ -31,12 +31,12 @@ def get_meta(options_source: dict, race: bool = False) -> Dict[str, Union[List[s
 
     server_options = {
         "hint_cost": int(options_source.get("hint_cost", ServerOptions.hint_cost)),
-        "release_mode": options_source.get("release_mode", ServerOptions.release_mode),
-        "remaining_mode": options_source.get("remaining_mode", ServerOptions.remaining_mode),
-        "collect_mode": options_source.get("collect_mode", ServerOptions.collect_mode),
+        "release_mode": str(options_source.get("release_mode", ServerOptions.release_mode)),
+        "remaining_mode": str(options_source.get("remaining_mode", ServerOptions.remaining_mode)),
+        "collect_mode": str(options_source.get("collect_mode", ServerOptions.collect_mode)),
         "item_cheat": bool(int(options_source.get("item_cheat", not ServerOptions.disable_item_cheat))),
         # None if None else str.strip()
-        "server_password": options_source.get("server_password") and options_source["server_password"].strip(),
+        "server_password": str(options_source.get("server_password") and options_source["server_password"].strip()),
     }
     generator_options = {
         "spoiler": int(options_source.get("spoiler", GeneratorOptions.spoiler)),

_speedups.pyx

@@ -69,6 +69,14 @@ cdef struct IndexEntry:
     size_t count
 
 
+if TYPE_CHECKING:
+    State = Dict[Tuple[int, int], Set[int]]
+else:
+    State = Union[Tuple[int, int], Set[int], defaultdict]
+
+T = TypeVar('T')
+
+
 @cython.auto_pickle(False)
 cdef class LocationStore:
     """Compact store for locations and their items in a MultiServer"""
@@ -137,10 +145,16 @@ cdef class LocationStore:
             warnings.warn("Game has no locations")
 
         # allocate the arrays and invalidate index (0xff...)
-        self.entries = <LocationEntry*>self._mem.alloc(count, sizeof(LocationEntry))
+        if count:
+            # leaving entries as NULL if there are none, makes potential memory errors more visible
+            self.entries = <LocationEntry*>self._mem.alloc(count, sizeof(LocationEntry))
         self.sender_index = <IndexEntry*>self._mem.alloc(max_sender + 1, sizeof(IndexEntry))
         self._raw_proxies = <PyObject**>self._mem.alloc(max_sender + 1, sizeof(PyObject*))
 
+        assert (not self.entries) == (not count)
+        assert self.sender_index
+        assert self._raw_proxies
+
         # build entries and index
         cdef size_t i = 0
         for sender, locations in sorted(locations_dict.items()):
@@ -190,8 +204,6 @@ cdef class LocationStore:
             raise KeyError(key)
         return <object>self._raw_proxies[key]
 
-    T = TypeVar('T')
-
     def get(self, key: int, default: T) -> Union[PlayerLocationProxy, T]:
         # calling into self.__getitem__ here is slow, but this is not used in MultiServer
         try:
@@ -246,12 +258,11 @@ cdef class LocationStore:
                         all_locations[sender].add(entry.location)
         return all_locations
 
-    if TYPE_CHECKING:
-        State = Dict[Tuple[int, int], Set[int]]
-    else:
-        State = Union[Tuple[int, int], Set[int], defaultdict]
-
     def get_checked(self, state: State, team: int, slot: int) -> List[int]:
+        cdef ap_player_t sender = slot
+        if sender < 0 or sender >= self.sender_index_size:
+            raise KeyError(slot)
+
         # This used to validate checks actually exist. A remnant from the past.
         # If the order of locations becomes relevant at some point, we could not do sorted(set), so leaving it.
         cdef set checked = state[team, slot]
@@ -263,7 +274,6 @@ cdef class LocationStore:
 
         # Unless the set is close to empty, it's cheaper to use the python set directly, so we do that.
         cdef LocationEntry* entry
-        cdef ap_player_t sender = slot
         cdef size_t start = self.sender_index[sender].start
         cdef size_t count = self.sender_index[sender].count
         return [entry.location for
@@ -273,9 +283,11 @@ cdef class LocationStore:
     def get_missing(self, state: State, team: int, slot: int) -> List[int]:
         cdef LocationEntry* entry
         cdef ap_player_t sender = slot
+        if sender < 0 or sender >= self.sender_index_size:
+            raise KeyError(slot)
+        cdef set checked = state[team, slot]
         cdef size_t start = self.sender_index[sender].start
         cdef size_t count = self.sender_index[sender].count
-        cdef set checked = state[team, slot]
         if not len(checked):
             # Skip `in` if none have been checked.
             # This optimizes the case where everyone connects to a fresh game at the same time.
@@ -290,9 +302,11 @@ cdef class LocationStore:
     def get_remaining(self, state: State, team: int, slot: int) -> List[Tuple[int, int]]:
         cdef LocationEntry* entry
         cdef ap_player_t sender = slot
+        if sender < 0 or sender >= self.sender_index_size:
+            raise KeyError(slot)
+        cdef set checked = state[team, slot]
         cdef size_t start = self.sender_index[sender].start
         cdef size_t count = self.sender_index[sender].count
-        cdef set checked = state[team, slot]
         return sorted([(entry.receiver, entry.item) for
                         entry in self.entries[start:start+count] if
                         entry.location not in checked])
@@ -328,7 +342,8 @@ cdef class PlayerLocationProxy:
         cdef LocationEntry* entry = NULL
         # binary search
         cdef size_t l = self._store.sender_index[self._player].start
-        cdef size_t r = l + self._store.sender_index[self._player].count
+        cdef size_t e = l + self._store.sender_index[self._player].count
+        cdef size_t r = e
         cdef size_t m
         while l < r:
             m = (l + r) // 2
@@ -337,7 +352,7 @@ cdef class PlayerLocationProxy:
                 l = m + 1
             else:
                 r = m
-        if entry:  # count != 0
+        if l < e:
             entry = self._store.entries + l
             if entry.location == loc:
                 return entry
@@ -349,8 +364,6 @@ cdef class PlayerLocationProxy:
             return entry.item, entry.receiver, entry.flags
         raise KeyError(f"No location {key} for player {self._player}")
 
-    T = TypeVar('T')
-
     def get(self, key: int, default: T) -> Union[Tuple[int, int, int], T]:
         cdef LocationEntry* entry = self._get(key)
         if entry:

_speedups.pyxbld

@@ -3,8 +3,16 @@ import os
 
 def make_ext(modname, pyxfilename):
     from distutils.extension import Extension
-    return Extension(name=modname,
-                     sources=[pyxfilename],
-                     depends=["intset.h"],
-                     include_dirs=[os.getcwd()],
-                     language="c")
+    return Extension(
+        name=modname,
+        sources=[pyxfilename],
+        depends=["intset.h"],
+        include_dirs=[os.getcwd()],
+        language="c",
+        # to enable ASAN and debug build:
+        # extra_compile_args=["-fsanitize=address", "-UNDEBUG", "-Og", "-g"],
+        # extra_objects=["-fsanitize=address"],
+        # NOTE: we can not put -lasan at the front of link args, so needs to be run with
+        #       LD_PRELOAD=/usr/lib/libasan.so ASAN_OPTIONS=detect_leaks=0 path/to/exe
+        # NOTE: this can't find everything unless libpython and cymem are also built with ASAN
+    )

docs/CODEOWNERS

@@ -36,6 +36,9 @@
 # Castlevania 64
 /worlds/cv64/ @LiquidCat64
 
+# Castlevania: Circle of the Moon
+/worlds/cvcotm/ @LiquidCat64
+
 # Celeste 64
 /worlds/celeste64/ @PoryGone
 

docs/apworld_dev_faq.md

@@ -43,3 +43,26 @@ A faster alternative to the `for` loop would be to use a [list comprehension](ht
 ```py
 item_pool += [self.create_filler() for _ in range(total_locations - len(item_pool))]
 ```
+
+---
+
+### I learned about indirect conditions in the world API document, but I want to know more. What are they and why are they necessary?
+
+The world API document mentions how to use `multiworld.register_indirect_condition` to register indirect conditions and **when** you should use them, but not *how* they work and *why* they are necessary. This is because the explanation is quite complicated.
+
+Region sweep (the algorithm that determines which regions are reachable) is a Breadth-First Search of the region graph. It starts from the origin region, checks entrances one by one, and adds newly reached regions and their entrances to the queue until there is nothing more to check.
+
+For performance reasons, AP only checks every entrance once. However, if an entrance's access_rule depends on region access, then the following may happen:
+1. The entrance is checked and determined to be nontraversable because the region in its access_rule hasn't been reached yet during the graph search.
+2. Then, the region in its access_rule is determined to be reachable.
+
+This entrance *would* be in logic if it were rechecked, but it won't be rechecked this cycle.
+To account for this case, AP would have to recheck all entrances every time a new region is reached until no new regions are reached.
+
+An indirect condition is how you can manually define that a specific entrance needs to be rechecked during region sweep if a specific region is reached during it.
+This keeps most of the performance upsides. Even in a game making heavy use of indirect conditions (ex: The Witness), using them is significantly faster than just "rechecking each entrance until nothing new is found".
+The reason entrance access rules using `location.can_reach` and `entrance.can_reach` are also affected is because they call `region.can_reach` on their respective parent/source region.
+
+We recognize it can feel like a trap since it will not alert you when you are missing an indirect condition, and that some games have very complex access rules.
+As of [PR #3682 (Core: Region handling customization)](https://github.com/ArchipelagoMW/Archipelago/pull/3682) being merged, it is possible for a world to opt out of indirect conditions entirely, instead using the system of checking each entrance whenever a region has been reached, although this does come with a performance cost.
+Opting out of using indirect conditions should only be used by games that *really* need it. For most games, it should be reasonable to know all entrance → region dependencies, making indirect conditions preferred because they are much faster.

docs/running from source.md

@@ -43,9 +43,9 @@ Recommended steps
      [Discord in #ap-core-dev](https://discord.com/channels/731205301247803413/731214280439103580/905154456377757808)
 
  * It is recommended to use [PyCharm IDE](https://www.jetbrains.com/pycharm/)
- * Run Generate.py which will prompt installation of missing modules, press enter to confirm
-   * In PyCharm: right-click Generate.py and select `Run 'Generate'`
-   * Without PyCharm: open a command prompt in the source folder and type `py Generate.py`
+ * Run ModuleUpdate.py which will prompt installation of missing modules, press enter to confirm
+   * In PyCharm: right-click ModuleUpdate.py and select `Run 'ModuleUpdate'`
+   * Without PyCharm: open a command prompt in the source folder and type `py ModuleUpdate.py`
 
 
 ## macOS

inno_setup.iss

@@ -186,6 +186,11 @@ Root: HKCR; Subkey: "{#MyAppName}cv64patch";                     ValueData: "Arc
 Root: HKCR; Subkey: "{#MyAppName}cv64patch\DefaultIcon";         ValueData: "{app}\ArchipelagoBizHawkClient.exe,0";                                          ValueType: string; ValueName: "";
 Root: HKCR; Subkey: "{#MyAppName}cv64patch\shell\open\command";  ValueData: """{app}\ArchipelagoBizHawkClient.exe"" ""%1""";                                 ValueType: string; ValueName: "";
 
+Root: HKCR; Subkey: ".apcvcotm";                                 ValueData: "{#MyAppName}cvcotmpatch";                               Flags: uninsdeletevalue; ValueType: string; ValueName: "";
+Root: HKCR; Subkey: "{#MyAppName}cvcotmpatch";                     ValueData: "Archipelago Castlevania Circle of the Moon Patch";                    Flags: uninsdeletekey;   ValueType: string; ValueName: "";
+Root: HKCR; Subkey: "{#MyAppName}cvcotmpatch\DefaultIcon";         ValueData: "{app}\ArchipelagoBizHawkClient.exe,0";                                          ValueType: string; ValueName: "";
+Root: HKCR; Subkey: "{#MyAppName}cvcotmpatch\shell\open\command";  ValueData: """{app}\ArchipelagoBizHawkClient.exe"" ""%1""";                                 ValueType: string; ValueName: "";
+
 Root: HKCR; Subkey: ".apmm2";                                   ValueData: "{#MyAppName}mm2patch";                               Flags: uninsdeletevalue; ValueType: string; ValueName: "";
 Root: HKCR; Subkey: "{#MyAppName}mm2patch";                     ValueData: "Archipelago Mega Man 2 Patch";                    Flags: uninsdeletekey;   ValueType: string; ValueName: "";
 Root: HKCR; Subkey: "{#MyAppName}mm2patch\DefaultIcon";         ValueData: "{app}\ArchipelagoBizHawkClient.exe,0";                                          ValueType: string; ValueName: "";

kvui.py

@@ -371,7 +371,7 @@ def on_touch_down(self, touch):
                     if self.hint["status"] == HintStatus.HINT_FOUND:
                         return
                     ctx = App.get_running_app().ctx
-                    if ctx.slot == self.hint["receiving_player"]:  # If this player owns this hint
+                    if ctx.slot_concerns_self(self.hint["receiving_player"]):  # If this player owns this hint
                         # open a dropdown
                         self.dropdown.open(self.ids["status"])
                 elif self.selected:
@@ -800,7 +800,7 @@ def refresh_hints(self, hints):
             hint_status_node = self.parser.handle_node({"type": "color",
                                                         "color": status_colors.get(hint["status"], "red"),
                                                         "text": status_names.get(hint["status"], "Unknown")})
-            if hint["status"] != HintStatus.HINT_FOUND and hint["receiving_player"] == ctx.slot:
+            if hint["status"] != HintStatus.HINT_FOUND and ctx.slot_concerns_self(hint["receiving_player"]):
                 hint_status_node = f"[u]{hint_status_node}[/u]"
             data.append({
                 "receiving": {"text": self.parser.handle_node({"type": "player_id", "text": hint["receiving_player"]})},