The current world is read by its owner, which the World page and the tree show - #115
Conversation
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. |
![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: f6cfac508a
ℹ️ 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".
| public WorldReading(GameLocation location) { | ||
| this.location = Objects.requireNonNull(location, "location"); | ||
| follow(); | ||
| this.stopFollowingGame = location.addListener(change -> this.strand.execute(this::follow)); |
There was a problem hiding this comment.
When Companion remains open while an unconnected game moves from world A to world B, GameLocation emits no listener event because that state is derived from filesystem locks in read(), so this callback never invokes follow() for B. Events from A only reread A, leaving the World page and tree on the wrong world until a later connection event; the owner needs a filesystem-owned trigger that reruns current-directory selection.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| FileReading<CurrentWorld.Saved> reading = new FileReading<>(current, entry -> ENTRIES.contains(entry.toString()), | ||
| world -> CurrentWorld.read(this.location.read(), world), Duration.ofMillis(500)); |
There was a problem hiding this comment.
With an existing datapacks directory, FileWatch registers only the supplied world directory and Java watch events are nonrecursive, so creating or removing world/datapacks/Foo produces no matching event on this watch. Because CurrentWorld.read() derives new packs by listing that child directory, the published snapshot and both consumers remain stale until an unrelated level.dat or lock event occurs; the owner must also watch the child directory and its creation.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| */ | ||
| public final class WorldReading implements AutoCloseable { | ||
| /** The entries of a world's folder that make up what is read of it. */ | ||
| private static final Set<String> ENTRIES = Set.of("level.dat", "level.dat_old", "session.lock", "datapacks"); |
There was a problem hiding this comment.
WorldPanel still loads icon.png, but the new page loader runs again only after WorldReading.changed(), while this entry set excludes the icon and the Saved snapshot contains no icon state. If icon.png changes by itself, hiding and showing the page no longer refreshes it as the old readsWhenShown flow did, so the old icon remains until another watched world change happens; make the icon part of the owner's watched domain and equality.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| Followed now = this.followed; | ||
| if (now.directory() == null) return new World(null, null, "No world has been played in this instance yet."); | ||
| try { | ||
| return new World(now.directory(), now.reading().value(), ""); |
There was a problem hiding this comment.
After a world has been read successfully, if its level.dat becomes permanently unreadable or is deleted, FileReading retains its last successful value and fires no signal for the failure, so this call keeps returning the old Saved snapshot with an empty problem. Since the page now reads only after owner signals, it can display stale, editable world data indefinitely instead of reporting the failure on its next showing; publish the failure as part of WorldReading state while preserving transient-read retries.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| if (Objects.equals(current, before.directory())) { | ||
| if (before.reading() != null) before.reading().readNow(); |
There was a problem hiding this comment.
On Windows, when a connected game leaves a singleplayer world for the menu, currentWorld() falls back to that same last-played directory, so this branch retains FileReading's native watch on the world. The repository's file-watching design notes that a watched directory cannot be renamed while a descendant watch handle is open, which means Minecraft's Delete World action or an external rename can fail for that closed world until Companion exits; release or relocate the native watch when the world is no longer open while preserving closed-world change detection through the owning system.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
…tree show docs/SYSTEMS.md, section 2. - WorldReading owns the instance's current world: the one the game has open, otherwise the one played last. It reads the world's folder as a FileReading, again when the game saves the world, opens or leaves it, or its datapacks folder changes, and moves to another world when the game plays one. - The World page follows it and the datapacks the game names, as a page of the loader: it reads when first shown and after a change, not whenever it is shown. - The Project tree's World rows show what the owner read; they no longer read level.dat themselves. - WorldReadings, through which the page told the tree what it read, goes. - FileReading can follow a whole folder, some of whose entries make up the value. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…on and datapacks are followed Which world is current is decided again when a world of saves changes, as when a new world is created or the current one deleted, and after Companion changed the datapacks. The world's own folder and its datapacks are watched only while the game holds it, so the game's Delete World can remove a world Companion shows. A new icon is a change. A follower whose name filter fails no longer stops the watcher thread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
f6cfac5 to
95d8100
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95d8100eac
ℹ️ 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".
| before.reading().watch(open); | ||
| watchDatapacks(open ? current : null, before.reading()); |
There was a problem hiding this comment.
When Companion starts while an unconnected game holds a world and that game returns to its menu, GameLocation emits no event, so follow() is not called to recompute open. The fresh evidence in this revision is that the new reading.watch(open) and watchDatapacks(...) toggles execute only inside follow(); the world reader may notice the final level.dat save, but its callback only rereads the same directory. Both native watches can therefore remain registered after session.lock is released, and on Windows the game's subsequent Delete World can fail until another top-level saves event or Companion exits; have the owner re-evaluate the held state after its watched world changes.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| */ | ||
| public WorldReading(GameLocation location, GamePacks packs) { | ||
| this.location = Objects.requireNonNull(location, "location"); | ||
| follow(); |
There was a problem hiding this comment.
If an unconnected game creates or opens a world while this owner is being constructed, the initial follow() can finish before the saves watch is registered. FileWatch does not replay events that happened before registration, and later writes to World/level.dat are below the nonrecursive saves watch, so the owner can continue reporting no world or the previous world for the whole session. Register the triggers before the initial scan, or rescan after registration, to close this lost-wakeup window.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| public void watch(boolean watch) { | ||
| this.strand.execute(() -> { | ||
| if (this.closed || this.watching == watch) return; | ||
| this.watching = watch; | ||
| if (watch) register(); | ||
| else unregister(); |
There was a problem hiding this comment.
watch(boolean) adds a new lifecycle to the shared FileReading system in which a registration is removed without closing its reading, but docs/SYSTEMS.md still states that registrations live until their readings close and describes moving the world reading instead. Because this changes a system-level file-watching invariant, leaving the recorded design unchanged will make subsequent migrations rely on the wrong lifecycle; update the systems decision with the suspension contract and rationale.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
On Windows a watch of a folder inside a world, as its datapacks, keeps the world's folder from being deleted or moved. The connected game tells it left a world only once it let go of it, so the watch always ends in time; a game Companion is not connected to has its worlds read when Companion looks again. The owner's triggers are registered before its first look. SYSTEMS.md records readings that stop watching and the Windows rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
Fourth PR of
docs/SYSTEMS.md, stacked on #114: the current world is read by one owner, which the World page and the Project tree show.Finish line
PageReadsTest.theWorldPageReadsOnceWhenOpenedAndAgainOnlyAfterTheGameSavedTheWorldWorldReadingTest.theWorldTheGamePlaysIsReadAgainWhenTheGameSavesItWorldReadingTest.theWorldTheGamePlaysIsFollowedAndTheOneBeforeTellsNothingWorldReadingTest.aNewWorldAGameCompanionIsNotConnectedToOpensIsFollowedWorldReadingTest.theWorldPlayedBeforeIsFollowedWhenTheCurrentOneIsDeleteddatapackswould hold itWorldReadingTest.aWorldTheGameDoesNotPlayCanBeMovedWhileItIsShownWorldReadingTest.aWorldTheConnectedGameLeftCanBeMovedWorldReadingTest.aWorldAGameCompanionIsNotConnectedToHoldsCanBeMovedOnceItLeftItWorldReadingTest.aNewIconOrDatapackOfTheWorldTheGamePlaysIsAChangeWorldReadingTest.withoutAWorldThePageIsToldWhyFileReadingTest.aFollowerThatFailsDoesNotStopTheWatchOfOthersWhat changes
WorldReadingowns the instance's current world: the one the game has open, otherwise the one played last. Which world that is, it decides on its strand again when the game connects, plays another world or leaves one, when a world ofsavesis created, removed or changed, and after Companion changed the datapacks. It reads the current world's folder as aFileReading(level.dat,level.dat_old,session.lock,datapacksandicon.png).datapacksare watched only while the connected game plays it. On Windows a folder cannot be deleted or renamed while a folder inside it is watched, which would break the game's Delete World. The game tells it left a world only once it let go of it. A game Companion is not connected to has its worlds read when Companion looks again, never watched. The triggers are registered before the first look, so none is missed in between.level.datthemselves.WorldReadings, through which the page told the tree what it read, goes.FileReadingcan follow a whole folder, some of whose entries make up the value, and can stop watching while it is read only when asked;docs/SYSTEMS.mdrecords both.FileWatch: a follower whose name filter fails no longer stops the one watcher thread.Left as it is
loader.updates, whichdocs/SYSTEMS.mddescribes for titles.savesis created or removed: its worlds are not watched, and Windows does not report a lock taken inside a world to the watch ofsaves. The game connects to Companion on its own.savessees a world's folder created or removed, not a file changed inside it.level.datthat stays unreadable keeps the world read before; the page shows that until the file can be read again.:companion:testpasses (1641).🤖 Generated with Claude Code