Skip to content

Commit 8e1ff22

Browse files
sp00ktoberclaude
andcommitted
fix(studio): the stage push owns its scratch, and a catch never freezes the game
Found in the live test. STAGE_SCRATCH lay inside the framework's Lua slab, and the collision push wrote over memory it did not own after a warning. The framework now reserves the scratch on the emulator before any mod allocates (its size is in addresses.toml), and the push writes only zeros or its own last records, within that size. A catch whose slot goes to another file ends at the first foreign write instead of stopping the CPU on every write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 099697b commit 8e1ff22

5 files changed

Lines changed: 188 additions & 61 deletions

File tree

‎apps/studio/src/mhfu_studio/stage/commands.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ def command(name: str, help: str) -> argparse.ArgumentParser:
7373
c.add_argument("--restore", action="store_true", help="put the file's bytes back instead")
7474
c.add_argument("--undo", type=Path, metavar="FILE", help="replay a collision undo file")
7575
c.add_argument("--scratch", type=lambda s: int(s, 0), help="free RAM for added collision")
76+
c.add_argument(
77+
"--scratch-size", type=int, help="bytes free at --scratch (default: STAGE_SCRATCH's)"
78+
)
7679
c.add_argument("--dry", action="store_true", help="plan it offline; touch no emulator")
7780
c.set_defaults(run=push)
7881

@@ -297,7 +300,9 @@ def push(args: argparse.Namespace) -> int:
297300
halves = {h: getattr(args, h) for h in ("mesh", "collision", "textures")}
298301
if not any(halves.values()):
299302
halves = dict.fromkeys(halves, True)
300-
p = live.prepare(sf, ops, args.ops.parent, scratch=args.scratch or a.STAGE_SCRATCH, **halves)
303+
scratch = args.scratch or a.STAGE_SCRATCH
304+
size = args.scratch_size or a.STAGE_SCRATCH.count or 0
305+
p = live.prepare(sf, ops, args.ops.parent, scratch=scratch, scratch_size=size, **halves)
301306
for line in [*p.log, *map(str, p.findings), *live.describe(p)]:
302307
print(line)
303308
catch = args.catch

‎apps/studio/src/mhfu_studio/stage/live.py‎

Lines changed: 112 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from collections.abc import Callable, Sequence
1919
from dataclasses import dataclass, field
2020
from pathlib import Path
21-
from typing import Protocol
21+
from typing import Any, Protocol
2222

2323
from mhfu import addresses as a
2424
from mhfu import files
@@ -110,65 +110,103 @@ def _holds(self, mem: Memory, pac: int, o: int, n: int) -> bool:
110110
return mem.read(pac + self.offset + o, n) == self.data[o : o + n]
111111

112112

