mirror of
https://github.com/jkdansereau/goldeneye-pc-port.git
synced 2026-10-23 15:52:42 +02:00
D243 M-168: render_pos freeze -- 2 crashes fixed, stable but incomplete
Extended M-167's freeze technique to render_pos (Bond's own draw position), aiming at the model-duplication symptom M-167 didn't fix. Two live crashes along the way, both from the same root cause: model pointers get reused by the allocator across different logical model instances, so any attempt to skip subcalcmatrices() based on inspecting render_pos's value or caching pointer identity can be fooled into treating a brand-new, uninitialized instance as already-validated -- leaving render_pos at its fresh tombstone value for a later consumer (sub_GAME_7F06C768, computing zDepth) to crash on. Fixed by never skipping subcalcmatrices() at all -- it always runs now, guaranteeing render_pos is never left invalid regardless of model lifecycle. The freeze is purely a contents copy on top of a now-guaranteed-valid buffer. Live-playtest stable (no crash, full session). But the result is informative-negative: freezing unconditionally also suppresses the LEGITIMATE per-shot camera/model repositioning (AI_TRYTeleportingChrToPad moves Bond between several distinct shots during the cutscene), not just the illegitimate intra-shot cycling -- so the camera now looks the wrong direction for later shots and Bond still appears stuck. Both GE_D243X3/GE_D243X4 remain diagnostic experiments, not real fixes. Documented two next-step candidates (delta-threshold re-baseline, or hooking the teleport call directly) for distinguishing legitimate repositioning from the cycling bug. porting-notes.md gets the generalized lesson: freezing a value and freezing a pointer target are different operations -- the latter needs the call that keeps the pointer valid to always run, with the freeze applied to contents afterward, not to whether the call happens.
This commit is contained in:
1 parent
53219c18ae
commit
d2caaa09bc
3 files changed
+90
-1
No files matched your search
@@ -1150,6 +1150,8 @@ produce the same visual symptom but need opposite fixes.
|
||||
|
||||
**Update (same D243 investigation, later in the same project's life): fixing the field you found the bug in doesn't mean you've fixed what's rendered — check which field the CONSUMER actually reads.** Having found `chrTick` overwrites `prop->pos`, two live-playtest cycles were spent neutralizing exactly that overwrite (save/restore `prop->pos` around `chrTick`) — it worked completely (confirmed in logs: the field went smooth) and yet the visible symptom (camera shake) was completely unchanged. The reason: the camera's look-at filter doesn't read `prop->pos` at all — it reads `field_488.pos`, a *different* field written from a *different* data path (`render_pos`, the model's animated-skeleton transform, computed independently of `prop->pos` inside the same function). Both fields happened to show the same "period-4 cycling" symptom, which made them look like one bug through a probe that only logged `prop->pos` — they were two independent outputs of the same underlying animation-tick call, and only one of them was what actually got rendered. **The generalizable check, sharpened:** once you've confirmed a field is being overwritten, don't assume that field is what the visible symptom's consumer (a camera filter, a draw call, a collision check) actually reads — trace the consumer's own read site explicitly and confirm it's the same field before spending a live-test cycle suppressing the wrong one. A quick static check (grep the consumer's exact read expression) is far cheaper than a live playtest.
|
||||
|
||||
**Update 2 (same investigation, immediately after): freezing a *value* and freezing a *pointer target* are different operations — treating them the same crashed twice.** Extending the same freeze technique to `render_pos` (a `Model*` field, unlike `field_488.pos` a plain value) by skipping the call that reassigns it (`subcalcmatrices`) seemed equivalent to M-167's "skip the write" but wasn't: `render_pos` is a pointer into a per-tick dynamically-reallocated arena, and skipping its reassignment only works if the pointer it's left holding is still valid — which depends on model lifecycle, not just on this tick's logic. Two fixes in a row tried to detect "is it safe to skip" by inspecting either the pointer's *value* (a sentinel check, `!= NULL && != -1`) or a *cached identity* (comparing the pointer against a remembered "already validated" pointer) — both crashed, because **the allocator reuses model-pointer addresses across genuinely different logical model instances**: a freshly-`CREATE`d model can land at the exact address a previously-removed model occupied, so both the sentinel check and the identity check wrongly conclude "this is already valid" for an instance whose `render_pos` is still an uninitialized tombstone. This is the same "don't guess validity from a value that can coincidentally look right" trap as the paragraph above, encountered a second time from a different angle (a *pointer's* target identity, not a *field's* content). **The robust fix:** never skip the call that establishes the pointer's validity — let it always run — and apply the freeze as a *contents* copy afterward, on top of a buffer that's now unconditionally guaranteed valid. If you must "hold something steady" and that something is reached through a pointer that some other code reassigns, freeze the *pointee's bytes*, not the *act of assignment* — the assignment is usually also the thing keeping the pointer sane.
|
||||
|
||||
## E. Process / method notes
|
||||
|
||||
- Investigation loop is: reproduce → env-gated capped probe → root-cause
|
||||
|
||||
Reference in new issue
Block a user