Repository navigation
Conversation
The World page's Game rules tab edits a value in place, or turns a true or false rule over with Space, and sets it at once. While the connected game has the world open, the mod runs /gamerule, which checks the value as the command does, and publishes the rules whenever they change (GAME_RULES, SET_GAME_RULE, protocol 30); a world no game has open gets its level.dat written, keeping level.dat_old. A value of the wrong kind is refused before anything is sent or written. Each change is a GameRule entry in Changes, with Revert. 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. |
There was a problem hiding this comment.
💡 Codex Review
This table does not set Swing's terminateEditOnFocusLost client property. After typing a rule value, clicking the filter or another tab therefore leaves the cell editor active without invoking the setter; a later setRules() refresh cancels that editor and silently discards the typed value. Configure focus-loss commit behavior as the existing configuration table does so leaving the field performs the documented commit.
AGENTS.md reference: AGENTS.md:L13-L13
ℹ️ 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".
| String previous = current(target); | ||
| if (previous == null) throw new IOException("The world " + target.world().getFileName() + " has no game rule " + target.name()); | ||
| String problem = problem(previous, value); | ||
| if (problem != null) throw new IOException(problem); | ||
| if (reverting != null && !previous.equals(reverting.current()) && !previous.equals(value)) { | ||
| throw new IOException(target.name() + " was changed outside Companion since, and reverting would replace that"); |
There was a problem hiding this comment.
Query the live value before recording or reverting
When /gamerule changes a rule after the latest GameRulesPayload and before this operation, current() returns the cached value published only once every 20 ticks. A revert can therefore see the stale recorded value, pass the outside-change guard, and overwrite the newer external value; a normal set can likewise record the wrong original value permanently. Have the game-side operation return or atomically validate the command-time value instead of basing the change record on this snapshot.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: SET_GAME_RULE now carries the value read (expected); the game checks it on the server thread where the command runs and refuses a rule changed since, so the record's original is what the game had.
| this.table.getInputMap(JComponent.WHEN_FOCUSED).put(KeyStroke.getKeyStroke("SPACE"), "toggleRule"); | ||
| this.table.getActionMap().put("toggleRule", new AbstractAction() { |
There was a problem hiding this comment.
Route Enter and F2 to the value cell
When a row or its Rule column has focus, Swing's inherited Enter action moves to another cell, while inherited F2 attempts to edit the lead cell and therefore fails if that is the non-editable Rule column. Consequently the advertised Enter/F2 editing shortcuts do not reliably open the value editor; bind both keys to start editing column 1 for the selected row, as the existing configuration table does.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: Enter and F2 are bound to edit column 1 of the selected row.
| String problem = GameRuleEdits.problem(rule.value(), value); | ||
| if (problem != null) { | ||
| this.body.showNotice("Not set: " + rule.name() + ": " + problem); | ||
| return; |
There was a problem hiding this comment.
Keep locally refused values in the editor
For an invalid local edit such as randomTickSpeed=fast, JTable has already stopped and removed its editor before setValueAt reaches this branch, so returning merely restores the old rendered value and forces the user to reopen and retype it. The UI guide requires a refused in-place value to remain editable with its reason next to it; validation needs to occur from a cell editor that can reject stopCellEditing() rather than after the model commit.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: A ValueEditor validates in stopCellEditing and keeps a refused value open, outlined, with the reason under the bar; covered in GameRulesPanelTest.
| int[] selected = this.table.getSelectedRows(); | ||
| applyFilter(); | ||
| for (int row : selected) { | ||
| if (row < this.model.shown.size()) this.table.addRowSelectionInterval(row, row); |
There was a problem hiding this comment.
Preserve selection by rule identity after filtering
When the active filter matches a value, changing the selected rule so it no longer matches removes that row during applyFilter(), but these saved numeric indices then select whichever unrelated rule shifted into the same position. For example, filtering by true and toggling the first result to false selects the next rule, so pressing Space again changes that different rule. Preserve selection by rule name and leave it empty when the edited rule is filtered out.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: replace() keeps the selection by rule name and leaves it empty when the filter hides the rule.
| * Protocol-30 payload asking the game to set the game rule {@code name} of the singleplayer world it has open to | ||
| * {@code value}, as {@code /gamerule} does. The game answers with a {@link ReloadResultPayload} of the same request id. | ||
| */ | ||
| public record SetGameRulePayload(int requestId, String name, String value) { |
There was a problem hiding this comment.
Bind each request to its target world
The request carries only an id, rule name, and value, while Companion checks that world A is open before asynchronously sending it and the mod later applies it to whichever integrated server is current. If the player closes A and opens B while the request is queued or in transit, the command changes B but the resulting change record is associated with A. Include a stable world or server-session identity in the request and reject it when the current world no longer matches.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: SET_GAME_RULE names the world's folder; the game refuses the request when another world is open.
| if (NUMBER.matcher(previous).matches()) { | ||
| if (!NUMBER.matcher(value).matches()) return "Enter a whole number"; | ||
| try { | ||
| Integer.parseInt(value); | ||
| } catch (NumberFormatException tooLarge) { |
There was a problem hiding this comment.
Canonicalize integer values before recording them
Integer spellings such as 03 and -0 pass this validation, but /gamerule stores and publishes their canonical values (3 and 0) while Companion records the typed strings. After setting 03, holds() therefore reports an outside change as soon as the next payload arrives, and a later revert can be refused because the live 3 differs from both the recorded 03 and the original value. Normalize accepted integers before sending, writing, and recording them, or record the value returned by the game.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: Accepted whole numbers are canonicalized (GameRuleEdits.canonical) before sending, writing and recording.
| CompletableFuture<ReloadResultPayload> result = new CompletableFuture<>(); | ||
| this.waiting.put(id, result); | ||
| if (send == null || !send.test(new SetGameRuleMessage(new SetGameRulePayload(id, name, value)))) { |
There was a problem hiding this comment.
Report disconnects as game-rule failures, not reload failures
A pending game-rule request is inserted into the reload request map here, so if the game disconnects before answering, gameDisconnected() completes it with “The game disconnected before it finished reloading.” That text is surfaced beneath the game-rules table even though setting a rule performs no reload, giving the user an inaccurate failure reason. Track the request kind or use an operation-neutral disconnect message.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 788aa2d: A disconnect now completes pending requests with 'The game disconnected before it answered'.
SET_GAME_RULE names the world's folder and the value the rule must still have; the game refuses the request for another world or a rule changed since, so the change record always holds what the game had. Whole numbers are written as the game writes them. In the table, Enter and F2 edit the value of the selected rule, a refused value stays in the editor with its reason, and the selection follows the rule's name through the filter. A disconnect names no reload. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Unlike ConfigSettingsTable, this table never sets JTable's terminateEditOnFocusLost client property. Consequently, after typing a numeric value, clicking the filter, another tab, or another control leaves the cell editor active instead of invoking stopCellEditing(), so the value is not sent and a later refresh can cancel and discard it; enable focus-loss termination so leaving the field commits as advertised.
AGENTS.md reference: AGENTS.md:L13-L13
ℹ️ 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".
| return before.thenCompose(values -> this.edits.setGameRule(target.world().getFileName().toString(), target.name(), | ||
| values[0], values[1]).thenApply(result -> { | ||
| if (!result.error().isEmpty()) throw new CompletionException(new IOException(result.error())); | ||
| this.record.changed(target, values[0], values[1]); |
There was a problem hiding this comment.
Refresh the live cache before completing a rule set
After a live set succeeds, this records it but leaves ResourceEdits.gameRules at the pre-command snapshot; the mod only republishes from GameRuleControl.tick() on its next 20-tick check. During that interval, an immediate second edit or Revert reads the old value and sends it as expected, so the game rejects the valid operation as “changed since,” and Changes can temporarily mark the new change as external. The expected-value guard prevents an overwrite, but this success-path cache gap remains; publish or cache the resulting value before completing the set.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 309644e: After a live set succeeds, ResourceEdits.gameRuleSet takes the written value into the cached rules at once, so the next set or revert sends it as expected.
| public boolean holds(ChangeRecord.Change change) { | ||
| if (!(change.target() instanceof ChangeRecord.GameRule target)) return true; | ||
| try { | ||
| return change.current().equals(current(target)); |
There was a problem hiding this comment.
Drop game-rule records when the original value returns
When an external /gamerule command restores a changed rule to its recorded original value, holds() only returns false instead of notifying ChangeRecord that the change ended. The persisted entry therefore survives reloads and restarts and remains listed as “Changed outside Companion” even though no Companion change is in effect; observe and remove the record when current(target) equals change.original(), as resource changes already do.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 309644e: holds() now calls ChangeRecord.observed, which ends the change when the rule is back at its original; test aRuleSetBackToItsOriginalOutsideCompanionEndsTheChange.
| try { | ||
| live = live(target.world()); |
There was a problem hiding this comment.
Move the open-world check off the event thread
Setting a rule from GameRulesPanel or reverting one from Changes invokes apply() on Swing's event thread, and this synchronous call to live() reaches Worlds.isOpen(), which opens and reads session.lock and may attempt a filesystem lock. A slow or unavailable save filesystem therefore freezes the entire Companion window before a future is even returned; perform the open-world detection on the project's write executor together with the other file reads.
AGENTS.md reference: companion/AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 309644e: live() now runs in the write queue together with the other reads, in GameRuleEdits and likewise in PackSelections.
| Rule shown = this.model.all.stream().filter(rule -> rule.name().equals(name)).findFirst().orElse(null); | ||
| all.add(this.setting.contains(name) && shown != null ? shown : Rule.of(name, value)); | ||
| }); |
There was a problem hiding this comment.
Preserve live updates that arrive during a rejected edit
When an external /gamerule changes this rule while a Companion request is pending, the resulting GAME_RULES payload can arrive before the rejection response, but this branch deliberately keeps the optimistic value instead of the newly published live value. The failure callback then restores the pre-edit rule.value(), so the table can remain permanently stale because the mod already published the external value and will not send it again until another change; retain the latest payload and apply it when the pending request fails.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 309644e: A GAME_RULES value that arrives while a set is pending is kept and shown if the set is refused; covered in GameRulesPanelTest.
…s rules Setting a rule or a pack selection reads whether its world is open with the rest, in the write queue, so a slow save folder never holds the Swing thread. A rule the game set is taken as its value at once, so the next set or revert expects it; a rule set back to its original outside Companion ends the change; and a value the game named while a set was pending is shown when the set is refused. 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: 309644e544
ℹ️ 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".
| * Protocol-30 payload with the game rules of the singleplayer world the game has open, by name, as {@code /gamerule} | ||
| * prints their values; empty while no world is open. | ||
| */ | ||
| public record GameRulesPayload(Map<String, String> rules) { |
There was a problem hiding this comment.
Bind game-rule snapshots to their world
Although SET_GAME_RULE now names its target world, this snapshot still carries no world identity. During a quick B-to-A world switch before the next 20-tick publication, or while another game process has A open, ResourceEdits can retain B's nonempty map while GameRuleEdits.current(A) accepts it merely because A's session lock is held; holds() then compares B's value and can call ChangeRecord.observed to permanently discard A's revert record when B happens to match A's original. Include the world or server-session identity in GAME_RULES and consume the snapshot only for the matching target.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3d485c3: GAME_RULES now names the world's folder; GameRuleEdits and the World page use the live rules only for that world (GameRulesPayload.of). Covered in GameRuleEditsTest.
| this.waiting.remove(id); | ||
| return CompletableFuture.failedFuture(new IOException("The game is not connected")); | ||
| } | ||
| return result.orTimeout(RELOAD_MINUTES, TimeUnit.MINUTES).whenComplete((ignored, failure) -> this.waiting.remove(id)); |
There was a problem hiding this comment.
Preserve late successful game-rule answers
When the integrated server or connection answers after RELOAD_MINUTES—for example, because the server thread remains suspended in the debugger for over ten minutes—orTimeout completes result itself exceptionally and this callback removes its request id. GameRuleControl can still execute the queued command afterward, but the late answer is discarded, so GameRuleEdits never calls record.changed and the successfully changed rule has no revert record while the UI reports failure. Keep the raw answer alive for late recording, as KeyBindingControl does, and time out only a caller-facing copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3d485c3: setGameRule returns the raw answer, which stays registered until the game answers or disconnects; GameRuleEdits records on it and times out only the caller's copy.
GAME_RULES names the folder of the world whose rules it lists, and Companion uses them only for that world. The game's answer to a rule it set is recorded however late it comes; only the caller's wait times out. 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: 3d485c3061
ℹ️ 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".
| if (Worlds.isOpen(target.world()) && rules != null && rules.of(target.world().getFileName().toString())) { | ||
| return rules.rules().get(target.name()); | ||
| } | ||
| NbtData.CompoundTag gameRules = gameRules(LevelDat.read(LevelDat.file(target.world())).tag()); |
There was a problem hiding this comment.
Avoid reading level.dat while its world is open
When this world is open but no matching live snapshot is available—for example, after the transport disconnects while the integrated server remains open—this falls through to level.dat, whose value may lag the server. Opening or refreshing Changes then calls holds(), which passes that stale value to ChangeRecord.observed; if the last save still contains the original value, the active revert record is permanently deleted even though the server still has Companion's change. Fresh evidence after the earlier world-binding fix is this unconditional disk fallback when the snapshot is absent or mismatched; treat the current value as unavailable until a matching live snapshot arrives.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b45f318: While the world is open without a matching live snapshot, current() now reports the value as unknown instead of reading level.dat, so holds() never ends a change from a lagging copy; test anOpenWorldWithoutTheGamesRulesKeepsItsChangesAndRefusesWrites.
| this.setting.remove(rule.name()); | ||
| String latest = this.named.remove(rule.name()); | ||
| if (failure != null) { |
There was a problem hiding this comment.
Scope pending rule state to the world being shown
If the current world changes while a set for the previous world is pending, setRules() treats the new world's same-named rule as that pending row and stores its real value in named. When the previous world's operation succeeds, this callback removes that value but applies it only on failure, leaving the new world displaying the old world's optimistic value indefinitely because its snapshot has already been consumed. Associate pending state with the target world, or apply/reset the latest snapshot when the displayed world changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b45f318: setRules now takes the world; showing another world clears pending sets and their late answers leave the rows alone.
| boolean live = live(target.world()); | ||
| String previous = current(target); | ||
| if (previous == null) throw new IOException("The world " + target.world().getFileName() + " has no game rule " + target.name()); | ||
| String problem = problem(previous, value); | ||
| if (problem != null) throw new IOException(problem); | ||
| String written = canonical(previous, value); | ||
| if (reverting != null && !previous.equals(reverting.current()) && !previous.equals(written)) { | ||
| throw new IOException(target.name() + " was changed outside Companion since, and reverting would replace that"); | ||
| } | ||
| if (!live) { | ||
| writeLevel(target, written); |
There was a problem hiding this comment.
Hold the world lock through offline writes
If the player opens this world while an offline edit is queued, live() can observe an unlocked session.lock, after which Minecraft can acquire it before writeLevel() runs. Companion then writes and records a successful offline change while the server owns the save; the server may already have loaded the old value and will overwrite Companion's file on its next save, leaving the change record out of sync and potentially replacing a concurrent save. Acquire and retain the world lock across the read and write, or abort when it cannot still be acquired immediately before writing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b45f318: LevelDat.update holds the world's session.lock across the read and the write (refusing when a game holds it), and the offline rule write checks the value read is still there; pack selections use the same.
level.dat is read and written while holding the world's session.lock, as the game holds it, so a game cannot open the world in between; a game rule written offline must still hold the value read. While a world is open, its level.dat lags behind the game, so without the connected game's rules a rule's value is unknown and no change is ended from it. The game rules table forgets what was being set when another world is shown. 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: b45f318e8d
ℹ️ 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 boolean of(String world) { | ||
| return !this.rules.isEmpty() && this.world.equals(world); |
There was a problem hiding this comment.
Include a server-session identity in snapshots
Fresh evidence after the folder-binding fix is that of() still treats an earlier server session for the same folder as current. GameRuleControl polls only every 20 ticks, so if world A publishes value 3, closes, is edited offline to 10, and reopens before the next poll, GameRuleEdits.current() accepts the stale A snapshot; holds() can then observe the recorded original 3 and permanently discard the new revert record even though the reopened server is using 10. Carry a server-session generation in GAME_RULES and reject snapshots from the prior opening of the same folder.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f8b64e2 differently: holds() no longer ends a change from the game's published rules at all, only from a closed world's level.dat, so a snapshot of an earlier opening can at most show the row as changed outside Companion until the next publication; a set or revert based on it is refused by the game's expected-value check.
| IntegratedServer server = Minecraft.getInstance().getSingleplayerServer(); | ||
| GameRulesPayload current = new GameRulesPayload(server == null ? "" : world(server), rules(server)); |
There was a problem hiding this comment.
Capture game rules on the server thread
When a command or server logic changes game rules concurrently with this client tick, rules(server) reads the mutable server-owned GameRules.Value instances from the client thread, even though set() correctly schedules mutations through server.execute. The resulting payload can contain stale or mixed values and is then treated as authoritative by Companion; for example, a stale original value can make GameRuleEdits.holds() discard a valid revert record. Capture the complete payload on the integrated server thread before publishing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f8b64e2: GameRuleControl reads the rules inside server.execute and publishes from there.
| rules.add(new ValueChange(change, target.name() + " in " + target.world().getFileName(), change.current(), | ||
| change.original(), this.gameRuleEdits.holds(change))); |
There was a problem hiding this comment.
Reload Changes when live rules change
When an external /gamerule command changes a recorded rule while the Changes page remains open, GAME_RULES notifies ResourceEdits' stack listeners, but this page's loader follows only the ChangeRecord. Because the record itself changes only when holds() runs during a load, the row never updates to “Changed outside Companion,” and a rule restored to its original remains listed indefinitely until the page is reopened or another record event occurs. Follow the live rule listener so this holds() result is recomputed for each snapshot.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f8b64e2: the Changes page's loader also follows ResourceEdits' stack listeners, which GAME_RULES and PACK_STACK notify.
…rver A rule set back to its original ends the change only as a closed world's level.dat says; the game's published rules may be a moment old, and a stale set or revert is refused by the game's expected value. The mod reads the rules on the server thread, and the Changes page reads again when the game's rules change. 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: f8b64e227c
ℹ️ 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".
| Map<String, String> liveRules = saved.open() && rules != null && rules.of(saved.directory().getFileName().toString()) | ||
| ? rules.rules() : saved.gameRules(); |
There was a problem hiding this comment.
Do not show saved rules for an open world
When the integrated server remains open but its matching GAME_RULES snapshot is unavailable—for example, after the transport disconnects or before the first snapshot arrives—this fallback displays values from level.dat, which may lag the server, and presents them as editable current state. Edits then fail the server-side expected-value check, while users continue seeing incorrect values. Fresh evidence after the earlier current() fix is this separate WorldPanel fallback; show the rules as unavailable until a matching live snapshot arrives instead.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 85238f9: for an open world without a matching GAME_RULES snapshot, the Game rules tab shows why in place of the rules (GameRulesPanel.showUnavailable) instead of level.dat's values; covered in GameRulesPanelTest.
While a game has the world open and its rules have not arrived, the Game rules tab says so instead of listing level.dat's values, which lag behind the game. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… read 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: 21fb79de79
ℹ️ 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".
| String current = current(target); | ||
| if (current == null) return true; | ||
| if (!Worlds.isOpen(target.world())) this.record.observed(target, current, String::equals); |
There was a problem hiding this comment.
Track whether the observed value came from disk
When the world closes between current(target) and the second Worlds.isOpen check, current may have returned the game's up-to-one-second-old snapshot, but this branch then treats that value as if it came from the closed world's level.dat. If the stale snapshot equals the recorded original, ChangeRecord.observed permanently deletes a still-needed revert record even though the close just saved a different value. Fresh evidence beyond the earlier snapshot fix is this time-of-check/time-of-use gap; return the value together with whether it came from a live snapshot, and only call observed for a value actually read from disk.
Useful? React with 👍 / 👎.
| this.record.changed(target, read.previous(), read.written()); | ||
| return new Applied(ConfigChanges.Effect.NOW); | ||
| }); | ||
| return recorded.copy().orTimeout(ResourceEdits.RELOAD_MINUTES, TimeUnit.MINUTES); |
There was a problem hiding this comment.
Give timed-out rule sets a concrete error
When the game does not answer within ten minutes, orTimeout completes the caller-facing future with a bare TimeoutException, whose message is null; GameRulesPanel consequently displays Not set: <rule>: null. Keep the raw future for late recording, but time out the copy with an exception that explains that the game has not answered and that a later successful change will still be recorded.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
|
|
||
| /** Starts afresh for another world, whatever was being set in the last one. */ | ||
| private void showWorld(Object world) { | ||
| if (this.table.isEditing()) this.table.getCellEditor().cancelCellEditing(); |
There was a problem hiding this comment.
Preserve an active edit during same-world refreshes
When any game rule changes while the user is typing another value, the resulting same-world GAME_RULES refresh calls setRules, and this unconditional cancellation discards the active editor and its uncommitted text before the world-equality check returns. This is separate from the pending-request snapshot handling: the value has not yet been submitted, so there is no recovery path. Only cancel editing when the displayed world actually changes, or merge a same-world refresh without removing the editor.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
The World page's Game rules tab now sets rules. The rules are in RESOURCE_EDITING.md, Game rules.
/gamerule <rule> <value>in the world, with a command source that captures failures. The game checks the value exactly as the command does, and the reported failure becomes the error.GAME_RULES), so the tab shows the running values rather than a stalelevel.dat.level.datis written as the game saves it (theLevelDatwriter from Enable, disable and order resource packs and the world's datapacks #74), withlevel.dat_oldkept as the backup.true/false, or an int). A wrong one is refused before anything is sent or written, and the notice says why; a refused live value is shown as it was again.GameRuletarget (world, name), with Revert and Revert All. A rule changed outside Companion since is left alone. The pack and rule tabs share one value-change table.GAME_RULES(game to Companion) andSET_GAME_RULE(Companion to game, answered withRELOAD_RESULT).Tests:
GameRuleEditsTest: offline write and revert, refused values and rules, a refused unconnected open world, the live path with a heldsession.lock.GameRulesPanelTest: refused before sending, shown at once, put back on failure.world-rule-refusedrender scenario.🤖 Generated with Claude Code