Merge branch 'sc2-next' into item-description-labels

.gitignore

@@ -150,7 +150,7 @@ venv/
 ENV/
 env.bak/
 venv.bak/
-.code-workspace
+*.code-workspace
 shell.nix
 
 # Spyder project settings

BaseClasses.py

@@ -680,13 +680,13 @@ def can_reach_entrance(self, spot: str, player: int) -> bool:
     def can_reach_region(self, spot: str, player: int) -> bool:
         return self.multiworld.get_region(spot, player).can_reach(self)
 
-    def sweep_for_events(self, key_only: bool = False, locations: Optional[Iterable[Location]] = None) -> None:
+    def sweep_for_events(self, locations: Optional[Iterable[Location]] = None) -> None:
         if locations is None:
             locations = self.multiworld.get_filled_locations()
         reachable_events = True
         # since the loop has a good chance to run more than once, only filter the events once
-        locations = {location for location in locations if location.advancement and location not in self.events and
-                     not key_only or getattr(location.item, "locked_dungeon_item", False)}
+        locations = {location for location in locations if location.advancement and location not in self.events}
+
         while reachable_events:
             reachable_events = {location for location in locations if location.can_reach(self)}
             locations -= reachable_events
@@ -1291,8 +1291,6 @@ def create_playthrough(self, create_paths: bool = True) -> None:
         state = CollectionState(multiworld)
         collection_spheres = []
         while required_locations:
-            state.sweep_for_events(key_only=True)
-
             sphere = set(filter(state.can_reach, required_locations))
 
             for location in sphere:

CommonClient.py

@@ -61,6 +61,7 @@ def _cmd_connect(self, address: str = "") -> bool:
         if address:
             self.ctx.server_address = None
             self.ctx.username = None
+            self.ctx.password = None
         elif not self.ctx.server_address:
             self.output("Please specify an address.")
             return False
@@ -514,6 +515,7 @@ def update_permissions(self, permissions: typing.Dict[str, int]):
     async def shutdown(self):
         self.server_address = ""
         self.username = None
+        self.password = None
         self.cancel_autoreconnect()
         if self.server and not self.server.socket.closed:
             await self.server.socket.close()

Fill.py

@@ -646,7 +646,6 @@ def balance_multiworld_progression(multiworld: MultiWorld) -> None:
 
         def get_sphere_locations(sphere_state: CollectionState,
                                  locations: typing.Set[Location]) -> typing.Set[Location]:
-            sphere_state.sweep_for_events(key_only=True, locations=locations)
             return {loc for loc in locations if sphere_state.can_reach(loc)}
 
         def item_percentage(player: int, num: int) -> float:

Main.py

