Fix what the verification campaign and the blind-user test found - #61
Merged
Conversation
The only worked example wrote to the "german" key. Where an id carries both "german" and "german_new" the game displays german_new, so following the example on a dialog or journal line rewrote the cache, reported "Applied 1 edit(s)", and changed nothing on screen. The guide never mentioned that the catalog carries two generations of German and three of English. Both examples now teach the rule by contrast, and a new section states it with the evidence behind it: observed on BuildID 24340829, in both directions, one install, one sitting. Also documents the split failure mode. An id with only german_new fails loudly through `gore loc import` but is dropped silently by a bundle deploy, because crates/gore-mod/src/lib.rs discards the set_value Result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Item and stat values are the toolkit's headline path and the only domain applied at runtime, so they need UE4SS in the game install. The guide said only that the mechanism "requires UE4SS to be installed and enabled" and never said what that is, how to check for it, or where it comes from. Nothing detects its absence either: gen and mod build produce a well-formed mod regardless, and mod deploy creates the ue4ss root itself, so a successful deploy looks identical whether or not anything will ever run the Lua. Adds a section covering what it is, that GORE neither installs nor checks it, how to tell whether you have it, and how to confirm afterwards from UE4SS.log that an override actually applied. The canonical download link is a marked TODO: it is recorded nowhere in this repository and a guessed one would be worse than none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…_Value Three fixes to the page. Finding class names pointed at apps/save-editor/assets/*_catalog.json, a path in this repository that no release install has. Documents `gore as default-sites` instead, which works from a release install, including two traps it carries: it spells classes with the UE `U` prefix (both spellings work, per runtime_class_name), and its module= column is the AngelScript module rather than the override's module field. m_Value was described only as "item value", which does not say whether it is what you pay or what you get. It is the number both derive from; measured readings and their conditions are now on the page, scoped to the one trader and difficulty they were taken at. Overrides land a few seconds after launch, not at it, because the target CDOs do not exist yet. Judging immediately shows nothing and reads as a broken mod. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SFX_UI_Action_MenuButton_Click_01..04 are one variant set the game picks from at random, and they print as unrelated rows thousands of indices apart. Replacing only _01 produces the intended sound on roughly one click in four, which is easy to mistake for a tool that does not work. Those samples also fire when backing out of a submenu with Escape, so the name is narrower than the behaviour. Button_Hover_01 does match its name. Every subcommand demands a --bank path and nothing lists the banks, so the page now names the directory and the four banks that carry samples. Records the in-game listening session for what it is, and leaves open which of Music.bank's title and title_MASTER backs the title event, since both were replaced in the same pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`gore gen --model` rejects unknown classes, unknown fields and type mismatches. `gore mod build` — the path the guide recommends — checked nothing at all, so a spec with a misspelled class produced a well-formed bundle whose Lua never resolves. The only report of that is a "gave up" line in UE4SS.log after 120 retries, two minutes into a play session. Two blind testers hit this; one assembled twelve class names by hand and had no way to find out any were wrong. Adds an optional --model to `gore mod build` running the same check, and moves that check into a helper both commands call so the two paths cannot drift into different verdicts or different error texts. Without --model the build now says on stderr that the names went unchecked, and still builds. A model is opt-in per invocation rather than compiled in: the only model tracked here is the GUI-shaped item catalogue, covering 798 It* classes with int/float/bool fields, while overrides are documented to work on item, NPC and ability classes. Compiling that in would report "unknown class" for two of the three categories the feature advertises, and a warning that is wrong for whole legitimate categories teaches people to ignore warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lcache is sparse: a language being declared does not mean every id carries it. The deploy path guarded on "is this language declared" and then discarded the result of the write, so an edit naming a declared language the target id has no slot for vanished silently — the bundle deployed, the backup was taken, the cache was rewritten, and the line in game was unchanged. The standalone `gore loc import` reports that same miss by name and exits non-zero, so the two paths disagreed about the same edit. This is how a mis-targeted translation became indistinguishable from a broken tool, and German makes it easy to hit: 31,590 ids carry german_new without carrying german. Collects the skipped edits and reports them after a successful deploy instead of refusing, so one unusable edit in a large bundle does not block the rest. The list rides back on DeployRecord under serde(skip): it describes one run, not the deployment's state, and the on-disk record is read back by undeploy and status. The neighbouring skip for a language this install does not declare at all is unchanged and still silent — that one is deliberate, and its comment says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Where an id carries both german and german_new the game displays german_new, so an edit to german lands in the cache and is never seen. 2,147 ids in the shipped cache are in that position, and the guide's own worked example pointed at the losing key until the previous commit. Deploy now says so. The generation rank comes from the suffix (bare < _new < _newer) rather than a list of names, so another generation needs no code change; only the ordering is assumed, and that came from watching the game. Suppressed when the same bundle also writes the winning generation for that id. That is the practice the guide recommends, and a warning that fires on every correct edit is one people learn to ignore. Adds Lcache::languages_for, since the records are sparse and the header's language list does not tell you what any single id carries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four "what is proven, and by what" sections were written when the only in-game evidence was a sighting or two recorded in commit messages. A campaign on 2026-08-07 against Steam BuildID 24340829 checked nine domains by hand, so those sections had drifted into understating what is known — which costs the same credibility as overstating, because a reader who finds the pessimism wrong stops treating it as a signal. textures: the logo replacement reproduced, on newer code than the first sighting; T_Logo is not drawn on the main menu; a 4K PF_BC7 container mounts and renders; undeploy confirmed on screen. Says plainly that pak_files was built but never deployed, and that nothing was checked on any other build. dataassets: had no proof section at all. Now records the first time anything in this domain was seen to take effect — one float32 leaf on W_SettingsRow, one asset, one build, one look. voice: a replacement has now been heard, and the deliberately untouched line between the two replaced ones played normally, which is what shows the edit is per-entry. Also documents the no-encoder route: copy a recording out of another language archive as the payload. bundles and mod-manager: three deploy mechanisms from one spec; undeploy byte-exact and residue-free across seven sequential runs; mgr analyze named the winner and the game agreed, and order flipped both. The 4K texture is recorded as UNCOMPRESSED evidence. The bundle path passes compress=false, and the deployed .ucas was 598 bytes over the raw mip, so that run says nothing about the compressed writer. Picks the shortest guide page for the whole-page read test: textures grew past the reader's one-part budget, and the test had been asserting on a named page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mation The consent gate works out what a call would overwrite, and for `mod build` that means reading the mod name out of the spec. Every way that read could fail — file missing, not JSON, no name at the pointer — collapsed into one "could not check", which the gate treated as "something might be in the way" and raised a confirmation about. A client that answers its own dialogs turned that into "refused: the confirmation came back no", so a misspelled field read as a permission problem and sent the caller looking in the wrong place. None of those calls could have run: the child reads the same file and fails on it before writing anything, so there was never anything for a person to allow. They are now rejected before the gate is consulted, naming which of the three went wrong and pointing at the guide page for the file's shape. A name that reads fine but is not a single path component still raises the question. That one the child does run, and the gate genuinely cannot tell where the write would land. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects with one cause: auditioning candidate samples is the normal way into this command, and both the filter's absence and the directory rule fought it. `extract` had no --filter while `list` did, so pulling a variant set meant one --sample call per member, each needing its own output directory — or extracting all 7,218 samples of a 260 MB bank. It now takes the same case-insensitive substring `list` does. The MCP gate then asked for confirmation whenever the output directory was non-empty, which after a first extract it always is. The protection behind that was real — names come from the bank, so this layer cannot know what a run will write, and fs::write truncates. The check moves into the CLI, per file: an existing target is refused by name instead of replaced, and a directory holding earlier extracts is no longer a reason to ask about anything. Both halves ship together. Dropping the facet against an older CLI would leave neither layer checking. `catalog stubs` and `as emit-all` keep it — same shape, but neither has a workflow that fills its own output directory. `gore audio banks` is still missing, so a caller must still know where the banks are. The guide names the directory; the command is unwritten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… finds symptoms Category C from the blind-user findings, in one branch. `gore audio banks` lists the banks an install carries with each one's sample count. Every other audio subcommand demands a --bank path and nothing produced one; a tester had to search the filesystem before his first call. It decrypts 60 bytes per bank rather than calling read_bank, so describing the directory costs 0.2 s instead of decrypting 260 MB ten times, and the six banks that carry no samples are rows saying so rather than omissions. `gore doctor` answers "what is my setup and what is wrong with it": where the game is and where that came from, whether UE4SS is there at all, which UE4SS mods are enabled and might compete, what is deployed, what an interrupted run left behind, whether the game is running, and whether the shared text catalog still matches. There was no diagnostic command of any kind, and the tester who arrived with "the tool said it worked, the game shows nothing" had to open PowerShell and rummage. That is the moment people abandon a tool. Voice now says what it wanted. A WAV handed to `voice replace` produced `invalid Ogg capture pattern at byte 0`; a voice actor read that and could not act on it. The error detects the common source formats and gives the conversion line, and the guide documents the encoding step the toolkit does not perform. Guide search ranks the guide above the reference pages unless the query is written in the reference's own vocabulary. "deployed but nothing changed in game" used to return Mod Studio internals; the section that answers it is titled "Deployment reality check" and search could not find it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The toolkit could enumerate what exists and never say what any of it does. Every blind tester hit that wall from a different side: three samples all named like the menu click, four sword textures with no hint which one the material reads, no way from a voice file to the spoken line, 43,898 localization ids and no search. And `items.md` answered "how do I find class names" with a path inside this repository, which no release install has. Two layers, because they need different trust. Catalogs enumerate, are machine derived, and any wrong entry is caught by the next tool run. The register records what a thing was observed to do in game — which only a person at a screen can establish, since nothing here ever sees the screen. That asymmetry is enforced rather than documented. An observation may claim `confirmed` or `refuted` only if it carries the observer's own words; without them the loader degrades it to `unconfirmed` and keeps what the file claimed in `degraded_from`, so the override is visible. Same idea as the consent gate's `user_approved`: an assertion is not blocked, it is marked. Status is derived from the observation list, never stored, so it cannot drift from its own evidence — including `disputed` when observations disagree, which is surfaced rather than resolved: it usually means a build, a language or a setting, and resolving it silently would throw the finding away. Corroboration is counted, because ten confirmations across three builds is a stronger claim than one note. Seeded with 32 entries from the 2026-08-07 campaign and nothing else: 27 confirmed, 3 refuted, 2 unconfirmed. The refutations are the ones nobody else produces — T_Logo deployed and not seen, ui_main_play bound to no visible button. `gore find` searches both layers across domains. It matches ids, categories and register text always, and display names only when the shared text catalog has been extracted — and says which, every time, because a name search that silently skipped its index answers "no such item" about an item that is there. Provenance is a property of the file and never blended in output. Only `bundled` ships; `community` and `local` load from the start so that opening a contribution path later is not a migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
One launch, two questions, both answerable from the main menu. The pak_files route works. All eight cursor PNGs shipped as a 2,312-byte override pak into ~mods\ and the pointer was magenta, where replacing the same eight files loose had changed nothing. An override pak beats the base pak. That settles a mechanism rather than a cursor, and the reach is wider than the example: G1R\Config has no loose copy anywhere, so every .ini the game reads, DefaultEngine.ini included, exists only inside a pak. This was the only route to any of them, and it had never been observed to work. The title event draws on title_MASTER, not title. The first pass replaced both at once and heard a siren, which proved the event uses at least one of the two without saying which; replacing title alone left the menu playing its normal theme. Recorded as what it is — elimination across two witnessed observations, not a direct test, since title_MASTER was never replaced on its own, and it does not rule out further samples. Register and guide both updated; the two entries that stood as unconfirmed become one refuted and one confirmed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every entry and every guide passage said the observations were made on Steam
BuildID 24340829. They were made on 24539464. The game updated automatically on
2026-08-06 at 22:25 — the executable and G1R-Windows.{pak,utoc} carry that
timestamp — and the campaign ran the following morning. The build id came from a
scout that had read the manifest two days earlier, and was never re-read.
The observations themselves are untouched: magenta was magenta and the beep was
a beep. Only the label was wrong. But the label is the axis the register uses to
decide whether an entry still applies, so a wrong one is worse here than
elsewhere.
Two claims needed more than a new number.
Textures: the pass was described as reproducing the original logo sighting "on
the same build". It reproduced it on a NEWER one, so the pipeline is now known
to have survived a game update rather than to have worked twice on one build.
That is a stronger result than the one previously written down.
DataAssets: the triplet was extracted and packed two days before it was
deployed, against the containers of the preceding build, and it still applied
after the update. Worth knowing, and not the same as having been produced
against the build it ran on. Both pages now say so.
Pre-existing references to 24340829 are left alone — the qualification record,
the reference pages and the gore-as fixtures mean that build legitimately, and
so do the two sentences here that describe the earlier sighting.
Found because `gore asset pack` refused a stale patch with
ASSET_GENERATION_MISMATCH after the install moved. The generation seal caught
the change that I had not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t reach The domain had one sighting: a float32 leaf on a UMG widget. Two generalisations were riding on it untested — whether a leaf kind other than float32 takes effect, and whether anything outside the UI does. Both are now observed, in one launch and with one edit. /Game/Blueprints/TrackingSystem/FootstepsPresets/DA_HumanFootsteps is a DataAsset proper (class /Script/G1R.FootstepTag, native rather than Blueprint-generated), and FeetTextureSize is a vector4_f64x4 reached through a struct inside a struct. Raised from (16,16,0,1) to (160,160,0,1), the hero's footprints came out roughly ten times too large. The settings-row triplet was left deployed beside it as the control, so "no change" could be told apart from "nothing was deployed". Provenance is clean this time: extracted, patched, packed, deployed and observed all on build 24539464. Three limits found while getting there, none of which the guide mentioned: Blueprint-generated exports cannot be bound at all — the USMAP describes native classes, so an export class ending in _C has no schema. DA_GlobalDaySettings is one, and no leaf choice changes that. One unproven struct costs its whole export. MPC_StencilColor is native and binds cleanly, then the walk stops at `struct "Guid" has no proven G1R UE5.4 wire form` and yields nothing. A second edit to the same package is not reachable. patch-fixed requires the extract receipt, that receipt seals the pristine pair, and after one patch the pair no longer matches. The patch receipt is a different document and is not accepted in its place — so one fixed-width change per package is the ceiling. The guide said to re-inspect before a second edit, which described a workflow that cannot be executed. Both refusals are recorded in the register as refutations, so `gore find` answers "why did this come back empty" without a second investigation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
![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: 4f19e3a7f7
ℹ️ 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".
**An output aimed inside the installation stopped being seen as one.** `output_paths` is assembled from the safety facets, so removing `clobbers_dir` from `audio extract` — done to stop it asking about its own previous output — also took `out` out of `installs_into_game_tree`. An MCP caller could have pointed `--out` at the game folder and filled it with thousands of WAVs without anybody being asked. Adds `Safety::writes_into`, which answers "is this an output" without also answering "is something in the way", and puts `audio extract` back under the install-path check. The command no longer needs its exemption in the output-coverage test, so that exemption is gone too. **A collision left partial output behind.** The refusal lived inside the write loop, so a clash on a later sample failed only after the earlier ones were on disk — and the obvious retry then failed on one of those instead of on the file the caller had to deal with. Every destination is now settled before the first write, so the failure leaves nothing. **`doctor` reported clean when it could not look.** `file_names` swallowed both read_dir failures and per-entry errors and returned an empty vector, which `~mods` read as "empty" and the backup scan read as "nothing left over". In exactly the damaged or inaccessible installs this command exists for, it said ok. A directory that is absent still answers "no files"; one that cannot be read now reports that it could not be read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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: a7e82c08ea
ℹ️ 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".
**A dropped directory entry made a listing lie about being one.** `audio banks` kept an unreadable bank as an error row on purpose — one damaged file must not cost the other nine — but silently discarded entries `read_dir` itself failed to yield, then printed a bank count, a sample total and a "complete directory listing" describing only what it managed to enumerate. Now every entry or none. **`doctor` hid competing override mods when it could not read.** `read_ue4ss_mods` swallowed both the directory failure and per-entry errors and returned an empty list, so the check reported ok with "no mod folders" — the one answer that hides a stale override mod, in precisely the install this command is asked about. Same treatment as the `~mods` and backup scans in the previous round. **A skipped sample could block an extraction it was never part of.** The pre-write pass planned a destination for every selected sample, but a sample whose codec `extract_wav` cannot read writes nothing, so a leftover WAV belonging to one of those refused the whole run with "would replace it" when no overwrite would have happened. Extraction now stages each WAV beside its destination and renames them in together: what is staged is exactly what was produced, the collision check sees only those, and a failure at any point leaves neither partial output nor staging files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f55023b1d
ℹ️ 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".
…length **Publishing a WAV had a check-then-write race.** The staging pass scanned for an existing destination and then renamed into it, and `std::fs::rename` replaces an existing file on every platform this ships to — the comment on `write_atomic` in the same file says exactly that. An editor, a watcher, or a second extraction that created the file in between would have lost it, despite the command's promise never to overwrite. Staging is gone. Each WAV is now published with `create_new(true)`, which asks the filesystem the question and takes the file in one operation, so there is no window. That also keeps the two properties the previous rounds established: the destination is opened only once a sample has actually produced audio, so a skipped codec cannot refuse a run over a leftover it would never have written; and a failure rolls back whatever this run created, so nothing partial survives. **The catalog staleness check could not see a same-length rewrite.** `gore loc import` re-encrypts in place, and a replacement string that encodes to the same number of bytes leaves the file exactly as long as it was — so comparing `metadata().len()` reported the shared catalog as current while it still held the old text. `gore find`, Mod Studio and the save editor all read that catalog, so this was the one condition the check exists to catch. Compares the modification time as well. A content hash would be better and is not available: the catalog records no digest, and adding one changes a format two other applications read. The `ok` wording now says what was actually checked rather than claiming a match. The test fixture left `extracted_at` at the epoch, which made every file look rewritten since; it now takes the source file's own mtime, which is the order reality has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c5f3d5bd8
ℹ️ 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".
someone else's file, and two outcomes reported as one **`--sample <name>` with `--filter` answered half the request.** The first match arm kept the named sample and dropped the filter, so a caller who passed both got a successful extraction that ignored part of what they asked for. Rejected now, with a message that says which flag does what. `--sample all` stays legal alongside a filter: that is the default, and it means "no sample selection". **Rollback removed published files by pathname alone.** `create_new` stops the creation from clobbering anything, but rollback happens later — so an editor or a watcher that replaced an early WAV during a long extraction could have had its file deleted by a failure it had no part in. Rollback now removes a file only while it still has the length this run wrote. Same-length replacement is not distinguished, and the comment says so. **A landed edit was reported as one that did not apply.** `loc_warnings` held two different outcomes in one list: an edit the id has no slot for, which was never written, and an edit that WAS written but sits in a generation the game does not read. Counting them together made the message claim edits "did not apply" when they had, which invites undoing a deployment that worked — and one edit can raise both, so the count was wrong as well. They are now separate lists with separate wording: a warning for what could not be written, a note for what was written and will not be seen. The distinction is the whole point, because one calls for changing the spec and the other does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
Owner
Author
|
@codex review |
"Installing a mod GORE did not build has one end-to-end run behind it" parses, and that is about all it does. "One end-to-end run behind it" is the phrasing I have been using in commit messages all day; it means "somebody tried it once", and that is what it should have said. Two neighbours had the same problem. "Adding a new line's audio is built but has never been heard" made the reader work out that "built" meant the code exists; "sit somewhere this route cannot reach" left "this route" to be inferred.
Owner
Author
|
@codex review |
I wrote that installing other people's mods had "only been tried once". It has not been tried at all. The one end-to-end run used a triplet `gore asset pack` produced, with no `gore-mod.json` in it -- "foreign" in this code means "not in the GORE bundle format", not "made by somebody else", and I read the word the way a reader of the README would have. The capability is real and stays: the manager accepts plain zips and folders, loose pak files, IoStore triplets, UE4SS mod folders and raw file replacements. What it has never seen is a mod written by another person, and that is now what the catch says.
Owner
Author
|
@codex review |
The quick start ends with a UE4SS mod. Without UE4SS that mod installs cleanly and then does nothing, with nothing anywhere reporting a problem -- which is the single confusion this branch was opened to remove, and the quick start walked straight into it. `gore doctor` goes where the guide already puts it: after the game path is set and before the first mod. It says whether that path is the game, whether UE4SS is there, what is deployed, and what an interrupted run left behind. Checked against a real run rather than from memory: every non-`ok` line does carry its `fix:` line, including the one whose evidence list is long enough to push it out of a short grep window.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 340ec6c530
ℹ️ 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".
`extract_wav` clamped each sample's range to the block it was given, so a truncated bank produced a WAV missing its tail and `gore audio extract` reported that as a success. Nothing about the file looks wrong until somebody plays it, which is the worst shape a failure can take -- and it is the shape this branch has spent its whole length removing. Worse, `header_fits` cited that clamp as the reason it does not check the audio extent itself: "Audio declared past the end of the block is still caught, by `extract_wav`". It was not caught anywhere. The division of labour is real now -- this one checks what can be read, that one checks what can be played -- and the comment says so instead of asserting it. Checked against the install rather than the fixtures alone: all ten banks still summarize, `list` still reads VO.bank, and `extract` still writes both its samples.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21569851af
ℹ️ 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".
The last commit stopped the PCM path from handing back a shortened recording and left the Vorbis path clamping exactly as before. That is the arm that matters: every bank the game ships is Vorbis, so the fix covered the case the fixtures can build and missed the case the install actually has. Clamping there is worse than in PCM. The shortened range is still a complete sequence of packets, so `extract_packets` accepts it and the remuxer closes it with an EOS page -- a playable Ogg, missing its tail, reported as a success. The fixture builder only makes PCM16 banks, which is why the earlier test could not have caught this. The new one drives `extract_ogg` with a hand-built `Fsb5` whose sample declares twice the bytes present, and a control with the declared bytes there, which fails on the payload instead -- a different sentence about a different thing. Checked on the install: VO and CINEMATICS still extract, SFX still lists 7218.
Owner
Author
|
@codex review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…of it `bank_shape_within` had its own inline chunk walk, so the probe bound added to `find_top_list_with` never applied to the in-memory route: a damaged 260 MB bank whose top-level chunks all declare zero still stepped through it eight bytes at a time, about 34 million times. Bounding the copy would have left two walks to keep to one rule by hand. It uses the shared one now, with `TopWalk::Any` — which is exactly what the loop did, follow every declared size with no guard on the fourcc, and the reason that variant exists at all. Verified against the install: `audio banks` is unchanged on all ten banks, same 7443 samples, same rows. The only JSON difference against the older baseline is the `occupied_names` field added earlier today.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2699445f43
ℹ️ 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".
… dangling links Three from one review, two of them the same distinction in the last two places that still got it wrong. `bank_shape_within` searched for the SNDH bounded by the end of the BUFFER, not by the end of the LIST it was walking. An SNDH lying outside that LIST still counted -- and then the two routes disagreed, because the on-disk one reads a prefix that stops where the LIST says it does: `audio banks` reported "no SNDH" while `bank_summary` and `audio list` accepted the same bytes. A chunk outside the chunk that contains it is not part of the wrapper, so the walk is bounded by the LIST now and the prefix is sufficient by construction. `check_mods_folder` kept its own two-case copy of the occupant question, so a dangling `~mods` link resolved to nothing, came back as absent, and the check said `ok` -- while `container::deploy` cannot create `~mods` while a link holds the name. It goes through `occupant` now, which is where the third case lives. `audio banks` had the same hole at the FMOD directory: a dangling link reports `NotFound`, so it fell into "verify the game files", which cannot create a directory whose name is taken. Verified on the install rather than on fixtures alone: all ten banks summarize byte-identically to before, and SFX still lists 7218 Vorbis samples.
Owner
Author
|
@codex review |
The version I committed with the fix passed with the fix reverted, which makes
it a test of nothing. The fixture is a few hundred bytes and the default probe
is 64 KiB, so the prefix was the whole file either way and there was no
divergence left to detect.
Driven through the injectable probe now. With a small first read the prefix is
exactly what the LIST declares, which is what a bank larger than the probe does
on its own. Checked by reverting the bound and watching it fail: `Err("SNDH
entry out of bounds")` from the on-disk route against `Ok(Samples { .. })` from
the in-memory one -- the disagreement the report described.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7a59c4c0c
ℹ️ 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".
…overstating identity The dangling-link tests cleaned up with `remove_dir`. Windows makes a directory symlink, which is right there; Unix makes a symlink object, which `remove_dir` refuses with `NotADirectory` -- so both tests panic on Linux and macOS after their assertions have already passed. CI runs Windows only, so it was green here and broken everywhere else. Both sites remove it the way the platform made it. I then tried to narrow the quarantine check-then-delete window with a metadata identity check in front of the unlink, and measured before believing it: NTFS file tunneling hands a file deleted and recreated under the same name its old creation AND last-write time. Both stamps came back identical across a delete-and-rewrite on this machine. The check cannot see the replacement it exists to catch, so it is not shipped. That measurement also contradicts what `identity_of` claimed -- that a replacement would have to be created "at the very same tick". It would not; the filesystem hands it the old one. The doc says what was measured, and says where the check is still worth taking: the partial-write path cannot hash, because what is on disk is a prefix of unknown length. The remaining gap is stated where it lives. Nothing in `std` closes it: every operation resolves the path again, and deleting the object a handle refers to needs `SetFileInformationByHandle` or `unlinkat`.
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Owner
Author
|
@codex review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
7 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9a5ccaa. Configure here.
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything here comes from two exercises run against the real thing: a verification campaign that
built mods, deployed them into a real install and had a human look at the screen, and a blind-user
test in which eight agents with no knowledge of this project tried to do ordinary modding tasks with
nothing but the plugin.
The campaign moved several domains from "structurally proven" to "seen in game". The blind test
found 10 blockers and 40 major frictions, and — the number that mattered most — the first thing a
newcomer reached for was the right tool only 3 times out of 8.
What was broken, and is now fixed
Localization edits could fail silently, in two different ways. The cache carries two generations
of German and three of English; where an id has both, the game displays
german_new. The guide'sonly worked example wrote to
german. Following it on a dialog or journal line rewrote the cache,reported "Applied 1 edit(s)", and changed nothing on screen.
Underneath it was a real defect: the deploy path guarded on "is this language declared" and then
discarded the result of the write, so an edit naming a declared language the target id has no slot
for vanished without a word — while the standalone
gore loc importreported the same miss by nameand exited non-zero. 31,590 ids carry
german_newwithout carryinggerman, so this was easy tohit. Both paths now agree, and an edit aimed at a shadowed generation says so.
gore mod buildvalidated nothing.gore gen --modelrejects unknown classes, unknown fieldsand type mismatches; the bundle path — the one the guide recommends — accepted anything. A blind
tester assembled twelve class names by hand and had no way to learn that any were wrong. It now
takes the same
--modelthrough the same helper, and says on stderr when it checked nothing.A broken spec was reported as a refused confirmation. The consent gate reads the mod name out
of the spec to work out what a call would overwrite. Every way that read could fail collapsed into
"could not check", which the gate raised a question about — and a client that answers its own
dialogs turned that into
refused: the confirmation came back "no". A misspelled field read as apermission problem. Those cases are now rejected as what they are, before the gate is consulted.
UE4SS was an undocumented prerequisite. Item and stat values are the toolkit's headline path and
the only domain applied at runtime, so they need a third-party loader that GORE neither installs nor
checks for.
mod deployeven creates theue4ss\Mods\directory itself, so a successful deploylooks identical whether or not anything will ever run the Lua. The guide now says what it is, how to
check for it, and how to confirm from
UE4SS.logthat an override actually applied.Finding class names sent readers into this repository.
items.mdpointed atapps/save-editor/assets/*_catalog.json, which no release install has. Two testers were strandedthere.
What is new
gore find— one lookup across the bundled catalogs and a new effect register.The effect register. The toolkit could enumerate what exists and never say what any of it does.
Two layers, because they need different trust: catalogs are machine-derived and any wrong entry is
caught by the next tool run; the register records what a thing was observed to do in game, which
only a person at a screen can establish, since nothing here ever sees the screen.
That asymmetry is enforced rather than documented. An observation may claim
confirmedorrefutedonly if it carries the observer's own words; without them the loader degrades it to
unconfirmedand keeps what the file claimed in
degraded_from, so the override stays visible. Same idea as theconsent gate's
user_approved: an assertion is not blocked, it is marked.Status is derived from the observation list and never stored, so it cannot drift from its own
evidence — including
disputedwhen observations disagree, which is surfaced rather than resolved.Seeded with 32 entries from the campaign and nothing else: 27 confirmed, 3 refuted, 2 unconfirmed.
The refutations are the ones nobody else produces —
T_Logodeployed and not seen,ui_main_playbound to no visible button.
Only
bundledships.communityandlocalload from the start so that opening a contributionpath later is not a migration.
gore doctor— where the game is and where that came from, whether UE4SS is there at all, whichUE4SS mods are enabled and might compete, what is deployed, what an interrupted run left behind,
whether the game is running, and whether the shared text catalog still matches. There was no
diagnostic command of any kind, and the tester who arrived with "the tool said it worked, the game
shows nothing" had to open PowerShell and rummage.
gore audio banks— every audio subcommand demands a--bankpath and nothing produced one. Itdecrypts 60 bytes per bank rather than calling
read_bank, so describing the directory costs 0.2 sinstead of decrypting 260 MB ten times.
gore audio extract --filter, and a per-file collision check replacing the whole-directoryconsent rule that fired on the second extract of any ordinary session.
A readable Ogg error. A WAV handed to
voice replaceproducedinvalid Ogg capture pattern at byte 0. A voice actor read that and could not act on it. It now detects the common source formatsand gives the conversion line.
Guide search that finds symptoms. "deployed but nothing changed in game" used to return Mod
Studio internals; the section that answers it is titled "Deployment reality check".
Documentation
The "what is proven, and by what" sections had drifted into understating what is known, which
costs the same credibility as overstating. They now record the campaign for exactly what it was —
one person, one install, one build, one sitting, and no screenshots.
One correction worth naming: the 4K texture run proves the uncompressed writer at that size. The
bundle path passes
compress=falseand the deployed.ucaswas 598 bytes over the raw mip. Anearlier summary of the campaign — mine — claimed it proved the compressed writer, and that claim had
already been written to two files before a check caught it. It is the reason the register carries
its evidence with every entry.
A correction this branch also carries
Every campaign observation was originally labelled Steam BuildID 24340829. They were made on
24539464: the game updated automatically on 2026-08-06 at 22:25 and the campaign ran the next
morning, while the build id came from a scout that had read the manifest two days earlier. The
observations are unaffected — only the label was wrong — but that label is the axis the register
uses to decide whether an entry still applies.
It makes the texture result stronger than previously written: the logo replacement reproduced on a
newer build than the original sighting, so the pipeline is known to have survived a game update
rather than to have worked twice on one build. It makes the DataAsset result weaker: that triplet
was packed two days earlier against the preceding build's containers and still applied after the
update, which is worth knowing and is not the same as having been produced against the build it ran
on. Both are now stated that way.
It surfaced because
gore asset packrefused a stale patch withASSET_GENERATION_MISMATCH. Thegeneration seal caught a change that I had not.
Verification
cargo test --workspace— 3319 passed, 0 failedpython scripts/check_docs_links.py— 42 files, no broken linkspython scripts/check_plugin.py— 1 plugin across 3 marketplacespython scripts/check_release_workflow.py— release jobs gated by the exact CI workflowgore find,gore doctorandgore audio bankssmoke-tested against a real installcargo fmt --checkis not clean, but it is not clean onmaineither — 70 files, mostly untouchedhere and including
vendor/retoc. Reformatting them would swamp this diff, so it is left alone.Closed after the first review pass
Two of the corners this branch originally listed as open were cheap to settle, so they were, in one
launch:
The
pak_filesroute works. All eight cursor PNGs shipped as a 2,312-byte override pak into~mods\and the pointer was magenta, where replacing the same eight files loose had changednothing. An override pak beats the base pak. The reach is wider than the example:
G1R\Confighas no loose copy anywhere, so every
.inithe game reads —DefaultEngine.iniincluded —exists only inside a pak, and this was the only route to any of them.
The title event draws on
title_MASTER, nottitle. The first pass replaced both at once andheard a siren; this one replaced
titlealone and the menu played its normal theme. Recorded aselimination across two witnessed observations rather than a direct test, since
title_MASTERwasnever replaced on its own.
DataAssets reach past the UI, and past
float32.DA_HumanFootsteps— a DataAsset proper, class/Script/G1R.FootstepTag— had itsFeetTextureSizeraised from(16,16,0,1)to(160,160,0,1), avector4_f64x4reached througha struct inside a struct, and the hero's footprints came out roughly ten times too large. The
settings-row triplet stayed deployed beside it as the control. Extracted, patched, packed,
deployed and observed all on the same build.
Three limits surfaced on the way, none of them previously documented: Blueprint-generated exports
(
*_C) cannot be bound to the USMAP at all; one unproven struct type costs its whole export(
MPC_StencilColordies onGuid); and a second edit to the same package is unreachable, becausethe extract receipt seals the pristine pair and the patch receipt is not accepted in its place —
so one fixed-width change per package is the ceiling. The guide had described a second edit as
routine.
Still open, by decision
AngelScript was not exercised at all by the campaign. It stays experimental.
voice addwas never tested; onlyreplacewas. Its experimental status is unchanged.The register has no write command, no export and no network. The friction of adding an entry by
hand is deliberate while it is small.
gore find heiltrankfinds nothing, because the item is called "Essenz heilender Kraft". Thecommand says honestly that it searched 3,853 display names and found none; bridging the gap
between a player's word and the game's name would mean searching dialog text, with the noise that
implies. Not attempted.
🤖 Generated with Claude Code
Note
Medium Risk
Touches localization catalog freshness, deploy API shape, and FMOD parsing used by
gore audio banks—behavior changes are intentional but could surface new errors or warnings on edge-case banks and deploys.Overview
Adds an effect register in
gore-catalog: bundledregister/*.jsonper domain (asset, audio, item, loc, texture, voice) plus aregistermodule that loads them with provenance, witness rules (confirmed/refuted without humanwitness→ unconfirmed), and derived status forgore find.README gains a capability/status table and documents
gore doctorin quick start.Deploy / FFI: successful deploy responses now always include
loc_skippedandloc_shadowedbeside the record (not inside serializedDeployRecord), so MCP/FFI clients see localization warnings the CLI already printed.gore-loc: catalogs recordsource_sha256of the.lcache; extract re-hashes before/after write to catch same-length or mid-run cache swaps;languages_foron decoded cache keys.gore-fmod:bank_summary/bank_summary_atdescribe banks from wrapper + FSB5 headers without full decrypt; shared LIST/SNDH walking with bounds and probe limits; sample-free banks asBankSummary::SampleFree;extract_wav/extract_oggrefuse truncated audio instead of returning shortened output; expanded tests and test fixtures (sample_free_bank, nested SNDH).Reviewed by Cursor Bugbot for commit 9a5ccaa. Bugbot is set up for automated code reviews on this repo. Configure here.