Repository navigation
[8/8] Add patch watches, RAM auditing and debugger tools - #120
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5616c4258e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| template <Squirk Q> | ||
| void Ketchup<Q>::Audit() |
There was a problem hiding this comment.
When a foreign writer corrupts a mirrored executable region, this diagnostic can never report it: repository-wide call-site search finds only explicit template instantiations of Audit(), while the per-frame _SQReturn_update_gadgets path calls Update() directly, which silently repairs the bytes. Because Audit() is also private, it cannot be invoked as an external explicit diagnostic; schedule it at the intended cadence before repair so RAM-corruption investigations produce the promised warnings.
Useful? React with 👍 / 👎.
| // Watches first: a watch is a report, and must see the patch whether or | ||
| // not a filter below then drops it. | ||
| for (auto &watch : PatchWatches) | ||
| { | ||
| if (offset < watch.start || offset >= watch.end) continue; |
There was a problem hiding this comment.
When DisableCDROM = true, either early return immediately above this loop exits before any watch is evaluated, despite the new comment saying watches must see patches that a filter drops. This makes the diagnostic omit every collection write in precisely the configuration used to suppress those writes; move the watch processing ahead of both master-filter checks.
Useful? React with 👍 / 👎.
| // span = the retail `option` stage, sectors 27136..27210, through | ||
| // (lba + fo / 2048) * 2352 + 24 + fo % 2048, with STAGE.DIR at LBA | ||
| // 136654 on disc 1 and 105178 on disc 2. | ||
| SQHook<Squirk::Standard>::SetPatchWatch(0x16F634B8ull, 0x16F8E5C8ull, |
There was a problem hiding this comment.
Doesn't this need to be guarded, to check that the running title is Integral? None of this needs to be applied for the other regions etc.
There was a problem hiding this comment.
Would you like me to fix this?
5616c42 to
ed6744e
Compare
9c32b64 to
87c1230
Compare
3cdd344 to
51a23c9
Compare
SetPatchWatch matched on the disc-image offset alone, so a CD-ROM patch from any title landing inside one of the Integral windows was reported under an Integral label. Nothing about these offsets is title-specific. Key each watch to a title, version and disk - the way SetPatchRangeBlacklist already keys its ranges - and test that key when the patch is submitted, since registration in Load() runs before the title is known. Version as well as title and disk because Integral's game discs and its VR disc share title 99 and disk 0.
51a23c9 to
e56c5ad
Compare
Add optional collection-patch watches, explicit RAM auditing and debugger bridge tooling for investigating the patch paths introduced earlier in the series. Keep these diagnostic additions in their own review. This completes the full repository contribution, including the historical documentation introduced in PR 6.
Review order
PR 8 of 8 in the complete TideGear contribution. These are an ordered stack against upstream
master; GitHub shows earlier unmerged changes cumulatively. Merge in order 1 through 8 and update the remaining branches after each merge.Previous PR: #119
Review only this step
Validation
Ready for maintainer review. The documented gameplay testing remains outstanding; no gameplay pass is claimed.
Complete series