113+
class ScratchError(ValueError):
114+
"""The scratch is not free: the push refuses rather than write memory it does not own."""
115+
116+
113117
@dataclass
114118
class Collision:
115119
"""A collision plan written into the running game: moved records where they are, added
116-
records and the changed cell lists in `scratch`, one grid word per changed cell."""
120+
records and the changed cell lists in `scratch`, one grid word per changed cell.
121+
122+
The scratch is written only where it is zero or holds what this document's last push
123+
wrote there (`owned`), never over anything else."""
117124

118125
stage: StageFile
119126
plan: Plan
120127
scratch: int = a.STAGE_SCRATCH
128+
size: int = a.STAGE_SCRATCH.count or 0
129+
"""Bytes the push may use from `scratch`."""
121130
name: str = "collision"
122-
undo: list[tuple[int, bytes]] = field(default_factory=list)
123-
"""(address, the bytes there before) of every write into the PAC, from the last apply."""
131+
undo: list[tuple[int, bytes, bytes]] = field(default_factory=list)
132+
"""(address, the bytes there before, what the push wrote) of every write into the PAC."""
133+
owned: bytes = b""
134+
"""What this document last wrote at `scratch`: the one non-zero content it may replace."""
124135
_probe: tuple[int, int, int, bytes] | None = None
125136
"""(chunk, GRID_POINTER or TRIS_POINTER, offset from that pointer, what we wrote there)."""
126137

127138
def pointers(self, mem: Memory, pac: int, chunk: int) -> tuple[int, int, int]:
128139
return chunk_pointers(mem, self.stage, pac, chunk)
129140

141+
@property
142+
def scratch_used(self) -> int:
143+
p = self.plan
144+
lists = sum(4 * (len(r) + 1) for cells in p.cells.values() for r in cells.values())
145+
return TRI_SIZE * len(p.added) + lists
146+
147+
def check_scratch(self, mem: Memory) -> None:
148+
"""Raise ScratchError unless the plan fits the scratch and every byte it would write
149+
over is zero or this document's own."""
150+
need = max(self.scratch_used, len(self.owned))
151+
if self.scratch_used > self.size:
152+
raise ScratchError(
153+
f"the collision needs {self.scratch_used} bytes of scratch;"
154+
f" 0x{self.scratch:08X} has {self.size}"
155+
)
156+
here = mem.read(self.scratch, need)
157+
mine = self.owned.ljust(need, b"\0")
158+
foreign = [i for i, (x, y) in enumerate(zip(here, mine, strict=True)) if x and x != y]
159+
if foreign:
160+
at = foreign[0]
161+
raise ScratchError(
162+
f"scratch at 0x{self.scratch + at:08X} holds data this document did not write;"
163+
" refusing to write over it (give --scratch free memory)"
164+
)
165+
166+
def adopt(self, saved: dict[str, Any]) -> None:
167+
"""Take a previous push's undo file as this one's own: the scratch it wrote."""
168+
at, data = saved.get("scratch", [self.scratch, ""])
169+
if at == self.scratch:
170+
self.owned = bytes.fromhex(data)
171+
130172
def apply(self, mem: Memory, pac: int) -> None:
131-
p, undo = self.plan, []
173+
p = self.plan
132174
chunks = sorted({c for c, _, _ in p.moved} | {c for c, _ in p.added} | set(p.cells))
133175
live = {c: self.pointers(mem, pac, c) for c in chunks}
134-
135-
def put(addr: int, data: bytes) -> None:
136-
undo.append((addr, mem.read(addr, len(data))))
137-
mem.write(addr, data)
138-
176+
self.check_scratch(mem)
177+
writes: list[tuple[int, bytes]] = []
139178
for c, t, tri in p.moved:
140-
put(live[c][2] + TRI_SIZE * t, tri.to_bytes())
141-
at = self.scratch
179+
writes.append((live[c][2] + TRI_SIZE * t, tri.to_bytes()))
180+
blob = bytearray()
142181
added = {}
143182
for k, (_, tri) in enumerate(p.added):
144-
added[k] = at
145-
mem.write(at, tri.to_bytes())
146-
at += TRI_SIZE
183+
added[k] = self.scratch + len(blob)
184+
blob += tri.to_bytes()
147185
probe = None
148186
for c, cells in sorted(p.cells.items()):
149187
_, grid, tris = live[c]
150188
for cell, refs in sorted(cells.items()):
189+
at = self.scratch + len(blob)
151190
words = [added[r.number] if r.added else tris + TRI_SIZE * r.number for r in refs]
152-
body = b"".join(w.to_bytes(4, "little") for w in [*words, TERM])
153-
mem.write(at, body)
154-
put(grid + 4 * cell, at.to_bytes(4, "little"))
191+
blob += b"".join(w.to_bytes(4, "little") for w in [*words, TERM])
192+
writes.append((grid + 4 * cell, at.to_bytes(4, "little")))
155193
probe = probe or (c, GRID_POINTER, 4 * cell, at.to_bytes(4, "little"))
156-
at += len(body)
157194
if probe is None and p.moved:
158195
c, t, tri = p.moved[0]
159196
probe = (c, TRIS_POINTER, TRI_SIZE * t, tri.to_bytes())
197+
# what the last push left past this one's end is ours and no longer linked: clear it
198+
mem.write(self.scratch, bytes(blob).ljust(len(self.owned), b"\0"))
199+
self.owned = bytes(blob)
200+
# a word still holding what the last apply wrote keeps that apply's "before"
201+
last = {addr: (old, new) for addr, old, new in self.undo}
202+
undo = []
203+
for addr, data in writes:
204+
here = mem.read(addr, len(data))
205+
old, new = last.get(addr, (here, b""))
206+
undo.append((addr, old if here == new else here, data))
207+
mem.write(addr, data)
160208
self._probe, self.undo = probe, undo
161209

