From 01e33c0772074fd7606d1c52f3835f1f908ab05e Mon Sep 17 00:00:00 2001 From: Moshu <1709219+DatMoshu@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:27:10 -0500 Subject: [PATCH 1/3] Fix good-first-issue batch (#5-#12) VD writer range check, VD reader clipping, vdtool usage errors, unique atomic temp files in jsonio, hash-verified restore, narrowed silhouette-fit annotation handling, tolerant Content Studio startup, and portable font/Blender lookup. Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Fable 5.1 --- common/jsonio.py | 22 ++++-- common/pipeline/versions.py | 5 +- .../ultima-online/outfit-lab/uo_vd_writer.py | 5 +- games/ultima-online/outfit-lab/vd.py | 6 +- .../region-masks/build_regions.py | 5 +- .../region-masks/clothing_fit.py | 5 +- .../region-masks/package_masks.py | 5 +- .../region-masks/pose_editor_server.py | 12 +++- .../region-masks/region_fonts.py | 21 ++++++ tests/unit/test_jsonio.py | 47 +++++++++++++ tests/unit/test_portability.py | 30 +++++++++ tests/unit/test_robust_startup.py | 40 +++++++++++ tests/unit/test_vd_robustness.py | 67 +++++++++++++++++++ tests/unit/test_versions.py | 21 ++++++ tools/silhouette-fit/run.py | 18 +++-- tools/uo-content/pipeline.py | 2 +- tools/uo-content/studio.py | 21 ++++-- tools/vd/vdtool.py | 11 ++- 18 files changed, 314 insertions(+), 29 deletions(-) create mode 100644 games/ultima-online/region-masks/region_fonts.py create mode 100644 tests/unit/test_jsonio.py create mode 100644 tests/unit/test_portability.py create mode 100644 tests/unit/test_robust_startup.py create mode 100644 tests/unit/test_vd_robustness.py diff --git a/common/jsonio.py b/common/jsonio.py index a400d75..7cf0ecc 100644 --- a/common/jsonio.py +++ b/common/jsonio.py @@ -4,6 +4,8 @@ import json import math import os +import shutil +import tempfile from pathlib import Path from typing import Any @@ -33,9 +35,19 @@ def write_json(path: str | Path, value: Any, indent: int | None = 1, backup: boo """Write via a temporary file and atomic replace; optionally keep the previous file as .bak.""" path = Path(path) path.parent.mkdir(parents=True, exist_ok=True) - temp = path.with_name(path.name + ".tmp") - temp.write_text(dumps(value, indent), encoding="utf-8") - if backup and path.exists(): - os.replace(path, path.with_name(path.name + ".bak")) - os.replace(temp, path) + text = dumps(value, indent) + handle = tempfile.NamedTemporaryFile("w", encoding="utf-8", newline="", dir=path.parent, + prefix=path.name + ".", suffix=".tmp", delete=False) + temp = Path(handle.name) + try: + with handle: + handle.write(text) + handle.flush() + os.fsync(handle.fileno()) + if backup and path.exists(): + shutil.copy2(path, path.with_name(path.name + ".bak")) # the target itself is never moved away + os.replace(temp, path) + except BaseException: + temp.unlink(missing_ok=True) + raise return path diff --git a/common/pipeline/versions.py b/common/pipeline/versions.py index 415da63..a3fc483 100644 --- a/common/pipeline/versions.py +++ b/common/pipeline/versions.py @@ -129,7 +129,10 @@ def restore(path: str | Path, version: int) -> dict: source = version_file(path, version) if not source.exists(): if source_entry is history["versions"][-1] and path.exists(): - return source_entry # already current + if _sha256(path) == source_entry.get("sha256"): + return source_entry # already current, verified by hash + raise FileNotFoundError(f"{path.name} has changed since version {version} was recorded and that version " + f"was never archived ({source.name} missing), so it cannot be restored.") raise FileNotFoundError(f"Version {version} was never archived ({source.name} missing).") archive_current(path) shutil.copy2(source, path) diff --git a/games/ultima-online/outfit-lab/uo_vd_writer.py b/games/ultima-online/outfit-lab/uo_vd_writer.py index 9d61356..328525a 100644 --- a/games/ultima-online/outfit-lab/uo_vd_writer.py +++ b/games/ultima-online/outfit-lab/uo_vd_writer.py @@ -94,7 +94,10 @@ def encode_frame(idx, cx, cy): x0 = x while x < w and row[x] >= 0 and x - x0 < 0xFFF: x += 1 - out += struct.pack(">22)&0x3ff); y=yb+((hdr>>12)&0x3ff); n=hdr&0xfff idx=d[p:p+n]; p+=n - for k,ci in enumerate(idx): - img[y,x+k,:3]=pal[ci]; img[y,x+k,3]=255 + if 0<=y tuple[dict, dict]: + """Annotated pose targets for the sequence; unreadable annotations are reported, not silently dropped.""" + try: + return select_targets(dataset, sequence, names, "all", allow_dependent=True) + except (KeyError, ValueError, OSError) as error: # malformed, mismatched or unreadable annotation files + LOG.warning("Ignoring annotations for sequence %s (%s: %s); fitting to the silhouette alone.", + sequence, type(error).__name__, error) + return {}, {"mode": "all", "used": [], "mirrored_into_partner": []} + + def fit_one(job: dict) -> dict: started = time.time() dataset = Dataset.load(job["dataset"]) @@ -222,10 +235,7 @@ def fit_one(job: dict) -> dict: fitter = SilhouetteFitter(rig, mapping, projection, names, direction_rotations(dataset.directions, (0.0, -1.0)), settings, edges=edges, mask_weight=job["mask_weight"], skin=skin, coverage_weight=job.get("coverage_weight", 0.0)) - try: - targets, selection = select_targets(dataset, sequence, names, "all", allow_dependent=True) - except Exception: - targets, selection = {}, {"mode": "all", "used": [], "mirrored_into_partner": []} + targets, selection = annotation_targets(dataset, sequence, names) if skin is not None: # every frame, every stored view; frames without marks are fitted to the silhouette alone stored = [d["id"] for d in dataset.directions if dataset.mirror_source(d["id"]) is None] count = next(s["frame_count"] for s in dataset.sequences if s["id"] == sequence) diff --git a/tools/uo-content/pipeline.py b/tools/uo-content/pipeline.py index 3cfd442..9236689 100644 --- a/tools/uo-content/pipeline.py +++ b/tools/uo-content/pipeline.py @@ -61,7 +61,7 @@ def blender_path(): found = shutil.which('blender') if found: return found - choices = sorted(Path('C:/Program Files/Blender Foundation').glob('Blender */blender.exe'), reverse=True) + choices = sorted(Path(os.environ.get('ProgramFiles', 'C:/Program Files'), 'Blender Foundation').glob('Blender */blender.exe'), reverse=True) if choices: return str(choices[0]) raise ValueError('Set SPRITEMOTION_BLENDER to your Blender 4.2+ executable.') diff --git a/tools/uo-content/studio.py b/tools/uo-content/studio.py index 000c9ac..1f04106 100644 --- a/tools/uo-content/studio.py +++ b/tools/uo-content/studio.py @@ -126,14 +126,25 @@ def job(identifier): if not (path/'job.json').exists(): raise ValueError('Job not found.') return path +def recover_interrupted_jobs(jobs): + """Mark jobs left queued/building as failed. Returns [(status file, error)] for unreadable status files, which are skipped.""" + skipped=[] + for file in Path(jobs).glob('*/status.json'): + try: + status=json.loads(file.read_text(encoding='utf-8')) + interrupted=status['state'] in ('queued','building') + except (OSError,ValueError,KeyError,TypeError) as e: + skipped.append((file,e)); continue + if interrupted: + status.update(state='failed',error='Studio stopped during build. Load the settings and build again.') + pipeline.write_json(file,status) + return skipped + if __name__=='__main__': p=argparse.ArgumentParser(description=__doc__); p.add_argument('--port',type=int,default=8772) args=p.parse_args(); (pipeline.HOME/'jobs').mkdir(parents=True,exist_ok=True) # Jobs interrupted by a server shutdown are never presented as still rendering. - for file in (pipeline.HOME/'jobs').glob('*/status.json'): - status=json.loads(file.read_text(encoding='utf-8')) - if status['state'] in ('queued','building'): - status.update(state='failed',error='Studio stopped during build. Load the settings and build again.') - pipeline.write_json(file,status) + for file,error in recover_interrupted_jobs(pipeline.HOME/'jobs'): + print(f'Skipped job {file.parent.name}: unreadable status.json ({error})',flush=True) print(f'SpriteMotion content studio: http://127.0.0.1:{args.port}',flush=True) ThreadingHTTPServer(('127.0.0.1',args.port),Handler).serve_forever() diff --git a/tools/vd/vdtool.py b/tools/vd/vdtool.py index a55b590..b976fdd 100644 --- a/tools/vd/vdtool.py +++ b/tools/vd/vdtool.py @@ -124,8 +124,11 @@ def encode_frame(idx, cx, cy): x0 = x while x < w and row[x] >= 0 and x - x0 < 0xFFF: x += 1 - dx = (x0 - cx) & 0x3FF - dy = (y - cy - h) & 0x3FF + dx, dy = x0 - cx, y - cy - h + if not (-512 <= dx <= 511 and -512 <= dy <= 511): + raise ValueError(f"run offset out of 10-bit range (-512..511): dx={dx}, dy={dy} at row {y}") + dx &= 0x3FF + dy &= 0x3FF out += struct.pack(" [--raw]", "pack": " ", "verify": " "} + print(f"usage: vdtool.py {cmd} {names[cmd]}") + return 2 if cmd == "info": cmd_info(argv[2]) elif cmd == "extract": From d4099710de866955e41f8e42ae286d38b89d5830 Mon Sep 17 00:00:00 2001 From: Moshu <1709219+DatMoshu@users.noreply.github.com> Date: Tue, 6 Oct 2026 03:35:22 -0500 Subject: [PATCH 2/3] Prefer project Blender runtime in pose editor bake; sort versions numerically; private temp files Co-Authored-By: Claude Sonnet 5.5 --- .../region-masks/pose_editor_server.py | 25 +++++++++---- tests/test_pose_editor.py | 36 +++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/games/ultima-online/region-masks/pose_editor_server.py b/games/ultima-online/region-masks/pose_editor_server.py index d759ab1..ef0f883 100644 --- a/games/ultima-online/region-masks/pose_editor_server.py +++ b/games/ultima-online/region-masks/pose_editor_server.py @@ -1,6 +1,6 @@ """Loopback-only live pose editor, local saves, and asynchronous Blender export.""" from pathlib import Path -import argparse, functools, json, math, os, shutil, subprocess, threading, time, uuid +import argparse, functools, json, math, os, re, shutil, subprocess, threading, time, uuid from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer from urllib.parse import urlparse, parse_qs @@ -9,14 +9,27 @@ SOURCE=Path(__file__).parent EDITABLE={f'{bone}_{side}' for bone in ('upperarm','lowerarm','hand','thigh','calf','foot') for side in ('l','r')}|{'LegPlate_L','LegPlate_R','Hip_L','Hip_R'} -def find_blender(): - exe=os.environ.get('SPRITEMOTION_BLENDER') or shutil.which('blender') +def _version_key(path): + return tuple(int(n) for n in re.findall(r'\d+',path.parent.name)) + +def find_blender(root=None,environ=None): + """SPRITEMOTION_BLENDER, then the newest tools/blender-runtime build, then PATH, then Program Files.""" + root=Path(root) if root else ROOT;environ=os.environ if environ is None else environ + exe=environ.get('SPRITEMOTION_BLENDER') + if not exe: + runtime=sorted((root/'tools/blender-runtime').glob('*/blender.exe'),key=_version_key) + exe=str(runtime[-1]) if runtime else None + exe=exe or shutil.which('blender') if not exe: - found=sorted(Path(os.environ.get('ProgramFiles','C:/Program Files'),'Blender Foundation').glob('Blender */blender.exe')) + found=sorted(Path(environ.get('ProgramFiles','C:/Program Files'),'Blender Foundation').glob('Blender */blender.exe'),key=_version_key) exe=str(found[-1]) if found else None if not exe:raise RuntimeError('Set SPRITEMOTION_BLENDER to blender.exe.') return exe +def write_private(path,text): + fd=os.open(path,os.O_WRONLY|os.O_CREAT|os.O_TRUNC,0o600) + with os.fdopen(fd,'w') as f:f.write(text) + def validate(doc,scene): if not isinstance(doc,dict) or doc.get('version')!=1 or doc.get('assetId')!=scene['assetId']: raise ValueError('Edits do not match this model version.') @@ -69,12 +82,12 @@ def do_POST(self): doc=validate(json.loads(self.rfile.read(size)),self.server.scene) if self.path=='/api/save': with self.server.lock: - tmp=OUT/'editor/edits.tmp';tmp.write_text(json.dumps(doc,indent=2));os.replace(tmp,OUT/'editor/edits.json') + tmp=OUT/'editor/edits.tmp';write_private(tmp,json.dumps(doc,indent=2));os.replace(tmp,OUT/'editor/edits.json') return self.send_json({'saved':True}) with self.server.lock: if any(j['status']=='running' for j in self.server.jobs.values()):return self.send_json({'error':'An export is already running.'},409) job=uuid.uuid4().hex[:12];folder=OUT/'editor/exports'/job;folder.mkdir(parents=True) - edits=folder/'edits.json';edits.write_text(json.dumps(doc)) + edits=folder/'edits.json';write_private(edits,json.dumps(doc)) self.server.jobs[job]={'id':job,'status':'running'} threading.Thread(target=self.bake,args=(job,folder,edits),daemon=True).start() self.send_json({'id':job},202) diff --git a/tests/test_pose_editor.py b/tests/test_pose_editor.py index 102b728..131388b 100644 --- a/tests/test_pose_editor.py +++ b/tests/test_pose_editor.py @@ -1,6 +1,9 @@ """Edits accepted for Blender export must be bounded and tied to the exact asset.""" import importlib.util +import os +import tempfile import unittest +from unittest import mock from pathlib import Path path=Path(__file__).resolve().parents[1]/'games/ultima-online/region-masks/pose_editor_server.py' @@ -28,4 +31,37 @@ def test_nonfinite_and_nonunit_quaternions_rejected(self): self.doc['edits']['Walk:9']['upperarm_l']=q with self.assertRaises(ValueError):module.validate(self.doc,self.scene) +class FindBlender(unittest.TestCase): + def make(self,root,*names): + for name in names: + exe=Path(root)/'tools/blender-runtime'/name/'blender.exe';exe.parent.mkdir(parents=True);exe.touch() + def test_env_wins(self): + with tempfile.TemporaryDirectory() as d: + self.make(d,'blender-5.2.2-windows-x64') + self.assertEqual(module.find_blender(d,{'SPRITEMOTION_BLENDER':'custom.exe'}),'custom.exe') + def test_newest_runtime_beats_path_and_program_files(self): + with tempfile.TemporaryDirectory() as d: + self.make(d,'blender-5.2.2-windows-x64','blender-5.10.0-windows-x64','blender-4.2.1-windows-x64') + with mock.patch.object(module.shutil,'which',return_value='path-blender.exe'): + found=module.find_blender(d,{'ProgramFiles':d}) + self.assertIn('blender-5.10.0',found) + def test_path_used_without_runtime(self): + with tempfile.TemporaryDirectory() as d,mock.patch.object(module.shutil,'which',return_value='path-blender.exe'): + self.assertEqual(module.find_blender(d,{}),'path-blender.exe') + def test_program_files_version_sorted_numerically(self): + with tempfile.TemporaryDirectory() as d,mock.patch.object(module.shutil,'which',return_value=None): + for v in ('Blender 4.2','Blender 10.0'): + exe=Path(d)/'Blender Foundation'/v/'blender.exe';exe.parent.mkdir(parents=True);exe.touch() + self.assertIn('Blender 10.0',module.find_blender(Path(d)/'none',{'ProgramFiles':d})) + def test_missing_raises(self): + with tempfile.TemporaryDirectory() as d,mock.patch.object(module.shutil,'which',return_value=None): + with self.assertRaises(RuntimeError):module.find_blender(d,{'ProgramFiles':d}) + +class PrivateWrite(unittest.TestCase): + def test_content_written_and_private_mode_requested(self): + with tempfile.TemporaryDirectory() as d: + target=Path(d)/'edits.json';module.write_private(target,'{}') + self.assertEqual(target.read_text(),'{}') + if os.name!='nt':self.assertEqual(target.stat().st_mode&0o777,0o600) + if __name__=='__main__':unittest.main() From e6687c29309ad6bbc592ac0677889e80fc25849a Mon Sep 17 00:00:00 2001 From: Moshu <1709219+DatMoshu@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:34:54 -0500 Subject: [PATCH 3/3] Skip the silhouette-fit startup test when scipy is missing (CI has no scipy) Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_robust_startup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_robust_startup.py b/tests/unit/test_robust_startup.py index bac807a..27e6598 100644 --- a/tests/unit/test_robust_startup.py +++ b/tests/unit/test_robust_startup.py @@ -21,6 +21,7 @@ def test_corrupt_job_status_is_skipped_and_reported(tmp_path, monkeypatch): def test_bad_annotations_log_a_warning_and_other_errors_propagate(monkeypatch, caplog): + pytest.importorskip("scipy") # silhouette-fit needs it; the CI image doesn't install it run = load_module(REPO / "tools" / "silhouette-fit" / "run.py", "silhouette_fit_run_test") def bad(*args, **kwargs):