Repository navigation
Conversation
… and search owners - A setting no specification describes has Restart.UNKNOWN (ConfigSetting.undescribed, one place); while the game runs, its save says the catalog does not describe when the game uses it, instead of that the game reloaded it. - A tool's status and output beyond 256 characters keep their whole text; a copy button beside the row copies it, also after an update in place. - A refused number names the bound it crossed, also a bound at an int's limit, which the table reads as none. - A world that cannot be listed is named, the others and the defaults are still listed, and the Configuration page shows that beside what an edit says. A missing saves folder is still no world. - Search Everywhere's module filter takes a resource's owner from the file that holds it, not its namespace. - An empty value the file sets counts as changed from a non-empty default. 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. |
![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: 2ce6b4a0cd
ℹ️ 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 this.setting != null && this.literal != null && !this.setting.defaultValue().isEmpty() | ||
| && !this.value.equals(this.setting.defaultValue()); |
There was a problem hiding this comment.
When NightConfig can read a configuration that TomlText cannot edit, ConfigValues.parse deliberately preserves the parsed values while leaving literals empty. Every row then has its actual value but a null literal, so this new guard marks even a non-default value as unmodified and hides its change marker and its row under the Modified filter. Track whether the parsed values contain the setting separately from whether an editable literal was found.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d2e8b28: a literal tells that the file sets a value, even an empty one; without literals (unset, or a file TomlText cannot edit) a non-empty value still counts, as before. Tested with a row that has a value but no literal.
| String own = this.problem.isEmpty() ? this.status : this.problem; | ||
| String text = this.unlisted.isEmpty() ? own : own.isEmpty() ? this.unlisted : this.unlisted + "; " + own; |
There was a problem hiding this comment.
This introduces a new configuration-page failure state, including the combined unlisted; edit problem layout, but the commit adds no UiRenderScenario or driver setup that displays it. The UI guide requires every new page or state to be rendered in both themes, so add a scenario that makes saves unreadable and exercises this notice composition.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in d2e8b28: configuration-unlisted puts a file where the saves folder belongs and opens the test mod's new server configuration, whose defaults copy is shown under the notice. The harness catalog gains that server configuration and a world copy.
| CopyValue copy = new CopyValue("Copy Value", value); | ||
| copy.setValue(whole); |
There was a problem hiding this comment.
The copy button beside clipped inspection-tool status/output is a new UI state, but the commit does not add or update a UiRenderScenario to show it; the existing inspection scenario only presents the ordinary sample read. Add a scenario with long tool output or failure so the required light- and dark-theme render checks cover this row layout and control.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in d2e8b28: inspection-tool-output shows a tool whose failure and output are too long to show whole, each row with its copy button. Tool runs need the game, so the scenario tells the session the finished run through finishTool, as script-problems does with showRunResult.
…scenarios for an unread saves folder and long tool output Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Second PR of the follow-ups plan (
docs/FOLLOW_UPS.md, local on 1.21.1): items 4-9, from #56 and #58.What was wrong, and what changes
Restart.NONE, so its save said the game reloaded it.ConfigSetting.undescribednow makes such settings in one place withRestart.UNKNOWN, which only Companion makes. While the game runs, a save of one says "the catalog does not describe when the game uses it" (Effect.UNDESCRIBED, not pending). A revert's result stays empty on success.FactsPanel.Partkeeps the whole text of a tool's status and output, and those rows get a copy button beside the value (CopyValue, which can now sit beside a component its owner keeps up to date). The problem row keeps its error icon and color, and the button is reachable by keyboard. Which rows have a button follows their kind and label, so an update in place keeps the layout; a change there rebuilds the section.Integer.MAX_VALUEread as no bound in the refusal, so a long ranged to it refused a larger value with "Accepts any". The refusal now names the bound it crossed ("Accepts at most 2147483647").ConfigSources.ofreturnsListed: the copies found, plus the worlds or the saves folder that could not be read. A missingsavesfolder is still no world. The Configuration page shows the reason beside what it listed and beside what an edit says, in place of "No world has created this file yet", and the pack's Configuration page adds it to its problems.assets/c/...) were filtered out. The owner now comes from the file that holds the resource, throughCatalogIndex.resourceFiles, mapped when the listing is built. A shared archive keeps all its mods, Minecraft keeps its own assets, and the shown owner prefers the mod named by the namespace.Tests
ConfigChangesTest: an undescribed setting applies when the game starts, and while it runs it isUNDESCRIBEDand not pending.FactsPanelTest: the copy buttons give the whole status and output, and the new whole text after an update in place; a row without whole text has none.ConfigEditTest: a value past an int's limit names the bound.ConfigPanelTest: nosavesfolder lists the defaults without a problem; asavesthat cannot be listed names it and still lists the defaults; the page shows that warning beside a refused edit's reason; an empty value the file sets is modified.CatalogSearchTest: a resource under another namespace is found by its mod's module, not by another.:storage:testand Companion's configuration, inspection and search tests pass locally.🤖 Generated with Claude Code