162-
def scratch_free(self, mem: Memory) -> bool:
163-
"""Zero where the plan writes: not a previous push's, nor anything the game keeps."""
164-
return not any(mem.read(self.scratch, self.scratch_used))
165-
166-
@property
167-
def scratch_used(self) -> int:
168-
p = self.plan
169-
lists = sum(4 * (len(r) + 1) for cells in p.cells.values() for r in cells.values())
170-
return TRI_SIZE * len(p.added) + lists
171-
172210
def intact(self, mem: Memory, pac: int) -> bool:
173211
if self._probe is None:
174212
return True
@@ -179,8 +217,9 @@ def intact(self, mem: Memory, pac: int) -> bool:
179217

180218
def save_undo(self, path: Path, pac: int) -> None:
181219
path.parent.mkdir(parents=True, exist_ok=True)
182-
undo = [[addr, data.hex()] for addr, data in self.undo]
183-
path.write_text(json.dumps({"stage": self.stage.number, "pac": pac, "undo": undo}))
220+
undo = [[addr, old.hex(), new.hex()] for addr, old, new in self.undo]
221+
saved = {"stage": self.stage.number, "pac": pac, "undo": undo}
222+
path.write_text(json.dumps(saved | {"scratch": [self.scratch, self.owned.hex()]}))
184223

185224

186225
def chunk_pointers(mem: Memory, stage: StageFile, pac: int, chunk: int) -> tuple[int, int, int]:
@@ -194,11 +233,14 @@ def chunk_pointers(mem: Memory, stage: StageFile, pac: int, chunk: int) -> tuple
194233

195234

