Skip to content

[26.3] Fix placeholder syntax that discarded message arguments - #10916

Open
berkay8987 wants to merge 1 commit into
MinecraftForge:26.3from
berkay8987:fix/format-placeholders
Open

berkay8987 wants to merge 1 commit into
MinecraftForge:26.3from
berkay8987:fix/format-placeholders

Conversation

@berkay8987

Copy link
Copy Markdown
Contributor

Two messages mix up printf and SLF4J/Log4j placeholder syntax. In both cases the format string is emitted verbatim and every argument is silently dropped, so a failure reports nothing useful about itself. No issue filed for either; found while auditing format strings.

JsonCodecProvider — datagen failures name neither the file nor the reason

String.format only understands %s, so this throws IllegalStateException: Failed to encode {}: {} with path and msg discarded. A modder whose datagen fails learns neither which file was being written nor why.

On 1.20.1 the line reads:

.getOrThrow(false, msg -> LOGGER.error("Failed to encode {}: {}", path, msg));

The {} placeholders were correct for the logger, and were carried over unchanged when DataFixerUpper's getOrThrow signature changed and the call became a thrown exception. Present on 26.3, 26.2, 26.1.2, 1.21.11, 1.21.10, 1.21.8, 1.21.5, 1.21.1 and 1.20.x; 1.20.1 predates the port and is unaffected. Happy to open the backports if wanted.

DisplayWindow — the inverse mistake

LOGGER here is an org.slf4j.Logger, which only understands {}, so %d/%s log literally and the GL version and SDL error are lost — exactly the detail the trace call exists to provide. Line 369 of the same file already uses {} correctly. This one is 26.3 only, since it is part of the SDL window creation added on this branch.

Notes

src/test_old/.../DataGeneratorTest.java has two more printf-in-logger calls, left alone deliberately: that tree is not referenced by any build script.

I have not run a full game build — both changes are placeholder characters inside string literals, with no change to control flow or arity. Verified by sweeping the repo afterwards: no {} remains inside any String.format, and no printf specifier remains in any logger call outside test_old.

🤖 Generated with Claude Code

Two messages mixed up printf and SLF4J/Log4j placeholder syntax, so in
both cases the format string was emitted verbatim and every argument was
silently dropped.

JsonCodecProvider passed `{}` to String.format, which only understands
`%s`. A datagen encoding failure therefore threw
`IllegalStateException: Failed to encode {}: {}`, naming neither the file
being written nor the reason it failed. On 1.20.1 this was
`LOGGER.error("Failed to encode {}: {}", path, msg)`; the placeholders
were carried over unchanged when the call became a thrown exception.

DisplayWindow made the inverse mistake, passing `%d`/`%s` to an
org.slf4j.Logger, which only understands `{}`. The failure to create a
window logged no GL version and no SDL error. The same file already uses
`{}` correctly elsewhere.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@autoforge autoforge Bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. LTS This issue/PR is related to the current LTS version. labels Nov 3, 2026
@autoforge
autoforge Bot requested a review from a team November 3, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LTS This issue/PR is related to the current LTS version. Triage This request requires the active attention of the Triage Team. Requires labelling or reviews.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant