From 7634d2f2a6669636a784e64fbfa11b0b9be6546b Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:41:48 -0600 Subject: [PATCH 1/9] bg3: XML-escape mod values in modsettings.lsx Names with & < or " produced an invalid modsettings.lsx. --- games/baldursgate3/bg3_utils.py | 5 +++++ tests/test_bg3_utils.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/test_bg3_utils.py diff --git a/games/baldursgate3/bg3_utils.py b/games/baldursgate3/bg3_utils.py index 79c8a7ce..8ab6dbcd 100644 --- a/games/baldursgate3/bg3_utils.py +++ b/games/baldursgate3/bg3_utils.py @@ -3,6 +3,7 @@ import typing from pathlib import Path from time import sleep +from xml.sax.saxutils import escape from PyQt6.QtCore import ( QCoreApplication, @@ -36,6 +37,10 @@ def get_node_string( uuid: str = "", version64: str = "0", ) -> str: + folder, md5, name, publish_handle, uuid, version64 = ( + escape(v, {'"': """}) + for v in (folder, md5, name, publish_handle, uuid, version64) + ) return f""" diff --git a/tests/test_bg3_utils.py b/tests/test_bg3_utils.py new file mode 100644 index 00000000..54224415 --- /dev/null +++ b/tests/test_bg3_utils.py @@ -0,0 +1,19 @@ +import sys +import unittest +from unittest.mock import MagicMock +from xml.etree import ElementTree + +sys.modules.setdefault("mobase", MagicMock()) + +from games.baldursgate3 import bg3_utils # noqa: E402 + + +class NodeStringTest(unittest.TestCase): + def test_special_characters_round_trip(self): + name = """Tom & Jerry's <"Best"> Mod""" + node = bg3_utils.get_node_string(folder=name, name=name, uuid="u") + attrs = { + a.get("id"): a.get("value") + for a in ElementTree.fromstring(node).iter("attribute") + } + self.assertEqual((attrs["Folder"], attrs["Name"]), (name, name)) From 771684a45964793a8e1126419181919edf27f6b2 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:43:12 -0600 Subject: [PATCH 2/9] bg3: fix LSLib re-extraction and harden its download Extraction overwrites existing files and keeps Packed/Tools subfolders. Download uses a timeout, picks the .zip asset, checks GitHub's sha256 digest and always closes its progress dialog. --- games/baldursgate3/lslib_retriever.py | 201 +++++++++++++------------- tests/test_lslib_retriever.py | 31 ++++ 2 files changed, 135 insertions(+), 97 deletions(-) create mode 100644 tests/test_lslib_retriever.py diff --git a/games/baldursgate3/lslib_retriever.py b/games/baldursgate3/lslib_retriever.py index 4c3d797c..259402c9 100644 --- a/games/baldursgate3/lslib_retriever.py +++ b/games/baldursgate3/lslib_retriever.py @@ -1,15 +1,19 @@ +import hashlib import json import shutil import traceback import urllib.request import zipfile from functools import cached_property +from pathlib import Path from PyQt6.QtCore import qDebug, qWarning from PyQt6.QtWidgets import QApplication, QMessageBox from . import bg3_utils +_TIMEOUT = 30 + class LSLibRetriever: def __init__(self, utils: bg3_utils.BG3Utils): @@ -31,78 +35,84 @@ def _needed_lslib_files(self): def download_lslib_if_missing(self, force: bool = False) -> bool: if not force and all(x.exists() for x in self._needed_lslib_files): return True + old_archives = sorted(self._utils.tools_dir.glob("*.zip")) try: self._utils.tools_dir.mkdir(exist_ok=True, parents=True) - downloaded = False - - def reporthook(block_num: int, block_size: int, total_size: int) -> None: - if total_size > 0: - progress.setValue( - min(int(block_num * block_size * 100 / total_size), 100) - ) - QApplication.processEvents() - with urllib.request.urlopen( - "https://api.github.com/repos/Norbyte/lslib/releases/latest" + "https://api.github.com/repos/Norbyte/lslib/releases/latest", + timeout=_TIMEOUT, ) as response: - assets = json.loads(response.read().decode("utf-8"))["assets"][0] - zip_path = self._utils.tools_dir / assets["name"] - if not zip_path.exists(): - old_archives = list(self._utils.tools_dir.glob("*.zip")) - msg_box = QMessageBox(self._utils.main_window) - msg_box.setWindowTitle( - self._utils.tr("Baldur's Gate 3 Plugin - Missing dependencies") - ) - if old_archives: - msg_box.setText(self._utils.tr("LSLib update available.")) - else: - msg_box.setText( - self._utils.tr( - "LSLib tools are missing.\nThese are necessary for the plugin to create the load order file for BG3." - ) - ) - msg_box.addButton( - self._utils.tr("Download"), - QMessageBox.ButtonRole.DestructiveRole, - ) - exit_btn = msg_box.addButton( - self._utils.tr("Exit"), QMessageBox.ButtonRole.ActionRole - ) - msg_box.setIcon(QMessageBox.Icon.Warning) - msg_box.exec() - - if msg_box.clickedButton() == exit_btn: - if not old_archives: - err = QMessageBox(self._utils.main_window) - err.setIcon(QMessageBox.Icon.Critical) - err.setText( - "LSLib tools are required for the proper generation of the modsettings.xml file, file will not be generated" - ) - err.exec() - return False - else: - progress = self._utils.create_progress_window( - "Downloading LSLib", 100, cancelable=False - ) - urllib.request.urlretrieve( - assets["browser_download_url"], str(zip_path), reporthook - ) - progress.close() - downloaded = True - for archive in old_archives: - archive.unlink() - old_archives = [] - else: - old_archives = [] - new_msg = QMessageBox(self._utils.main_window) - new_msg.setIcon(QMessageBox.Icon.Information) - new_msg.setText( - self._utils.tr("Latest version of LSLib already downloaded!") + assets = json.loads(response.read().decode("utf-8"))["assets"] + asset = next(a for a in assets if a["name"].endswith(".zip")) + zip_path = self._utils.tools_dir / asset["name"] + if zip_path.exists(): + new_msg = QMessageBox(self._utils.main_window) + new_msg.setIcon(QMessageBox.Icon.Information) + new_msg.setText( + self._utils.tr("Latest version of LSLib already downloaded!") + ) + new_msg.exec() + return self._extract(zip_path) + msg_box = QMessageBox(self._utils.main_window) + msg_box.setWindowTitle( + self._utils.tr("Baldur's Gate 3 Plugin - Missing dependencies") + ) + if old_archives: + msg_box.setText(self._utils.tr("LSLib update available.")) + else: + msg_box.setText( + self._utils.tr( + "LSLib tools are missing.\nThese are necessary for the plugin to create the load order file for BG3." ) - new_msg.exec() - - except Exception as e: - qDebug(f"Download failed: {e}") + ) + msg_box.addButton( + self._utils.tr("Download"), QMessageBox.ButtonRole.DestructiveRole + ) + exit_btn = msg_box.addButton( + self._utils.tr("Exit"), QMessageBox.ButtonRole.ActionRole + ) + msg_box.setIcon(QMessageBox.Icon.Warning) + msg_box.exec() + if msg_box.clickedButton() == exit_btn: + if old_archives: + return self._extract(old_archives[-1]) + err = QMessageBox(self._utils.main_window) + err.setIcon(QMessageBox.Icon.Critical) + err.setText( + "LSLib tools are required for the proper generation of the modsettings.xml file, file will not be generated" + ) + err.exec() + return False + progress = self._utils.create_progress_window( + "Downloading LSLib", asset["size"], cancelable=False + ) + digest = hashlib.sha256() + part_path = zip_path.with_suffix(".part") + try: + with ( + urllib.request.urlopen( + asset["browser_download_url"], timeout=_TIMEOUT + ) as src, + part_path.open("wb") as dst, + ): + while chunk := src.read(1 << 16): + dst.write(chunk) + digest.update(chunk) + progress.setValue(progress.value() + len(chunk)) + QApplication.processEvents() + finally: + progress.close() + expected = asset.get("digest") or "" + if expected and expected != f"sha256:{digest.hexdigest()}": + part_path.unlink() + raise ValueError( + f"{asset['name']} digest mismatch, expected {expected}" + ) + part_path.replace(zip_path) + for archive in old_archives: + archive.unlink() + except Exception: + qDebug(f"Download failed: {traceback.format_exc()}") err = QMessageBox(self._utils.main_window) err.setIcon(QMessageBox.Icon.Critical) err.setText( @@ -112,40 +122,37 @@ def reporthook(block_num: int, block_size: int, total_size: int) -> None: ) err.exec() return False + return self._extract(zip_path) + + def _extract(self, zip_path: Path) -> bool: try: - if old_archives: - zip_path = sorted(old_archives)[-1] - if old_archives or not downloaded: - dialog_message = "Ensuring all necessary LSLib files have been extracted from archive..." - win_title = "Verifying LSLib files" - else: - dialog_message = "Extracting/Updating LSLib files..." - win_title = "Extracting LSLib" with zipfile.ZipFile(zip_path, "r") as zip_ref: - names = list( - filter( - lambda name: name.startswith("Packed/Tools"), zip_ref.namelist() - ) - ) + names = [ + n + for n in zip_ref.namelist() + if n.startswith("Packed/Tools/") and not n.endswith("/") + ] qDebug(f"found files: {','.join(names)}") - x_progress = self._utils.create_progress_window( - win_title, len(names), msg=dialog_message + progress = self._utils.create_progress_window( + "Extracting LSLib", len(names), msg="Extracting LSLib files..." ) - for file in names: - shutil.move( - zip_ref.extract(file, self._utils.tools_dir), - self._utils.tools_dir, - ) - x_progress.setValue(x_progress.value() + 1) - QApplication.processEvents() - if x_progress.wasCanceled(): - x_progress.close() - qWarning("processing canceled by user") - return False - x_progress.close() - shutil.rmtree(self._utils.tools_dir / "Packed", ignore_errors=True) - except Exception as e: - qDebug(f"Extraction failed: {e}") + try: + for name in names: + target = self._utils.tools_dir / Path(name).relative_to( + "Packed/Tools" + ) + target.parent.mkdir(parents=True, exist_ok=True) + with zip_ref.open(name) as src, target.open("wb") as dst: + shutil.copyfileobj(src, dst) + progress.setValue(progress.value() + 1) + QApplication.processEvents() + if progress.wasCanceled(): + qWarning("processing canceled by user") + return False + finally: + progress.close() + except Exception: + qDebug(f"Extraction failed: {traceback.format_exc()}") err = QMessageBox(self._utils.main_window) err.setIcon(QMessageBox.Icon.Critical) err.setText( diff --git a/tests/test_lslib_retriever.py b/tests/test_lslib_retriever.py new file mode 100644 index 00000000..5ab83f16 --- /dev/null +++ b/tests/test_lslib_retriever.py @@ -0,0 +1,31 @@ +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path +from unittest.mock import MagicMock + +sys.modules.setdefault("mobase", MagicMock()) + +from games.baldursgate3 import lslib_retriever # noqa: E402 + + +class ExtractTest(unittest.TestCase): + def test_overwrites_existing_files_and_keeps_subfolders(self): + with tempfile.TemporaryDirectory() as tmp: + tools = Path(tmp) + (tools / "Divine.exe").write_bytes(b"old") + zip_path = tools / "lslib.zip" + with zipfile.ZipFile(zip_path, "w") as z: + z.writestr("Packed/Tools/", b"") + z.writestr("Packed/Tools/Divine.exe", b"new") + z.writestr("Packed/Tools/runtimes/win/x.dll", b"dll") + z.writestr("Packed/Other/skip.txt", b"skip") + utils = MagicMock(tools_dir=tools) + utils.create_progress_window.return_value.wasCanceled.return_value = False + retriever = lslib_retriever.LSLibRetriever(utils) + self.assertTrue(retriever._extract(zip_path)) # pyright: ignore[reportPrivateUsage] + self.assertEqual((tools / "Divine.exe").read_bytes(), b"new") + self.assertEqual((tools / "runtimes/win/x.dll").read_bytes(), b"dll") + self.assertFalse((tools / "skip.txt").exists()) + self.assertFalse((tools / "Packed").exists()) From f7910f7dd7f7d4c44ce251c4005a71a3da286dd6 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:49:47 -0600 Subject: [PATCH 3/9] bg3: resolve modsettings.lsx path from the current profile A cached path kept writing the first profile's file after a switch. --- games/baldursgate3/bg3_utils.py | 2 +- tests/test_bg3_utils.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/games/baldursgate3/bg3_utils.py b/games/baldursgate3/bg3_utils.py index 8ab6dbcd..d747b3ec 100644 --- a/games/baldursgate3/bg3_utils.py +++ b/games/baldursgate3/bg3_utils.py @@ -118,7 +118,7 @@ def log_dir(self): def modsettings_backup(self): return create_dir_if_needed(self.plugin_data_path / "temp" / "modsettings.lsx") - @functools.cached_property + @property def modsettings_path(self): return create_dir_if_needed( Path(self._organizer.profilePath()) / "modsettings.lsx" diff --git a/tests/test_bg3_utils.py b/tests/test_bg3_utils.py index 54224415..b1eb975e 100644 --- a/tests/test_bg3_utils.py +++ b/tests/test_bg3_utils.py @@ -1,5 +1,7 @@ import sys +import tempfile import unittest +from pathlib import Path from unittest.mock import MagicMock from xml.etree import ElementTree @@ -17,3 +19,16 @@ def test_special_characters_round_trip(self): for a in ElementTree.fromstring(node).iter("attribute") } self.assertEqual((attrs["Folder"], attrs["Name"]), (name, name)) + + +class ProfilePathTest(unittest.TestCase): + def test_modsettings_path_follows_profile_switch(self): + with tempfile.TemporaryDirectory() as tmp: + organizer = MagicMock() + utils = bg3_utils.BG3Utils("Baldur's Gate 3 Plugin") + utils.init(organizer) + for profile in ("a", "b"): + organizer.profilePath.return_value = str(Path(tmp, profile)) + self.assertEqual( + utils.modsettings_path, Path(tmp, profile, "modsettings.lsx") + ) From 77005b2fc88534b45d2029176119782bfd55f990 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:52:52 -0600 Subject: [PATCH 4/9] bg3: force-load mod DLLs only into the game executables --- games/game_baldursgate3.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/games/game_baldursgate3.py b/games/game_baldursgate3.py index 6f1ff226..254d28aa 100644 --- a/games/game_baldursgate3.py +++ b/games/game_baldursgate3.py @@ -132,7 +132,7 @@ def executableForcedLoads(self) -> list[mobase.ExecutableForcedLoadSetting]: tree: mobase.IFileTree | mobase.FileTreeEntry | None = ( self._organizer.virtualFileTree().find("bin") ) - if type(tree) is not mobase.IFileTree: + if not isinstance(tree, mobase.IFileTree): return efls def find_dlls( @@ -148,12 +148,13 @@ def find_dlls( return mobase.IFileTree.WalkReturn.CONTINUE tree.walk(find_dlls) - exes = self.executables() + exes = {exe.binary().fileName() for exe in self.executables()} & { + "bg3.exe", + "bg3_dx11.exe", + } qDebug(f"dlls to force load: {libs}") efls = efls + [ - mobase.ExecutableForcedLoadSetting( - exe.binary().fileName(), lib - ).withEnabled(True) + mobase.ExecutableForcedLoadSetting(exe, lib).withEnabled(True) for lib in libs for exe in exes ] From 11705d3e36c0ecfdd5247757e1d4d6d211914ce1 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:54:07 -0600 Subject: [PATCH 5/9] bg3: remove nested empty folders bottom-up and keep the root --- games/baldursgate3/bg3_utils.py | 13 ++++++++++++ games/game_baldursgate3.py | 12 ++--------- tests/test_game_baldursgate3.py | 35 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 tests/test_game_baldursgate3.py diff --git a/games/baldursgate3/bg3_utils.py b/games/baldursgate3/bg3_utils.py index d747b3ec..c4c6f745 100644 --- a/games/baldursgate3/bg3_utils.py +++ b/games/baldursgate3/bg3_utils.py @@ -278,3 +278,16 @@ def create_dir_if_needed(path: Path) -> Path: else: path.parent.mkdir(parents=True, exist_ok=True) return path + + +def remove_empty_dirs(root: Path) -> set[Path]: + removed: set[Path] = set() + for folder, _, _ in root.walk(top_down=False): + if folder == root: + continue + try: + folder.rmdir() + removed.add(folder) + except OSError: + pass + return removed diff --git a/games/game_baldursgate3.py b/games/game_baldursgate3.py index 254d28aa..8ac09661 100644 --- a/games/game_baldursgate3.py +++ b/games/game_baldursgate3.py @@ -12,7 +12,7 @@ from ..basic_features import BasicGameSaveGameInfo, BasicLocalSavegames from ..basic_game import BasicGame -from .baldursgate3 import bg3_file_mapper +from .baldursgate3 import bg3_file_mapper, bg3_utils class BG3Game(BasicGame, bg3_file_mapper.BG3FileMapper): @@ -37,8 +37,6 @@ class BG3Game(BasicGame, bg3_file_mapper.BG3FileMapper): def __init__(self): BasicGame.__init__(self) - from .baldursgate3 import bg3_utils - self.utils = bg3_utils.BG3Utils(self.name()) bg3_file_mapper.BG3FileMapper.__init__( self, self.utils, self.documentsDirectory @@ -223,12 +221,6 @@ def _on_finished_run(self, exec_path: str, exit_code: int): f"cleaned the following folders due to them being older than {cutoff_time}: {removed}" ) for fdir in {self.utils.overwrite_path, self.doc_path}: - removed: set[Path] = set() - for folder in sorted(list(fdir.walk(top_down=False)))[:-1]: - try: - folder[0].rmdir() - removed.add(folder[0]) - except OSError: - pass + removed = bg3_utils.remove_empty_dirs(fdir) if cat is not None and cat.isDebugEnabled() and len(removed) > 0: qDebug(f"cleaned empty dirs from {fdir} {removed}") diff --git a/tests/test_game_baldursgate3.py b/tests/test_game_baldursgate3.py new file mode 100644 index 00000000..1d7e294b --- /dev/null +++ b/tests/test_game_baldursgate3.py @@ -0,0 +1,35 @@ +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import MagicMock + +sys.modules.setdefault("mobase", MagicMock()) + +from games.baldursgate3 import bg3_utils # noqa: E402 + + +class RemoveEmptyDirsTest(unittest.TestCase): + def test_removes_nested_empty_dirs_and_keeps_root(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "a" / "b" / "c").mkdir(parents=True) + (root / "z").mkdir() + (root / "keep").mkdir() + (root / "keep" / "f.txt").write_text("x") + removed = bg3_utils.remove_empty_dirs(root) + self.assertTrue(root.exists()) + self.assertEqual(sorted(p.name for p in root.iterdir()), ["keep"]) + self.assertEqual( + removed, + {root / "a", root / "a" / "b", root / "a" / "b" / "c", root / "z"}, + ) + + def test_keeps_empty_root(self): + with tempfile.TemporaryDirectory() as tmp: + self.assertEqual(bg3_utils.remove_empty_dirs(Path(tmp)), set()) + self.assertTrue(Path(tmp).exists()) + + +if __name__ == "__main__": + unittest.main() From e9fb57b4ffff1b4e5b3d2a4638a0cbcb7485ab6b Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:54:41 -0600 Subject: [PATCH 6/9] bg3: close diff file handles, use isinstance and explicit is_file flag --- games/baldursgate3/bg3_file_mapper.py | 2 +- games/baldursgate3/bg3_utils.py | 13 ++++++------- games/game_baldursgate3.py | 6 +++--- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/games/baldursgate3/bg3_file_mapper.py b/games/baldursgate3/bg3_file_mapper.py index 0ae6ef8b..92c97c68 100644 --- a/games/baldursgate3/bg3_file_mapper.py +++ b/games/baldursgate3/bg3_file_mapper.py @@ -119,7 +119,7 @@ def map_files( self.create_mapping(file, dest / dest_func(file)) def create_mapping(self, file: Path, dest: Path): - bg3_utils.create_dir_if_needed(dest) + bg3_utils.create_dir_if_needed(dest, is_file=not file.is_dir()) self.current_mappings.append( mobase.Mapping( diff --git a/games/baldursgate3/bg3_utils.py b/games/baldursgate3/bg3_utils.py index c4c6f745..951158b7 100644 --- a/games/baldursgate3/bg3_utils.py +++ b/games/baldursgate3/bg3_utils.py @@ -116,12 +116,14 @@ def log_dir(self): @functools.cached_property def modsettings_backup(self): - return create_dir_if_needed(self.plugin_data_path / "temp" / "modsettings.lsx") + return create_dir_if_needed( + self.plugin_data_path / "temp" / "modsettings.lsx", is_file=True + ) @property def modsettings_path(self): return create_dir_if_needed( - Path(self._organizer.profilePath()) / "modsettings.lsx" + Path(self._organizer.profilePath()) / "modsettings.lsx", is_file=True ) @functools.cached_property @@ -272,11 +274,8 @@ def on_mod_installed(self, mod: mobase.IModInterface) -> None: self._pak_parser.get_metadata_for_files_in_mod(mod, True) -def create_dir_if_needed(path: Path) -> Path: - if "." not in path.name[1:]: - path.mkdir(parents=True, exist_ok=True) - else: - path.parent.mkdir(parents=True, exist_ok=True) +def create_dir_if_needed(path: Path, is_file: bool = False) -> Path: + (path.parent if is_file else path).mkdir(parents=True, exist_ok=True) return path diff --git a/games/game_baldursgate3.py b/games/game_baldursgate3.py index 8ac09661..5275ef89 100644 --- a/games/game_baldursgate3.py +++ b/games/game_baldursgate3.py @@ -179,8 +179,8 @@ def _on_finished_run(self, exec_path: str, exit_code: int): and self.utils.modsettings_path.exists() ): for x in difflib.unified_diff( - self.utils.modsettings_backup.open().readlines(), - self.utils.modsettings_path.open().readlines(), + self.utils.modsettings_backup.read_text(encoding="utf-8").splitlines(), + self.utils.modsettings_path.read_text(encoding="utf-8").splitlines(), fromfile=str(self.utils.modsettings_backup), tofile=str(self.utils.modsettings_path), lineterm="", @@ -205,7 +205,7 @@ def _on_finished_run(self, exec_path: str, exit_code: int): if cat is not None and cat.isDebugEnabled() and len(moved) > 0: qDebug(f"moved log files to logs dir: {moved}") days = self.utils.get_setting("delete_levelcache_folders_older_than_x_days") - if type(days) is int and days >= 0: + if isinstance(days, int) and days >= 0: cutoff_time = datetime.datetime.now() - datetime.timedelta(days=days) qDebug(f"cleaning folders in overwrite/LevelCache older than {cutoff_time}") removed: set[Path] = set() From 12de613bceac7e6ed587d3c43796ea45062a2e61 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:54:47 -0600 Subject: [PATCH 7/9] bg3: keep the strongest status when merging data checker entries --- games/baldursgate3/bg3_data_checker.py | 26 +++++----- tests/test_bg3_data_checker.py | 70 ++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 tests/test_bg3_data_checker.py diff --git a/games/baldursgate3/bg3_data_checker.py b/games/baldursgate3/bg3_data_checker.py index 695d21cd..0d5934ca 100644 --- a/games/baldursgate3/bg3_data_checker.py +++ b/games/baldursgate3/bg3_data_checker.py @@ -31,28 +31,30 @@ def __init__(self): def dataLooksValid( self, filetree: mobase.IFileTree ) -> mobase.ModDataChecker.CheckReturn: - status = mobase.ModDataChecker.INVALID + invalid, valid, fixable = ( + mobase.ModDataChecker.INVALID, + mobase.ModDataChecker.VALID, + mobase.ModDataChecker.FIXABLE, + ) + rank = (invalid, valid, fixable).index + status = invalid rp = self._regex_patterns for entry in filetree: name = entry.name().casefold() if rp.unfold.match(name): if utils.is_directory(entry): - status = self.dataLooksValid(entry) + status = max(status, self.dataLooksValid(entry), key=rank) else: - status = mobase.ModDataChecker.INVALID + status = invalid break elif rp.valid.match(name): - if status is mobase.ModDataChecker.INVALID: - status = mobase.ModDataChecker.VALID + status = max(status, valid, key=rank) elif isinstance(entry, mobase.IFileTree): - status = ( - mobase.ModDataChecker.VALID - if all(rp.valid.match(e.pathFrom(filetree)) for e in entry) - else mobase.ModDataChecker.INVALID - ) + if all(rp.valid.match(e.pathFrom(filetree)) for e in entry): + status = max(status, valid, key=rank) elif rp.delete.match(name) or rp.move_match(name) is not None: - status = mobase.ModDataChecker.FIXABLE + status = fixable else: - status = mobase.ModDataChecker.INVALID + status = invalid break return status diff --git a/tests/test_bg3_data_checker.py b/tests/test_bg3_data_checker.py new file mode 100644 index 00000000..bc91c7e1 --- /dev/null +++ b/tests/test_bg3_data_checker.py @@ -0,0 +1,70 @@ +import importlib +import sys +import types +import unittest +from enum import IntEnum +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock + +mobase: Any = sys.modules.setdefault("mobase", MagicMock()) + + +class _ModDataChecker: + class CheckReturn(IntEnum): + INVALID = 0 + FIXABLE = 1 + VALID = 2 + + INVALID = CheckReturn.INVALID + FIXABLE = CheckReturn.FIXABLE + VALID = CheckReturn.VALID + + +class _IFileTree: + def __init__(self, name: str, children: list[MagicMock]) -> None: + self._name, self._children = name, children + for c in children: + c.pathFrom.return_value = f"{name}/{c.name()}" + + def name(self) -> str: + return self._name + + def isDir(self) -> bool: + return True + + def __iter__(self): + return iter(self._children) + + +mobase.ModDataChecker = _ModDataChecker +mobase.IFileTree = _IFileTree + +# The repo root is a package, so the module's relative imports need a parent package name. +_pkg = types.ModuleType("basic_games") +_pkg.__path__ = [str(Path(__file__).resolve().parents[1])] +sys.modules.setdefault("basic_games", _pkg) + +BG3ModDataChecker = importlib.import_module( + "basic_games.games.baldursgate3.bg3_data_checker" +).BG3ModDataChecker + + +def _file(name: str) -> MagicMock: + entry = MagicMock() + entry.name.return_value = name + entry.isDir.return_value = False + return entry + + +class DataLooksValidTest(unittest.TestCase): + def test_fixable_entry_survives_later_valid_directory(self): + root = [_file("info.json"), _IFileTree("Extra", [_file("x.pak")])] + self.assertEqual( + BG3ModDataChecker().dataLooksValid(root), # type: ignore[arg-type] + _ModDataChecker.FIXABLE, + ) + + +if __name__ == "__main__": + unittest.main() From bc3ec2ab9d40f320745f683230fce96957702633 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:54:51 -0600 Subject: [PATCH 8/9] bg3: skip unreadable files in YAML/JSON conversion, use utf-8 --- games/baldursgate3/bg3_file_mapper.py | 24 +++++----- .../plugins/convert_jsons_to_yaml_plugin.py | 20 ++++----- tests/test_bg3_file_mapper.py | 44 +++++++++++++++++++ 3 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 tests/test_bg3_file_mapper.py diff --git a/games/baldursgate3/bg3_file_mapper.py b/games/baldursgate3/bg3_file_mapper.py index 92c97c68..47c3ddb5 100644 --- a/games/baldursgate3/bg3_file_mapper.py +++ b/games/baldursgate3/bg3_file_mapper.py @@ -1,4 +1,5 @@ import functools +import json import os from pathlib import Path from typing import Callable, Optional @@ -89,26 +90,21 @@ def map_files( if self._utils.convert_yamls_to_json and ( file.name.endswith(".yaml") or file.name.endswith(".yml") ): - converted_path = file.parent / file.name.replace( - ".yaml", ".json" - ).replace(".yml", ".json") + import yaml + + converted_path = file.with_suffix(".json") try: if not converted_path.exists() or os.path.getmtime( file ) > os.path.getmtime(converted_path): - import json - - import yaml - - with open(file, "r") as yaml_file: - with open(converted_path, "w") as json_file: - json.dump( - yaml.safe_load(yaml_file), json_file, indent=2 - ) + data = yaml.safe_load(file.read_text(encoding="utf-8")) + converted_path.write_text( + json.dumps(data, indent=2), encoding="utf-8" + ) qDebug(f"Converted {file} to JSON") found_jsons.add(converted_path) - except OSError as e: - qWarning(f"Error accessing file {converted_path}: {e}") + except (OSError, ValueError, yaml.YAMLError) as e: + qWarning(f"Skipping {file}, conversion to JSON failed: {e}") elif file.name.endswith(".json"): found_jsons.add(file) elif not only_convert: diff --git a/games/baldursgate3/plugins/convert_jsons_to_yaml_plugin.py b/games/baldursgate3/plugins/convert_jsons_to_yaml_plugin.py index fde34315..edcf9449 100644 --- a/games/baldursgate3/plugins/convert_jsons_to_yaml_plugin.py +++ b/games/baldursgate3/plugins/convert_jsons_to_yaml_plugin.py @@ -40,19 +40,19 @@ def display(self): def _convert_jsons_in_dir_to_yaml(path: Path): + import yaml + for file in list(path.rglob("*.json")): - converted_path = file.parent / file.name.replace(".json", ".yaml") + converted_path = file.with_suffix(".yaml") try: if not converted_path.exists() or os.path.getmtime(file) > os.path.getmtime( converted_path ): - import yaml - - with open(file, "r") as json_file: - with open(converted_path, "w") as yaml_file: - yaml.dump( - json.load(json_file), yaml_file, indent=2, sort_keys=False - ) + data = json.loads(file.read_text(encoding="utf-8")) + converted_path.write_text( + yaml.dump(data, indent=2, sort_keys=False, allow_unicode=True), + encoding="utf-8", + ) qInfo(f"Converted {file} to YAML") - except OSError as e: - qWarning(f"Error accessing file {converted_path}: {e}") + except (OSError, ValueError, yaml.YAMLError) as e: + qWarning(f"Skipping {file}, conversion to YAML failed: {e}") diff --git a/tests/test_bg3_file_mapper.py b/tests/test_bg3_file_mapper.py new file mode 100644 index 00000000..3f8b7346 --- /dev/null +++ b/tests/test_bg3_file_mapper.py @@ -0,0 +1,44 @@ +# pyright: reportPrivateUsage=false, reportAbstractUsage=false +import json +import sys +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from typing import Any +from unittest.mock import MagicMock, patch + +mobase: Any = sys.modules.setdefault("mobase", MagicMock()) +mobase.IPluginFileMapper = object +mobase.IPluginTool = type("IPluginTool", (), {}) +mobase.IPlugin = type("IPlugin", (), {}) +mobase.Mapping = SimpleNamespace + +from games.baldursgate3 import bg3_file_mapper # noqa: E402 +from games.baldursgate3.plugins import convert_jsons_to_yaml_plugin # noqa: E402 + + +class ConvertTest(unittest.TestCase): + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()) + + def test_yaml_to_json_skips_bad_file(self): + (self.tmp / "bad.yaml").write_text("a: [", encoding="utf-8") + (self.tmp / "good.yml").write_text("name: Ünï\n", encoding="utf-8") + utils = MagicMock(convert_yamls_to_json=True) + mapper = bg3_file_mapper.BG3FileMapper(utils, MagicMock()) + mapper.map_files(self.tmp, only_convert=True) + good = self.tmp / "good.json" + self.assertEqual(json.loads(good.read_text(encoding="utf-8")), {"name": "Ünï"}) + self.assertFalse((self.tmp / "bad.json").exists()) + + def test_json_to_yaml_skips_bad_file(self): + (self.tmp / "bad.json").write_text("{", encoding="utf-8") + (self.tmp / "good.json").write_text('{"name": "Ünï"}', encoding="utf-8") + convert_jsons_to_yaml_plugin._convert_jsons_in_dir_to_yaml(self.tmp) + self.assertIn("Ünï", (self.tmp / "good.yaml").read_text(encoding="utf-8")) + self.assertFalse((self.tmp / "bad.yaml").exists()) + + +if __name__ == "__main__": + unittest.main() From 02f4f237d4ff56f82054abf09a0da0d7869c12e9 Mon Sep 17 00:00:00 2001 From: "d.schlatter" Date: Thu, 8 Oct 2026 23:55:12 -0600 Subject: [PATCH 9/9] bg3: keep modsettings mapping when file mapping is canceled --- games/baldursgate3/bg3_file_mapper.py | 45 ++++++++++++++------------- tests/test_bg3_file_mapper.py | 18 ++++++++++- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/games/baldursgate3/bg3_file_mapper.py b/games/baldursgate3/bg3_file_mapper.py index 47c3ddb5..74b8af87 100644 --- a/games/baldursgate3/bg3_file_mapper.py +++ b/games/baldursgate3/bg3_file_mapper.py @@ -13,8 +13,6 @@ class BG3FileMapper(mobase.IPluginFileMapper): - current_mappings: list[mobase.Mapping] = [] - def __init__(self, utils: bg3_utils.BG3Utils, doc_dir: Callable[[], QDir]): super().__init__() self._utils = utils @@ -26,55 +24,58 @@ def doc_path(self): def mappings(self) -> list[mobase.Mapping]: qInfo("creating custom bg3 mappings") - self.current_mappings.clear() active_mods = self._utils.active_mods() if not active_mods: return [] progress = self._utils.create_progress_window( "Mapping files to documents folder", len(active_mods) + 1 ) + mappings: list[mobase.Mapping] = [] + self.create_mapping( + mappings, + self._utils.modsettings_path, + self.doc_path + / "PlayerProfiles" + / "Public" + / self._utils.modsettings_path.name, + ) docs_path_mods = self.doc_path / "Mods" docs_path_se = self.doc_path / "Script Extender" for mod in active_mods: modpath = Path(mod.absolutePath()) - self.map_files(modpath, dest=docs_path_mods, pattern="*.pak", rel=False) - self.map_files(modpath / "Script Extender", dest=docs_path_se) + self.map_files( + mappings, modpath, dest=docs_path_mods, pattern="*.pak", rel=False + ) + self.map_files(mappings, modpath / "Script Extender", dest=docs_path_se) if self._utils.convert_yamls_to_json: - self.map_files(modpath / "bin", only_convert=True) + self.map_files(mappings, modpath / "bin", only_convert=True) progress.setValue(progress.value() + 1) QApplication.processEvents() if progress.wasCanceled(): qWarning("mapping canceled by user") - return self.current_mappings + return mappings (self._utils.overwrite_path / "Script Extender").mkdir( parents=True, exist_ok=True ) (self._utils.overwrite_path / "Stats").mkdir(parents=True, exist_ok=True) (self._utils.overwrite_path / "Temp").mkdir(parents=True, exist_ok=True) (self._utils.overwrite_path / "LevelCache").mkdir(parents=True, exist_ok=True) - (self._utils.overwrite_path / "Stats").mkdir(parents=True, exist_ok=True) (self._utils.overwrite_path / "Mods").mkdir(parents=True, exist_ok=True) (self._utils.overwrite_path / "GMCampaigns").mkdir(parents=True, exist_ok=True) - self.map_files(self._utils.overwrite_path) - self.create_mapping( - self._utils.modsettings_path, - self.doc_path - / "PlayerProfiles" - / "Public" - / self._utils.modsettings_path.name, - ) + self.map_files(mappings, self._utils.overwrite_path) progress.setValue(len(active_mods) + 1) QApplication.processEvents() progress.close() cat = QLoggingCategory.defaultCategory() if cat is not None and cat.isDebugEnabled(): qDebug( - f"resolved mappings: { {m.source: m.destination for m in self.current_mappings} }" + f"resolved mappings: { {m.source: m.destination for m in mappings} }" ) - return self.current_mappings + return mappings def map_files( self, + mappings: list[mobase.Mapping], path: Path, dest: Optional[Path] = None, pattern: str = "*", @@ -108,16 +109,16 @@ def map_files( elif file.name.endswith(".json"): found_jsons.add(file) elif not only_convert: - self.create_mapping(file, dest / dest_func(file)) + self.create_mapping(mappings, file, dest / dest_func(file)) if only_convert: return for file in found_jsons: - self.create_mapping(file, dest / dest_func(file)) + self.create_mapping(mappings, file, dest / dest_func(file)) - def create_mapping(self, file: Path, dest: Path): + def create_mapping(self, mappings: list[mobase.Mapping], file: Path, dest: Path): bg3_utils.create_dir_if_needed(dest, is_file=not file.is_dir()) - self.current_mappings.append( + mappings.append( mobase.Mapping( source=str(file), destination=str(dest), diff --git a/tests/test_bg3_file_mapper.py b/tests/test_bg3_file_mapper.py index 3f8b7346..5ccd68cd 100644 --- a/tests/test_bg3_file_mapper.py +++ b/tests/test_bg3_file_mapper.py @@ -27,7 +27,7 @@ def test_yaml_to_json_skips_bad_file(self): (self.tmp / "good.yml").write_text("name: Ünï\n", encoding="utf-8") utils = MagicMock(convert_yamls_to_json=True) mapper = bg3_file_mapper.BG3FileMapper(utils, MagicMock()) - mapper.map_files(self.tmp, only_convert=True) + mapper.map_files([], self.tmp, only_convert=True) good = self.tmp / "good.json" self.assertEqual(json.loads(good.read_text(encoding="utf-8")), {"name": "Ünï"}) self.assertFalse((self.tmp / "bad.json").exists()) @@ -40,5 +40,21 @@ def test_json_to_yaml_skips_bad_file(self): self.assertFalse((self.tmp / "bad.yaml").exists()) +class CancelTest(unittest.TestCase): + def test_cancel_keeps_modsettings_mapping(self): + tmp = Path(tempfile.mkdtemp()) + modsettings = tmp / "modsettings.lsx" + modsettings.touch() + utils = MagicMock(convert_yamls_to_json=False, modsettings_path=modsettings) + utils.active_mods.return_value = [MagicMock(absolutePath=lambda: str(tmp))] + utils.create_progress_window.return_value.wasCanceled.return_value = True + mapper = bg3_file_mapper.BG3FileMapper( + utils, lambda: MagicMock(path=lambda: str(tmp / "docs")) + ) + with patch.object(bg3_file_mapper, "QApplication"): + sources = [m.source for m in mapper.mappings()] + self.assertIn(str(modsettings), sources) + + if __name__ == "__main__": unittest.main()