196235
def undo(mem: Memory, path: Path) -> int:
197-
"""Replay an undo file backwards; returns how many writes it held."""
198-
saved = json.loads(path.read_text())["undo"]
199-
for addr, data in reversed(saved):
200-
mem.write(addr, bytes.fromhex(data))
201-
return len(saved)
236+
"""Take an undo file's writes back out, newest first, where each still holds what the push
237+
wrote (a reload or another push has replaced the rest); returns how many."""
238+
n = 0
239+
for addr, old, new in reversed(json.loads(path.read_text())["undo"]):
240+
if mem.read(addr, len(new) // 2) == bytes.fromhex(new):
241+
mem.write(addr, bytes.fromhex(old))
242+
n += 1
243+
return n
202244

203245

204246
@dataclass
@@ -231,6 +273,7 @@ def prepare(
231273
collision: bool = True,
232274
textures: bool = True,
233275
scratch: int = a.STAGE_SCRATCH,
276+
scratch_size: int = a.STAGE_SCRATCH.count or 0,
234277
) -> Push:
235278
"""The writes for the halves asked for, built offline."""
236279
out = Push(stage)
@@ -249,7 +292,7 @@ def prepare(
249292
out.log += p.log
250293
out.findings += p.findings
251294
if not p.empty:
252-
out.collision = Collision(stage, p, scratch)
295+
out.collision = Collision(stage, p, scratch, scratch_size)
253296
if textures and any(O.touches_textures(op) for op in ops):
254297
bank = build(stage, ops, base_dir)
255298
out.log += bank.log
@@ -318,6 +361,13 @@ def run(
318361
pac = pac_address(mem, stage.number)
319362
if pac is None:
320363
raise ValueError(f"{stage.label}.pac is not resident: stand in the area first")
364+
saved = None
365+
if push.collision:
366+
if undo_file is not None and undo_file.exists():
367+
saved = json.loads(undo_file.read_text())
368+
if saved.get("stage") == stage.number:
369+
push.collision.adopt(saved)
370+
push.collision.check_scratch(mem) # before anything is written
321371
for b in push.mesh:
322372
b.apply(mem, pac)
323373
log(f"{b.name}: written at 0x{pac + b.offset:08X}, verified {b.verify(mem, pac)}")
@@ -330,8 +380,8 @@ def run(
330380
catch(client, mem, push, catch_for, log)
331381
pac = pac_address(mem, stage.number) or pac
332382
if push.collision:
333-
if not push.collision.scratch_free(mem):
334-
log(f"[!] scratch at 0x{push.collision.scratch:08X} is not zero: a previous push's?")
383+
if saved is not None and undo_file is not None and (n := undo(mem, undo_file)):
384+
log(f"collision: took the last push's {n} writes back out")
335385
push.collision.apply(mem, pac)
336386
log(f"collision: applied at 0x{pac:08X}")
337387
if undo_file is not None:
@@ -366,35 +416,49 @@ def catch(
366416
poll: float = 0.25,
367417
) -> bool:
368418
"""Wait up to `seconds` for the stage's PAC to be re-read, and put the mesh edit on top
369-
while the loader is stopped on it. A slot can be reused by another stage, so a stop counts
370-
only when the PAC there is this stage's."""
419+
while the loader is stopped on it. A slot can be reused by another file: the first write
420+
to the watched word that is not this stage's ends the watch, so a file that keeps writing
421+
there never stops the game write by write."""
371422
if not push.mesh:
372423
return False
424+
label = push.stage.label
373425
deadline = time.monotonic() + seconds
374426
log(
375-
f"watching the tail of {push.stage.label}.pac for {seconds:.0f} s: take the transition"
376-
" now (the game runs slower while it is armed)"
427+
f"watching the tail of {label}.pac for {seconds:.0f} s: take the transition now"
428+
" (the game runs slower while it is armed)"
377429
)
378430
while time.monotonic() < deadline:
379431
addr = pac_address(mem, push.stage.number)
380432
if addr is None:
381433
time.sleep(poll)
382434
continue
383435
caught = _watch(client, mem, push, addr, deadline, poll)
436+
if caught == "moved":
437+
continue
384438
if caught is not None:
385439
client.resume()
440+
if isinstance(caught, tuple):
386441
here, ok = caught
387442
log(f"caught the reload at 0x{here:08X}; re-applied, verified: {ok}")
388443
return True
444+
if caught == "reused":
445+
log(
446+
f"{label}'s slot at 0x{addr:08X} went to another file: nothing caught; push"
447+
" again with --hold to carry the edit across the next transition"
448+
)
449+
return False
450+
break
389451
log(f"nothing caught in {seconds:.0f} s: the PAC was not re-read")
390452
return False
391453

392454

393455
def _watch(
394456
client: Client, mem: Memory, push: Push, addr: int, deadline: float, poll: float
395-
) -> tuple[int, bool] | None:
396-
"""One watch on the tail of the PAC at `addr`: (where, verified) with the CPU left stopped,
397-
or None once the PAC moves or time runs out."""
457+
) -> tuple[int, bool] | str | None:
458+
"""One watch on the tail of the PAC at `addr`, removed before this returns:
459+
(where, verified) with the CPU left stopped on a reload of this stage; "reused", the CPU
460+
left stopped, when a write there is another file's; "moved" when the PAC moved; None
461+
when time ran out."""
398462
stage = push.stage
399463
tail = max(b.offset + len(b.original) for b in push.mesh) - 4
400464
ident_at = stage.table[TERRAIN][0] + IDENT_AT
@@ -406,12 +470,11 @@ def _watch(
406470
except TimeoutError:
407471
now = pac_address(mem, stage.number)
408472
if now is not None and now != addr:
409-
return None
473+
return "moved"
410474
continue
411475
here = pac_address(mem, stage.number) or addr
412476
if mem.read(here + ident_at, IDENT_SIZE) != ident:
413-
client.resume()
414-
continue
477+
return "reused"
415478
for b in push.mesh:
416479
b.apply(mem, here)
417480
return here, all(b.verify(mem, here) for b in push.mesh)

0 commit comments

Comments
 (0)