@@ -124,14 +124,19 @@ def main(args, seed=None, baked_server_options: Optional[Dict[str, object]] = No
     for player in multiworld.player_ids:
         exclusion_rules(multiworld, player, multiworld.worlds[player].options.exclude_locations.value)
         multiworld.worlds[player].options.priority_locations.value -= multiworld.worlds[player].options.exclude_locations.value
+        world_excluded_locations = set()
         for location_name in multiworld.worlds[player].options.priority_locations.value:
             try:
                 location = multiworld.get_location(location_name, player)
-            except KeyError as e:  # failed to find the given location. Check if it's a legitimate location
-                if location_name not in multiworld.worlds[player].location_name_to_id:
-                    raise Exception(f"Unable to prioritize location {location_name} in player {player}'s world.") from e
-            else:
+            except KeyError:
+                continue
+
+            if location.progress_type != LocationProgressType.EXCLUDED:
                 location.progress_type = LocationProgressType.PRIORITY
+            else:
+                logger.warning(f"Unable to prioritize location \"{location_name}\" in player {player}'s world because the world excluded it.")
+                world_excluded_locations.add(location_name)
+        multiworld.worlds[player].options.priority_locations.value -= world_excluded_locations
 
     # Set local and non-local item rules.
     if multiworld.players > 1:

MultiServer.py

@@ -1352,7 +1352,7 @@ def _cmd_remaining(self) -> bool:
         if self.ctx.remaining_mode == "enabled":
             remaining_item_ids = get_remaining(self.ctx, self.client.team, self.client.slot)
             if remaining_item_ids:
-                self.output("Remaining items: " + ", ".join(self.ctx.item_names[self.client.slot.game][item_id]
+                self.output("Remaining items: " + ", ".join(self.ctx.item_names[self.ctx.games[self.client.slot]][item_id]
                                                             for item_id in remaining_item_ids))
             else:
                 self.output("No remaining items found.")
@@ -1365,7 +1365,7 @@ def _cmd_remaining(self) -> bool:
             if self.ctx.client_game_state[self.client.team, self.client.slot] == ClientStatus.CLIENT_GOAL:
                 remaining_item_ids = get_remaining(self.ctx, self.client.team, self.client.slot)
                 if remaining_item_ids:
-                    self.output("Remaining items: " + ", ".join(self.ctx.item_names[self.client.slot.game][item_id]
+                    self.output("Remaining items: " + ", ".join(self.ctx.item_names[self.ctx.games[self.client.slot]][item_id]
                                                                 for item_id in remaining_item_ids))
                 else:
                     self.output("No remaining items found.")

UndertaleClient.py

@@ -29,7 +29,7 @@ def _cmd_resync(self):
     def _cmd_patch(self):
         """Patch the game. Only use this command if /auto_patch fails."""
         if isinstance(self.ctx, UndertaleContext):
-            os.makedirs(name=os.path.join(os.getcwd(), "Undertale"), exist_ok=True)
+            os.makedirs(name=Utils.user_path("Undertale"), exist_ok=True)
             self.ctx.patch_game()
             self.output("Patched.")
 
@@ -43,7 +43,7 @@ def _cmd_savepath(self, directory: str):
     def _cmd_auto_patch(self, steaminstall: typing.Optional[str] = None):
         """Patch the game automatically."""
         if isinstance(self.ctx, UndertaleContext):
-            os.makedirs(name=os.path.join(os.getcwd(), "Undertale"), exist_ok=True)
+            os.makedirs(name=Utils.user_path("Undertale"), exist_ok=True)
             tempInstall = steaminstall
             if not os.path.isfile(os.path.join(tempInstall, "data.win")):
                 tempInstall = None
@@ -62,7 +62,7 @@ def _cmd_auto_patch(self, steaminstall: typing.Optional[str] = None):
                 for file_name in os.listdir(tempInstall):
                     if file_name != "steam_api.dll":
                         shutil.copy(os.path.join(tempInstall, file_name),
-                               os.path.join(os.getcwd(), "Undertale", file_name))
+                               Utils.user_path("Undertale", file_name))
                 self.ctx.patch_game()
                 self.output("Patching successful!")
 
@@ -111,12 +111,12 @@ def __init__(self, server_address, password):
         self.save_game_folder = os.path.expandvars(r"%localappdata%/UNDERTALE")
 
     def patch_game(self):
-        with open(os.path.join(os.getcwd(), "Undertale", "data.win"), "rb") as f:
+        with open(Utils.user_path("Undertale", "data.win"), "rb") as f:
             patchedFile = bsdiff4.patch(f.read(), undertale.data_path("patch.bsdiff"))
-        with open(os.path.join(os.getcwd(), "Undertale", "data.win"), "wb") as f:
+        with open(Utils.user_path("Undertale", "data.win"), "wb") as f:
             f.write(patchedFile)
-        os.makedirs(name=os.path.join(os.getcwd(), "Undertale", "Custom Sprites"), exist_ok=True)
-        with open(os.path.expandvars(os.path.join(os.getcwd(), "Undertale", "Custom Sprites",
+        os.makedirs(name=Utils.user_path("Undertale", "Custom Sprites"), exist_ok=True)
+        with open(os.path.expandvars(Utils.user_path("Undertale", "Custom Sprites",
                                      "Which Character.txt")), "w") as f:
             f.writelines(["// Put the folder name of the sprites you want to play as, make sure it is the only "
                           "line other than this one.\n", "frisk"])

WebHostLib/customserver.py

@@ -325,10 +325,12 @@ def _done(self, task: asyncio.Future):
         def run(self):
             while 1:
                 next_room = rooms_to_run.get(block=True,  timeout=None)
+                gc.collect(0)
                 task = asyncio.run_coroutine_threadsafe(start_room(next_room), loop)
                 self._tasks.append(task)
                 task.add_done_callback(self._done)
                 logging.info(f"Starting room {next_room} on {name}.")
+                del task  # delete reference to task object
 
     starter = Starter()
     starter.daemon = True

WebHostLib/tracker.py

@@ -79,7 +79,7 @@ def __init__(self, room: Room):
 
             # Normal lookup tables as well.
             self.item_name_to_id[game] = game_package["item_name_to_id"]
-            self.location_name_to_id[game] = game_package["item_name_to_id"]
+            self.location_name_to_id[game] = game_package["location_name_to_id"]
 
     def get_seed_name(self) -> str:
         """Retrieves the seed name."""

docs/CODEOWNERS

@@ -1,8 +1,8 @@
 # Archipelago World Code Owners / Maintainers Document
 #
-# This file is used to notate the current "owners" or "maintainers" of any currently merged world folder. For any pull
-# requests that modify these worlds, a code owner must approve the PR in addition to a core maintainer. This is not to
-# be used for files/folders outside the /worlds folder, those will always need sign off from a core maintainer.
+# This file is used to notate the current "owners" or "maintainers" of any currently merged world folder as well as
+# certain documentation. For any pull requests that modify these worlds/docs, a code owner must approve the PR in
+# addition to a core maintainer. All other files and folders are owned and maintained by core maintainers directly.
 #
 # All usernames must be GitHub usernames (and are case sensitive).
 
@@ -226,3 +226,11 @@
 
 # Ori and the Blind Forest
 # /worlds_disabled/oribf/
+
+###################
+## Documentation ##
+###################
+
+# Apworld Dev Faq
+/docs/apworld_dev_faq.md @qwint @ScipioWright
+

docs/apworld_dev_faq.md

@@ -0,0 +1,45 @@
+# APWorld Dev FAQ
+
+This document is meant as a reference tool to show solutions to common problems when developing an apworld.
+It is not intended to answer every question about Archipelago and it assumes you have read the other docs, 
+including [Contributing](contributing.md), [Adding Games](<adding games.md>), and [World API](<world api.md>).
+
+---
+
+### My game has a restrictive start that leads to fill errors
+
+Hint to the Generator that an item needs to be in sphere one with local_early_items. Here, `1` represents the number of "Sword" items to attempt to place in sphere one.
+```py
+early_item_name = "Sword"
+self.multiworld.local_early_items[self.player][early_item_name] = 1
+```
+
+Some alternative ways to try to fix this problem are:
+* Add more locations to sphere one of your world, potentially only when there would be a restrictive start
+* Pre-place items yourself, such as during `create_items`
+* Put items into the player's starting inventory using `push_precollected`
+* Raise an exception, such as an `OptionError` during `generate_early`, to disallow options that would lead to a restrictive start
+
+---
+
+### I have multiple settings that change the item/location pool counts and need to balance them out
+
+In an ideal situation your system for producing locations and items wouldn't leave any opportunity for them to be unbalanced. But in real, complex situations, that might be unfeasible.
+
+If that's the case, you can create extra filler based on the difference between your unfilled locations and your itempool by comparing [get_unfilled_locations](https://github.com/ArchipelagoMW/Archipelago/blob/main/BaseClasses.py#:~:text=get_unfilled_locations) to your list of items to submit
+
+Note: to use self.create_filler(), self.get_filler_item_name() should be defined to only return valid filler item names
+```py
+total_locations = len(self.multiworld.get_unfilled_locations(self.player))
+item_pool = self.create_non_filler_items()
+
+for _ in range(total_locations - len(item_pool)):
+    item_pool.append(self.create_filler())
+
+self.multiworld.itempool += item_pool
+```
+
+A faster alternative to the `for` loop would be to use a [list comprehension](https://docs.python.org/3/tutorial/datastructures.html#list-comprehensions):
+```py
+item_pool += [self.create_filler() for _ in range(total_locations - len(item_pool))]
+```

kvui.py

@@ -595,8 +595,9 @@ def command_button_action(self, button):
                                              "!help for server commands.")
 
     def connect_button_action(self, button):
+        self.ctx.username = None
+        self.ctx.password = None
         if self.ctx.server:
-            self.ctx.username = None
             async_start(self.ctx.disconnect())
         else:
             async_start(self.ctx.connect(self.server_connect_bar.text.replace("/connect ", "")))
@@ -836,6 +837,10 @@ def _handle_color(self, node: JSONMessagePart):
         return self._handle_text(node)
 
     def _handle_text(self, node: JSONMessagePart):
+        # All other text goes through _handle_color, and we don't want to escape markup twice,
+        # or mess up text that already has intentional markup applied to it
+        if node.get("type", "text") == "text":
+            node["text"] = escape_markup(node["text"])
         for ref in node.get("refs", []):
             node["text"] = f"[ref={self.ref_count}|{ref}]{node['text']}[/ref]"
             self.ref_count += 1

settings.py

@@ -3,6 +3,7 @@
 This is different from player options.
 """
 
+import os
 import os.path
 import shutil
 import sys
@@ -11,7 +12,6 @@
 from enum import IntEnum
 from threading import Lock
 from typing import cast, Any, BinaryIO, ClassVar, Dict, Iterator, List, Optional, TextIO, Tuple, Union, TypeVar
-import os
 
 __all__ = [
     "get_settings", "fmt_doc", "no_gui",
@@ -798,6 +798,7 @@ def autosave() -> None:
             atexit.register(autosave)
 
     def save(self, location: Optional[str] = None) -> None:  # as above
+        from Utils import parse_yaml
         location = location or self._filename
         assert location, "No file specified"
         temp_location = location + ".tmp"  # not using tempfile to test expected file access
@@ -807,10 +808,18 @@ def save(self, location: Optional[str] = None) -> None:  # as above
         # can't use utf-8-sig because it breaks backward compat: pyyaml on Windows with bytes does not strip the BOM
         with open(temp_location, "w", encoding="utf-8") as f:
             self.dump(f)
-        # replace old with new
-        if os.path.exists(location):
+            f.flush()
+            if hasattr(os, "fsync"):
+                os.fsync(f.fileno())
+        # validate new file is valid yaml
+        with open(temp_location, encoding="utf-8") as f:
+            parse_yaml(f.read())
+        # replace old with new, try atomic operation first
+        try:
+            os.rename(temp_location, location)
+        except (OSError, FileExistsError):
             os.unlink(location)
-        os.rename(temp_location, location)
+            os.rename(temp_location, location)
         self._filename = location
 
     def dump(self, f: TextIO, level: int = 0) -> None:
@@ -832,7 +841,6 @@ def get_settings() -> Settings:
     with _lock:  # make sure we only have one instance
         res = getattr(get_settings, "_cache", None)
         if not res:
-            import os
             from Utils import user_path, local_path
             filenames = ("options.yaml", "host.yaml")
             locations: List[str] = []

setup.py

@@ -21,7 +21,7 @@
 
 # This is a bit jank. We need cx-Freeze to be able to run anything from this script, so install it
 try:
-    requirement = 'cx-Freeze==7.0.0'
+    requirement = 'cx-Freeze==7.2.0'
     import pkg_resources
     try:
         pkg_resources.require(requirement)

test/general/test_host_yaml.py

@@ -1,11 +1,12 @@
 import os
+import os.path
 import unittest
 from io import StringIO
-from tempfile import TemporaryFile
+from tempfile import TemporaryDirectory, TemporaryFile
 from typing import Any, Dict, List, cast
 
 import Utils
-from settings import Settings, Group
+from settings import Group, Settings, ServerOptions
 
 
 class TestIDs(unittest.TestCase):
@@ -80,3 +81,27 @@ class AGroup(Group):
                 self.assertEqual(value_spaces[2], value_spaces[0])  # start of sub-list
                 self.assertGreater(value_spaces[3], value_spaces[0],
                                    f"{value_lines[3]} should have more indentation than {value_lines[0]} in {lines}")
+
+
+class TestSettingsSave(unittest.TestCase):
+    def test_save(self) -> None:
+        """Test that saving and updating works"""
+        with TemporaryDirectory() as d:
+            filename = os.path.join(d, "host.yaml")
+            new_release_mode = ServerOptions.ReleaseMode("enabled")
+            # create default host.yaml
+            settings = Settings(None)
+            settings.save(filename)
+            self.assertTrue(os.path.exists(filename),
+                            "Default settings could not be saved")
+            self.assertNotEqual(settings.server_options.release_mode, new_release_mode,
+                                "Unexpected default release mode")
+            # update host.yaml
+            settings.server_options.release_mode = new_release_mode
+            settings.save(filename)
+            self.assertFalse(os.path.exists(filename + ".tmp"),
+                             "Temp file was not removed during save")
+            # read back host.yaml
+            settings = Settings(filename)
+            self.assertEqual(settings.server_options.release_mode, new_release_mode,
+                             "Settings were not overwritten")