Companion's systems, first slice: signals and one page loader - #112
Conversation
docs/SYSTEMS.md describes the few systems every feature of Companion uses, and the rules that keep features on them. This is its first PR: the mechanisms with their first users. - Signal: an owner's change, carrying nothing. The catalog, the change record, the packs, the resource edits and the key assignments fire one instead of keeping their own listener lists. - PageLoader: a page names itself and the signals it follows. It reads when first shown, and after a signal once it is shown; a follow can say whether a change concerns the page; a page that writes holds its reads. - The Key bindings page, which only shows, and the resource editor, which edits, move completely onto it. They no longer read in their constructors or on navigation, and hidden resource tabs no longer read on every reload. The editor's read and write counters and follow flags become the loader's hold. - Key assignments are read again right after Companion's own write, so the page shows it without reading again. - Tables.keepingSelection keeps a table's selection and scroll when it is read again. - SystemsRulesTest lists every file that makes its own thread, listener list, watcher or shared-pool call today; a new one fails the build. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…les test counts more From an adversarial review of the slice: - KeyAssignments always reads its first value, also where the game's folder cannot be watched yet, and keeps it when a newer write overtakes that read, so no later change goes untold. The Key bindings page shows the keys it holds rather than reading options.txt itself; unwatched, it reads them whenever the page is shown. - A page whose read failed reads again when it is shown again. - The resource editor releases its held reads in finally, so nothing thrown can leave it holding them. - SystemsRulesTest also counts then...Async calls without an executor, commonPool, startVirtualThread, java.util.Timer, statically imported executor factories, Thread subclasses, and callbacks kept in any collection; the files that do so today are listed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… pack the read found The pack a mod's resource is saved into is known only once the copies are read. A change of its entry in the change record while the tab opened, or moved to another working pack, was taken as not concerning the tab and dropped, leaving the content read before on screen. After each read the entry is compared again, as before the slice. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…another pack The slice made every read of the copies hold Save back, also one after a failed save or an edit elsewhere, so a Save pressed during it did nothing; CI's slower machine hit that in aSaveOverTextAnotherTabSavedSinceAsksFirst. As before the slice, only the first read and one after a pack or working pack change hold it. The page loader tells a read's preparation which followed signals led to it (fired), rather than signals carrying what changed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ir target when made From the review's finish line for the slice, each behavior now proven with a read paused while something else happens: - A page named with page() reads nothing while hidden, also when the page itself asks, as the resource editor does to check a finished read against a later change; it reads once when shown. Changes missed while hidden are kept once per source, however often they came. - The resource editor's save checks the pack it goes into when it is made: a working pack or world that changed before the tab heard of it is refused, and the tab reads the pack it saves into now, keeping the edit unsaved. - ResourceEdits tells its edits only when a save, revert or adoption wrote something. - ResourceTabReadsTest opens a mod's resource through navigation and counts its reads, as PageReadsTest does for the Key bindings page; PackResourceEditorReadsTest pauses the editor's first read while its tab is left and the file is saved elsewhere. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A texture's animation lands before its image; when the image then fails, what landed stays and is recorded, so the resource listing must read it. Only a save refused before it wrote, as over a copy changed since, or one that found nothing to write, tells nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![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: c150e7484c
ℹ️ 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".
| Throwable cause = failure; | ||
| while (cause instanceof CompletionException && cause.getCause() != null) cause = cause.getCause(); | ||
| boolean wrote = failure == null ? saved != null : !(cause instanceof ChangePipeline.Stale); | ||
| if (wrote) this.edited.fire(); |
There was a problem hiding this comment.
When a save fails before reaching pipeline.write—for example, because the selected folder pack was deleted or the project write executor rejects the task—the unwrapped cause is an IOException, so this treats the operation as having written and fires edited. PackResourcesPanel follows that signal and re-joins every pack, an operation its own comment identifies as too slow to repeat, even though no state changed. Track whether any write actually landed rather than treating every non-Stale failure as a write.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
… edit ended The last two rounds guessed from the outcome: a failure told nothing, then every failure but a refusal told. Both were wrong somewhere, a texture's animation landing before its image failed, or a save failing before it wrote. ResourceEdits now counts each file it writes into a pack or the options, and each program's save it takes, and an edit tells when that count moved while it ran. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A folder removed while watched, or missing when the project opened, left the keys read before on screen: its watch had ended, yet the page took the keys as watched. KeyAssignments now tries the watch again after 1 and 5 seconds and every 30 seconds after, reads the keys fresh whenever asked meanwhile, and once watching reads and tells what changed. A read on request is not what the next change is told against, so every follower hears it. The page's own read-when-shown fallback goes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The helper waited before the read that showing starts had begun, a Swing step later; on CI's slower machine that read was still running when aHeldPageReadsWhatItMissedOnceReleased held the page, which then rightly read once more after release. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ange is told against A page that read the keys before the owner's first read, followed by the game saving others, left the owner taking the new keys as its first and telling nobody; the page kept the old ones. The first read now sets what later reads compare with, whoever makes it; reads on request after it still leave that to the owner. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Where the game's folder can be read but not watched, the retries only tried the watch again, so a key rebound in the game was never found. Each retry now also reads the file and tells a change, at most every 30 seconds. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a436119540
ℹ️ 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".
| private CompletableFuture<Saved> finished(Supplier<CompletableFuture<Saved>> edit) { | ||
| long before = this.landed.get(); | ||
| return edit.get().whenComplete((saved, failure) -> { | ||
| if (this.landed.get() != before) this.edited.fire(); |
There was a problem hiding this comment.
When two saves or reverts overlap, both can snapshot the same process-wide landed value; after one operation writes and waits for its reload, the other can be refused without writing yet still observe the first operation's increment and fire edited(). The successful operation later fires again, causing PackResourcesPanel to perform its costly pack join twice even though only one edit landed. The fresh evidence versus the earlier comment is the newly added global AtomicLong, which still cannot associate a landing with its originating operation; track landed writes per operation instead.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
The tests asserted a read count right after a change, which CI's slower machine sometimes had not reached yet: a read under way was still being shown, so the next one started a moment later. They now wait for the count. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
First PR of
docs/SYSTEMS.md: the few systems every Companion feature uses to learn of changes, load pages, watch files, run work, write and receive game messages, and the rules that keep features on them. It brings the first mechanisms with their first users, to show the design holds for a page that only shows and for one that edits before anything else moves.Finish line
Each behavior is proven with a test that pauses a read, or follows the user's path, rather than only the parts:
PageReadsTest(Key bindings),ResourceTabReadsTest(a mod's resource): opened through navigation in the application's window (offscreen), one read; navigating to the shown page, nonePackResourceEditorReadsTest(the editor's first read paused, the tab left, the file saved elsewhere: no read while hidden, one when shown);PageLoaderTest.aPageHiddenWhileItReadsReadsNothingMoreUntilShownWhoeverAsks,manyChangesWhileHiddenAreAskedAboutOncePerSourceResourceTextEditorTestandTextureEditorTest, includingaSaveAfterTheWorkingPackMovedBeforeTheTabHeardGoesIntoNoPack;PageLoaderTest.aReadUnderWayWhenThePageHoldsIsNotShownAndIsReadAgain;CatalogPanelsTest.keyBindingsKeepTheirSelectionWhenTheirKeysAreReadAgainPageLoaderTest.aFailureIsReportedWithItsCauseAndNothingIsShownAfterDisposingWhat changes
Signal: an owner's change, carrying nothing; the follower asks the owner. The catalog, change record, packs (per side), resource edits and key assignments fire one instead of keeping their own listener lists.ResourceEditsfires only when something was written.PageLoader: a page names itself (page) and the signals it follows (follows). It reads when first shown, and after a signal once it is shown; no read runs for a hidden page, whoever asks; missed changes are kept once per source. A follow can say whether a change concerns the page, and a read can ask which signals led to it (fired). A page that writes holds its reads (hold/release). A failed read is tried again when the page is shown again. The old modes stay for pages not moved yet.KeyAssignmentsholds, which readsoptions.txtagain right after Companion's own write, and always reads its first value, also where the game's folder cannot be watched yet. Selection and scroll are kept throughTables.keepingSelection.page,follows,firedandhold. Hidden resource tabs no longer read on every reload. A save checks the pack it goes into when it is made.SystemsRulesTestparses Companion's sources and lists every file that makes its own thread, listener list, file watcher or shared-pool async call today, with why. A new one fails the build, and a listed one that goes away must leave the list.AGENTS.mdpoints every agent todocs/SYSTEMS.md.Production code: +620 −408. Mechanisms arrive here; the later PRs mostly delete.
Known, left to PR 4
KeyAssignmentsis replaced by the shared file reading in PR 4, which updates state on one serialized path. Two theoretical cases from the last review round are left to it rather than patched here:Also known and left as it is: two resource saves that overlap exactly, one refused, can tell the Files tab twice, which then reads its pack list once more than needed. Nothing shown is wrong.
:companion:testpasses (1614 tests).🤖 Generated with Claude Code