Skip to content

The mod's small items: fluids and energy per face, a language reload without the icon archive - #145

Open
Pelotrio wants to merge 3 commits into
1.21.1from
claude/mod-items
Open

Pelotrio wants to merge 3 commits into
1.21.1from
claude/mod-items

Conversation

@Pelotrio

@Pelotrio Pelotrio commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Fourth PR of the follow-ups plan (docs/FOLLOW_UPS.md, local on 1.21.1): items 10 and 11, from #56 and #110.

What was wrong

  • 10. Fluid and energy amounts per face (Follow-ups from the review of the inspection and modpack stack #56). StorageReader showed the handler without a side, or when there was none, the face with the most tanks or capacity, noted as "Read through". Other faces' storages were lost, equal in size or not. SideReader grouped faces by what they do alone, so a machine whose input and output tanks both take and give water showed one group.
  • 11. A language-only reload rebuilt the icon archive (The icon archive is written again after every resource reload #110). Companion's quick reload of a language file reached resourcesReloaded, which marked the resource snapshot changed and captured every model and texture again, about 8 s.

What changes

  • Faces group by their contents and by what they do (SideReader.Exposed). Fluids keep each tank's fluid and capacity, empty tanks too. Energy keeps its amounts, such as 1,200 / 10,000 FE: takes. Faces of one storage stay one group, and separate storages with different contents are separate groups.
    • When all sides are alike, the section shows the contents without a side, then the "All sides" row of what they do, as before.
    • When the sides differ, each group shows its own tanks as fluid rows with bars, within the section's fact budget as slots are, then a row of what it does. Energy shows a row with its amounts. The block shows no contents of its own.
    • The largest-face fallback (contents, Found, "Read through") is gone, for items too. With the sides alike it could only name a face exposing nothing.
    • StorageReader now writes the side rows of fluids and energy itself, as it already did for items. A failing capability lookup is reported once in its section.
  • A quick language-only reload calls TotalDebugClient.languageReloaded. That captures the catalog again for its translated names and tells Companion, and leaves the snapshot (models, textures, atlases, fluid appearances; no language) and the pack stacks alone. Textures put in place still go through resourcesReloaded. A language request that fell back to a full reload still takes the full path through the reload listener.

Tests

  • SideReaderTest: faces of two storages that do the same but hold different amounts are separate groups; amounts are formatted as grouped numbers of their capacity.
  • Review found the first version wrote a group's tanks as one clipped text, dropped empty tanks' capacity, and reported a failing lookup twice; fixed.
  • :mod:test inspection tests pass.

Not checked in a running game: the TotalDebug game connection was down this session. The reader paths need a game with a block exposing different storages per face.

🤖 Generated with Claude Code

…oad without the icon archive

- A block whose faces expose fluids or energy differently showed the amounts of the handler without a side, or of one
  largest face, and grouped faces by what they do alone, so separate storages doing the same merged. Faces now group by
  their tanks (fluid and capacity, empty ones too) or energy amounts as well, and where they differ, each group shows its
  own tanks as fluid facts within the section's budget, or its amounts; the largest-face fallback and its "Read through"
  row are gone. A side that cannot be read leaves the contents shown.
- Companion's quick reload of a language file marked the icon archive changed, which captured every model and texture
  again (about 8 s). The language alone now only has the catalog captured again for its names; textures put in place
  still mark the archive.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T23:25:50.055227Z f0beec9 New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b00d64183d

ℹ️ 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 (!access.equals(NOT_EXPOSED)) facts.section(section).text(groups.getFirst().label(), access);
return;
}
int perGroup = Math.max(1, FactSection.MAX_FACTS / groups.size() - 2);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle failed side reads before dividing by group count

When any fluid or energy capability lookup throws, StorageReader.sided catches that failure and leaves groups empty before calling this method. The resulting division by zero adds a second, misleading ArithmeticException to the inspection after the original capability error, contrary to the intended behavior of preserving unsided contents with one reported side-read problem. Return immediately when the group list is empty, or skip write after the guarded read fails.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e1fa19: write returns at once for no groups; tested, and the test fails without it.

List<Tank> tanks = new ArrayList<>();
List<String> access = new ArrayList<>();
int empty = 0;
for (int tank = 0; tank < Math.min(handler.getTanks(), StorageReader.MAX_TANKS); tank++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the actual tank count before capping side views

For a side handler exposing more than MAX_TANKS, this loop discards every tank after the first 32 before grouping and truncation reporting. In the common side-only case there are two groups, so perGroup is 62 and the later warning sees a 32-element list as complete: the omitted tanks are silently hidden, and faces differing only after tank 32 can be merged. Retain the handler's total tank count so the output reports the cap and grouping can distinguish truncated views.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one: a side exposing more than 32 tanks is the same cap StorageReader has always applied to tanks, and the inspection already shows at most 32 per handler. Faces that differ only after tank 32 are not a case worth more code.

Comment on lines +199 to 201
public void languageReloaded() {
this.catalogs.recapture();
this.companionApp.announceInventory();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Republish translated pack titles after language reloads

When the edited language file overrides a translation used by a built-in or mod resource pack's translatable title, the quick reload changes pack.getTitle().getString(), which PackStacks.of publishes to Companion. This path only recaptures the catalog, so Companion keeps the old pack title until an unrelated full reload or reconnect; republish the lightweight pack stack here while continuing to skip the expensive resource snapshot.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e1fa19: languageReloaded republishes the pack stacks too, which is cheap; only the resource snapshot is skipped.

…shes the packs' titles

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e1fa19374

ℹ️ 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".

ScriptFacts facts) {
List<SideReader.Group<SideReader.Exposed>> sides = new ArrayList<>();
facts.guarded(section, () -> sides.addAll(read.get()));
if (sides.size() <= 1) contents.run();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not retry failed unsided capability reads

When the unsided fluid or energy capability lookup throws, it fails at the first lookup in SideReader.groups; the inner facts.guarded records that failure and leaves sides empty. This condition then runs contents, which performs the same unsided lookup again, and the exception escapes to the outer guard in readBlock, so the inspection reports and logs the same provider failure twice. Track whether the side read succeeded or reuse a single guarded unsided lookup rather than retrying after this failure.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f0beec9: the side read is no longer guarded on its own, so a failing capability lookup is reported once by the section's guard, and nothing is read again. The empty-groups case cannot arise any more, so that guard and its test are gone.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant