From 88efb5156c38211ab080e2138e8d7525813bfa48 Mon Sep 17 00:00:00 2001 From: Sami Laine Date: Wed, 23 Apr 2025 15:57:04 +0000 Subject: [PATCH 1/3] Autocomplete mission filenames in /zeus-upload --- CHANGELOG.md | 4 ++++ src/zeusops_bot/cogs/zeus_upload.py | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5017069..870bbde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ The project uses semantic versioning (see [semver](https://semver.org)). ## [Unreleased] +### Added + +- `/zeus-upload` now autocompletes existing mission filenames. + ## v0.6.0 - 2025-04-06 ### Added diff --git a/src/zeusops_bot/cogs/zeus_upload.py b/src/zeusops_bot/cogs/zeus_upload.py index 2f27edd..39a13b3 100644 --- a/src/zeusops_bot/cogs/zeus_upload.py +++ b/src/zeusops_bot/cogs/zeus_upload.py @@ -22,7 +22,8 @@ def _autocomplete_missions(ctx: discord.AutocompleteContext) -> list[str]: """List known missions - Used to populate the autocomplete list in /zeus-set-mission. + Used to populate the autocomplete list in /zeus-set-mission and + /zeus-upload. TODO: Return list[discord.OptionChoice] instead? """ @@ -42,6 +43,13 @@ def __init__(self, bot: "ZeusopsBot", reforger_confgen: ReforgerConfigGenerator) self.reforger_confgen = reforger_confgen @commands.slash_command(name="zeus-upload") + @discord.option( + "filename", + description=( + "Mission filename. Choose an existing filename to update the mission." + ), + autocomplete=_autocomplete_missions, + ) @discord.option( "modlist", description="Modlist JSON exported from Reforger", From 437b2ed897ec4480eae5d1e8fbc8e9bcf2f97b87 Mon Sep 17 00:00:00 2001 From: Sami Laine Date: Wed, 23 Apr 2025 17:46:59 +0000 Subject: [PATCH 2/3] Feature and xfail test for scenarioId autodiscovery --- features/reforger_upload.feature | 12 ++++++ src/zeusops_bot/cli.py | 4 +- src/zeusops_bot/cogs/zeus_upload.py | 4 +- src/zeusops_bot/reforger_config_gen.py | 12 ++++-- tests/fixtures.py | 4 +- tests/test_upload_mission.py | 54 ++++++++++++++++++++++---- 6 files changed, 72 insertions(+), 18 deletions(-) diff --git a/features/reforger_upload.feature b/features/reforger_upload.feature index 1a27ac9..490ecc3 100644 --- a/features/reforger_upload.feature +++ b/features/reforger_upload.feature @@ -33,3 +33,15 @@ Scenario: Upload next mission without modlist And Zeus specifies , Then a new server config file is created And the config file is patched with just + +Scenario: Update modlist of an existing mission + Given a Zeusops mission uploaded previously + When Zeus calls "/zeus-upload" with an existing filename + And Zeus specifies + Then the scenario ID is deduced automatically from the existing config + And the existing config is updated with the new mods. + +Scenario: Not providing a scenario ID for a new mission produces an error + When Zeus calls "/zeus-upload" with a new filename + And Zeus does not specify + Then an error about a missing mission is raised. diff --git a/src/zeusops_bot/cli.py b/src/zeusops_bot/cli.py index 4fd0a5f..ce84b35 100644 --- a/src/zeusops_bot/cli.py +++ b/src/zeusops_bot/cli.py @@ -76,8 +76,8 @@ def reforger_upload( base_config_file: Path, target_folder: Path, modlist: list[ModDetail] | None, - scenario_id: str, filename: str, + scenario_id: str | None = None, ): """Run the program's /zeus-upload command""" conf_generator = cmd.ReforgerConfigGenerator(base_config_file, target_folder) @@ -85,6 +85,6 @@ def reforger_upload( print(f"Loading {len(modlist)} mods, for {scenario_id=}...") else: print(f"Loading {scenario_id=}...") - out_path = conf_generator.zeus_upload(scenario_id, filename, modlist) + out_path = conf_generator.zeus_upload(filename, scenario_id, modlist) print(f"Saved under file {out_path.name}") return Exit.SUCCESS diff --git a/src/zeusops_bot/cogs/zeus_upload.py b/src/zeusops_bot/cogs/zeus_upload.py index 39a13b3..80fa622 100644 --- a/src/zeusops_bot/cogs/zeus_upload.py +++ b/src/zeusops_bot/cogs/zeus_upload.py @@ -71,8 +71,8 @@ def __init__(self, bot: "ZeusopsBot", reforger_confgen: ReforgerConfigGenerator) async def zeus_upload( self, ctx: discord.ApplicationContext, - scenario_id: str, filename: str, + scenario_id: str, modlist: discord.Attachment | None = None, activate: bool = False, keep_versions: bool = True, @@ -104,7 +104,7 @@ async def zeus_upload( return try: path = self.reforger_confgen.zeus_upload( - scenario_id, filename, modlist=extracted_mods, activate=activate + filename, scenario_id, modlist=extracted_mods, activate=activate ) except ConfigFileNotFound: await ctx.respond( diff --git a/src/zeusops_bot/reforger_config_gen.py b/src/zeusops_bot/reforger_config_gen.py index 2d7b0fa..7002358 100644 --- a/src/zeusops_bot/reforger_config_gen.py +++ b/src/zeusops_bot/reforger_config_gen.py @@ -35,19 +35,21 @@ def __init__(self, base_config_file: Path, target_folder: Path): def zeus_upload( self, - scenario_id: str, filename: str, - modlist: list[ModDetail] | None, + scenario_id: str | None = None, + modlist: list[ModDetail] | None = None, activate: bool = False, ) -> Path: """Convert a modlist+scenario into a file on server at given path Args: - scenario_id: The scenarioID to load within the modlist (selects mission) filename: The filename to store the resulting file under + scenario_id: The scenarioID to load within the modlist (selects + mission). If set to None, will be automatically fetched from the + file based on the filename. modlist: The exhaustive list of mods to load, or None to mean no change needed activate: If set to true, the added config is immediately set as the - currently active config + currently active config Returns: Path: Path to the file generated on filesystem, under {py:attr}`target_folder` @@ -58,6 +60,8 @@ def zeus_upload( ConfigPatchingError: Patching of the file failed, sending lib exception as arg """ + if scenario_id is None: + raise NotImplementedError # Ensure parent folder exists self.target_dest.mkdir(parents=True, exist_ok=True) # Read the original config file diff --git a/tests/fixtures.py b/tests/fixtures.py index 8208d4e..cce6aad 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -16,13 +16,13 @@ } } -MODLIST_DICT: list[ModDetail] = [ +MODLIST_DICTS: list[ModDetail] = [ {"modId": "595F2BF2F44836FB", "name": "RHS - Status Quo", "version": "0.10.4075"}, {"modId": "5EB744C5F42E0800", "name": "ACE Chopping", "version": "1.2.0"}, {"modId": "60EAEA0389DB3CC2", "name": "ACE Trenches", "version": "1.2.0"}, ] -MODLIST_DICT_VERSIONLESS: list[ModDetail] = [ +MODLIST_DICTS_VERSIONLESS: list[ModDetail] = [ {"modId": "595F2BF2F44836FB", "name": "RHS - Status Quo"}, {"modId": "5EB744C5F42E0800", "name": "ACE Chopping"}, {"modId": "60EAEA0389DB3CC2", "name": "ACE Trenches"}, diff --git a/tests/test_upload_mission.py b/tests/test_upload_mission.py index b42c1ef..8b8d88b 100644 --- a/tests/test_upload_mission.py +++ b/tests/test_upload_mission.py @@ -13,17 +13,18 @@ from tests.fixtures import ( BASE_CONFIG, - MODLIST_DICT, - MODLIST_DICT_VERSIONLESS, + MODLIST_DICTS, + MODLIST_DICTS_VERSIONLESS, MODLIST_JSON, ) +from zeusops_bot.errors import ConfigFileNotFound from zeusops_bot.models import ModDetail from zeusops_bot.reforger_config_gen import ReforgerConfigGenerator, extract_mods @pytest.mark.parametrize( "keep_versions,mods", - [(False, MODLIST_DICT_VERSIONLESS), (True, MODLIST_DICT)], + [(False, MODLIST_DICTS_VERSIONLESS), (True, MODLIST_DICTS)], ids=["strip versions", "keep versions"], ) @pytest.mark.parametrize("activate", (False, True), ids=["no activate", "activate"]) @@ -44,7 +45,7 @@ def test_upload_edits_files( ) # When Zeus calls "/zeus-upload" modlist = extract_mods(MODLIST_JSON, keep_versions) - out_path = config_gen.zeus_upload(scenario_id, filename, modlist, activate) + out_path = config_gen.zeus_upload(filename, scenario_id, modlist, activate) # Then a new server config file is created assert out_path.is_file(), "Should have generated a file on disk" # And the config file is patched with and @@ -63,7 +64,7 @@ def test_upload_activate_mission(base_config: Path, mission_dir: Path): ) modlist = extract_mods(MODLIST_JSON) out_path = config_gen.zeus_upload( - "cool-scenario-1", "Jib_20250228", modlist, activate=True + "Jib_20250228", "cool-scenario-1", modlist, activate=True ) # Then the server config file is set as the active mission target = mission_dir / "current-config.json" @@ -82,9 +83,7 @@ def test_upload_edits_files_without_modlist(base_config: Path, mission_dir: Path base_config_file=base_config, target_folder=mission_dir ) # When Zeus calls "/zeus-upload" - out_path = config_gen.zeus_upload( - scenario_id=scenario_id, filename=filename, modlist=None - ) + out_path = config_gen.zeus_upload(filename, scenario_id, modlist=None) # Then a new server config file is created assert out_path.is_file(), "Should have generated a file on disk" # And the config file is patched with just @@ -92,3 +91,42 @@ def test_upload_edits_files_without_modlist(base_config: Path, mission_dir: Path assert config["game"]["scenarioId"] == scenario_id, "Should update scenarioId" assert isinstance(config["game"]["mods"], list) assert config["game"]["mods"] == BASE_CONFIG["game"]["mods"], "Should keep modlist" + + +@pytest.mark.xfail(raises=NotImplementedError) +def test_upload_existing_filename_without_scenarioid( + base_config: Path, mission_dir: Path +): + """Scenario: Update modlist of an existing mission""" + # Given a Zeusops mission uploaded previously + scenario_id = "cool-scenario-1" + filename = "Jib_20250228" + config_gen = ReforgerConfigGenerator( + base_config_file=base_config, target_folder=mission_dir + ) + # When Zeus calls "/zeus-upload" + modlist = MODLIST_DICTS[0:-1] + out_path = config_gen.zeus_upload(filename, scenario_id, modlist) + # When Zeus calls "/zeus-upload" with an existing filename + # And Zeus specifies + out_path = config_gen.zeus_upload(filename, scenario_id=None, modlist=MODLIST_DICTS) + # Then the scenario ID is deduced automatically from the existing config + config = json.loads(out_path.read_text()) + assert config["game"]["scenarioId"] == scenario_id, "Should use existing scenarioId" + # And the existing config is updated with the new mods. + assert isinstance(config["game"]["mods"], list) + assert config["game"]["mods"] == MODLIST_DICTS, "Should update modlist" + + +@pytest.mark.xfail(raises=NotImplementedError) +def test_upload_no_scenarioid_without_file_fails(base_config: Path, mission_dir: Path): + """Scenario: Not providing a scenario ID for a new mission produces an error""" + # When Zeus calls "/zeus-upload" with a new filename + # And Zeus does not specify + filename = "Jib_20250228" + config_gen = ReforgerConfigGenerator( + base_config_file=base_config, target_folder=mission_dir + ) + # Then an error about a missing mission is raised. + with pytest.raises(ConfigFileNotFound): + config_gen.zeus_upload(filename, scenario_id=None, modlist=MODLIST_DICTS) From 5bb7fdd20bf83a29ebe1c25b62fd8b63e31523fc Mon Sep 17 00:00:00 2001 From: Sami Laine Date: Wed, 7 May 2025 20:35:22 +0000 Subject: [PATCH 3/3] Implement scenarioId autodiscovery Automatically finds scenarioId when updating an existing mission. --- CHANGELOG.md | 3 + src/zeusops_bot/cogs/zeus_upload.py | 18 +++-- src/zeusops_bot/models.py | 22 +++++ src/zeusops_bot/reforger_config_gen.py | 107 +++++++++++++++++++------ tests/test_set_mission.py | 12 +-- tests/test_upload_mission.py | 8 +- 6 files changed, 127 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 870bbde..753758c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ The project uses semantic versioning (see [semver](https://semver.org)). ### Added - `/zeus-upload` now autocompletes existing mission filenames. +- When updating an existing mission with `/zeus-upload`, the `scenario_id` + parameter is optional and will be automatically deduced from the existing + mission config. ## v0.6.0 - 2025-04-06 diff --git a/src/zeusops_bot/cogs/zeus_upload.py b/src/zeusops_bot/cogs/zeus_upload.py index 80fa622..86690b0 100644 --- a/src/zeusops_bot/cogs/zeus_upload.py +++ b/src/zeusops_bot/cogs/zeus_upload.py @@ -50,6 +50,15 @@ def __init__(self, bot: "ZeusopsBot", reforger_confgen: ReforgerConfigGenerator) ), autocomplete=_autocomplete_missions, ) + @discord.option( + "scenario_id", + description=( + "Workshop ID for the scenario. Not required " + "updating an existing mission config" + ), + input_type=str, + required=False, + ) @discord.option( "modlist", description="Modlist JSON exported from Reforger", @@ -72,7 +81,7 @@ async def zeus_upload( self, ctx: discord.ApplicationContext, filename: str, - scenario_id: str, + scenario_id: str | None = None, modlist: discord.Attachment | None = None, activate: bool = False, keep_versions: bool = True, @@ -106,11 +115,8 @@ async def zeus_upload( path = self.reforger_confgen.zeus_upload( filename, scenario_id, modlist=extracted_mods, activate=activate ) - except ConfigFileNotFound: - await ctx.respond( - "Bot config error: the base config file could not be found" - f" Tell the Techmins! Path was: {self.reforger_confgen.base_config}" - ) + except ConfigFileNotFound as e: + await ctx.respond(f"Config file not found. Error was: \n ```\n{e}\n```") return except ConfigFileInvalidJson as e: await ctx.respond( diff --git a/src/zeusops_bot/models.py b/src/zeusops_bot/models.py index 36fdb6a..be2d5e0 100644 --- a/src/zeusops_bot/models.py +++ b/src/zeusops_bot/models.py @@ -2,6 +2,8 @@ from typing import NotRequired, TypedDict +from pydantic import TypeAdapter + class ModDetail(TypedDict): """A single mod's details @@ -26,3 +28,23 @@ class ConfigFile(TypedDict): """The reforger config file""" game: ConfigFileGameSection + + +class TypedTypeAdapter[T]: + """A wrapper around Pydantic's TypeAdapter to make mypy happy + + This class defines basically the same types as the actual TypeAdapter, but + somehow mypy doesn't complain when using this one. + """ + + def __init__(self, type: type[T], **kwargs) -> None: + """Typed init""" + self.adapter = TypeAdapter(type, *kwargs) + + def validate_json(self, data: str | bytes | bytearray, /, **kwargs) -> T: + """Typed method""" + return self.adapter.validate_json(data, *kwargs) + + +modlist_typeadapter = TypedTypeAdapter(list[ModDetail]) +configfile_typeadapter = TypedTypeAdapter(ConfigFile) diff --git a/src/zeusops_bot/reforger_config_gen.py b/src/zeusops_bot/reforger_config_gen.py index 7002358..c88f2d7 100644 --- a/src/zeusops_bot/reforger_config_gen.py +++ b/src/zeusops_bot/reforger_config_gen.py @@ -4,13 +4,17 @@ from pathlib import Path import jsonpatch -from pydantic import TypeAdapter, ValidationError +from pydantic import ValidationError from zeusops_bot import errors from zeusops_bot.errors import ConfigFileInvalidJson -from zeusops_bot.models import ModDetail - -modlist_typeadapter = TypeAdapter(list["ModDetail"]) +from zeusops_bot.models import ( + ConfigFile, + ModDetail, + TypedTypeAdapter, + configfile_typeadapter, + modlist_typeadapter, +) SYMLINK_FILENAME = "current-config.json" @@ -55,24 +59,34 @@ def zeus_upload( Path: Path to the file generated on filesystem, under {py:attr}`target_folder` Raises: - ConfigFileNotFound: Base config file not found at all + ConfigFileNotFound: Base config file not found at all or trying to + update a non-existent mission config file ConfigFileInvalidJson: Base config file doesn't decode as valid JSON ConfigPatchingError: Patching of the file failed, sending lib exception as arg """ - if scenario_id is None: - raise NotImplementedError + target_filepath = as_config_file(self.target_dest, filename) + if scenario_id is None and not target_filepath.is_file(): + raise errors.ConfigFileNotFound( + "Cannot detect scenario ID automatically: mission config " + f"{target_filepath} does not exist" + ) + # Ensure parent folder exists self.target_dest.mkdir(parents=True, exist_ok=True) # Read the original config file - if not self.base_config.is_file(): - raise errors.ConfigFileNotFound(self.base_config) try: - base_config_content = json.loads(self.base_config.read_text()) - except json.JSONDecodeError as e: - raise errors.ConfigFileInvalidJson(e) + base_config_content = load_config(self.base_config) + except errors.ConfigFileNotFound as e: + raise errors.ConfigFileNotFound( + "Bot config error: the base config file could not be found. " + f"Tell the Techmins! Path was: {self.base_config}" + ) from e + + if scenario_id is None: + data = load_config(target_filepath) + scenario_id = data["game"]["scenarioId"] modded_config_dict = patch_file(base_config_content, modlist, scenario_id) - target_filepath = as_config_file(self.target_dest, filename) # Create the file itself target_filepath.write_text(json.dumps(modded_config_dict, indent=4)) if activate: @@ -137,7 +151,9 @@ def current_mission(self) -> str: return target.stem -def patch_file(source: dict, modlist: list[ModDetail] | None, scenario_id: str) -> dict: +def patch_file( + source: ConfigFile, modlist: list[ModDetail] | None, scenario_id: str +) -> dict: """Edit the content of source with new modlist and scenarioID >>> modlist=[{"modId": "1", "name":"mod1"}] @@ -162,27 +178,22 @@ def patch_file(source: dict, modlist: list[ModDetail] | None, scenario_id: str) return mod -def extract_mods(modlist: str | None, keep_versions=False) -> list[ModDetail] | None: - """Extracts a list of ModDetail entries from a mod list exported from Reforger +def load_json[T](adapter: TypedTypeAdapter[T], data: str) -> T: + """Load and validate data from JSON Args: - modlist: A partial JSON string of mods to extract. Can be None if the - user did not provide any mods, in which case the operation is a - no-op. - keep_versions: Preserve mod versions in the extracted list of mods. + adapter: A TypeAdapter for loading the data + data: A JSON string containing the data to be loaded Raises: - pydantic.ValidationError: Generic error in validating the mod list + pydantic.ValidationError: Generic error in validating the data ConfigFileInvalidJson: The input was not valid JSON Returns: - A list of ModDetail, or None if the input was None. + Loaded data, as described by the TypeAdapter """ - if modlist is None: - return None - modlist = f"[{modlist}]" try: - mods = modlist_typeadapter.validate_json(modlist) + return adapter.validate_json(data) except ValidationError as e: errors = e.errors() if len(errors) > 1: @@ -196,8 +207,52 @@ def extract_mods(modlist: str | None, keep_versions=False) -> list[ModDetail] | # Unknown error raise e + +def extract_mods(modlist: str | None, keep_versions=False) -> list[ModDetail] | None: + """Extract a list of ModDetail entries from a mod list exported from Reforger + + Args: + modlist: A partial JSON string of mods to extract. Can be None if the + user did not provide any mods, in which case the operation is a + no-op. + keep_versions: Preserve mod versions in the extracted list of mods. + + Raises: + pydantic.ValidationError: Generic error in validating the mod list + ConfigFileInvalidJson: The input was not valid JSON + + Returns: + A list of ModDetail, or None if the input was None. + """ + if modlist is None: + return None + modlist = f"[{modlist}]" + mods = load_json(modlist_typeadapter, modlist) + if not keep_versions: for mod in mods: del mod["version"] return mods + + +def load_config(config: Path | str) -> ConfigFile: + """Load a ConfigFile from JSON + + Args: + config: Either a path to the config or a string containing the config to + be loaded. + + Raises: + pydantic.ValidationError: Generic error in validating the mod list + ConfigFileInvalidJson: The input was not valid JSON + ConfigFileNotFound: The config file could not be found + + Returns: + The loaded ConfigFile. + """ + if isinstance(config, Path): + if not config.is_file(): + raise errors.ConfigFileNotFound(config) + config = config.read_text() + return load_json(configfile_typeadapter, config) diff --git a/tests/test_set_mission.py b/tests/test_set_mission.py index a3e42b0..930cb5d 100644 --- a/tests/test_set_mission.py +++ b/tests/test_set_mission.py @@ -22,9 +22,9 @@ def test_load_mission(base_config: Path, mission_dir: Path): target = mission_dir / "current-config.json" assert target.exists(), "Should have created latest config symlink" assert target.is_symlink(), "Target config should be a symlink" - assert target.readlink() == uploaded_conf_path.relative_to( - mission_dir - ), "Target should point to uploaded file" + assert target.readlink() == uploaded_conf_path.relative_to(mission_dir), ( + "Target should point to uploaded file" + ) def test_load_mission_twice(base_config: Path, mission_dir: Path): @@ -48,6 +48,6 @@ def test_load_mission_twice(base_config: Path, mission_dir: Path): target = mission_dir / "current-config.json" assert target.exists(), "Should have created latest config symlink" assert target.is_symlink(), "Target config should be a symlink" - assert target.readlink() == uploaded_conf_path2.relative_to( - mission_dir - ), "Target should point to latest mission set" + assert target.readlink() == uploaded_conf_path2.relative_to(mission_dir), ( + "Target should point to latest mission set" + ) diff --git a/tests/test_upload_mission.py b/tests/test_upload_mission.py index 8b8d88b..b452afb 100644 --- a/tests/test_upload_mission.py +++ b/tests/test_upload_mission.py @@ -68,9 +68,9 @@ def test_upload_activate_mission(base_config: Path, mission_dir: Path): ) # Then the server config file is set as the active mission target = mission_dir / "current-config.json" - assert target.readlink() == out_path.relative_to( - mission_dir - ), "Target should point to uploaded file" + assert target.readlink() == out_path.relative_to(mission_dir), ( + "Target should point to uploaded file" + ) def test_upload_edits_files_without_modlist(base_config: Path, mission_dir: Path): @@ -93,7 +93,6 @@ def test_upload_edits_files_without_modlist(base_config: Path, mission_dir: Path assert config["game"]["mods"] == BASE_CONFIG["game"]["mods"], "Should keep modlist" -@pytest.mark.xfail(raises=NotImplementedError) def test_upload_existing_filename_without_scenarioid( base_config: Path, mission_dir: Path ): @@ -118,7 +117,6 @@ def test_upload_existing_filename_without_scenarioid( assert config["game"]["mods"] == MODLIST_DICTS, "Should update modlist" -@pytest.mark.xfail(raises=NotImplementedError) def test_upload_no_scenarioid_without_file_fails(base_config: Path, mission_dir: Path): """Scenario: Not providing a scenario ID for a new mission produces an error""" # When Zeus calls "/zeus-upload" with a new filename