Skip to content

fix: report unsupported server version at startup instead of misleading per-message errors - #39

Open
UltraGameCoder wants to merge 2 commits into
PikaMug:mainfrom
UltraGameCoder:fix/reflection-null-check
Open

fix: report unsupported server version at startup instead of misleading per-message errors#39
UltraGameCoder wants to merge 2 commits into
PikaMug:mainfrom
UltraGameCoder:fix/reflection-null-check

Conversation

@UltraGameCoder

Copy link
Copy Markdown
Contributor

Problem

If the constructor's NMS/CraftBukkit class resolution fails (e.g. an
unsupported or unrecognized server version), the failure was only ever
printed to console via e.printStackTrace() and then silently dropped.
craftMagicNumbers/itemClazz are left null, but queryMaterial
dereferences them directly with no null check.

That dereference is caught by queryMaterial's existing catch-all, so it
doesn't crash — but it gets wrapped into a generic
"[LocaleLib] Unable to query Material: X" error, repeated every single
time a player triggers a translated message, with nothing pointing at the
actual cause (the server version isn't supported).

Fix

  • Replaced the swallowed e.printStackTrace() in the constructor with
    Bukkit.getLogger().severe(...), and added one clear severe log line
    at startup when NMS class resolution fails on a modern (1.13+) server,
    so the real problem is visible immediately instead of only in a console
    stack trace.
  • Added an explicit null check in queryMaterial before attempting the
    reflection call, throwing a specific message ("NMS/CraftBukkit classes
    were not resolved for this server version at startup") instead of
    falling into the generic per-material catch-all.

Testing

Reviewed manually; behavior for supported server versions is unchanged.
No automated tests included with this PR.

@PikaMug

PikaMug commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Include the detected server version in each log, so we don't have to ask when an issue is opened.

@UltraGameCoder

Copy link
Copy Markdown
Contributor Author

Sounds good, I'll start by proposing the addition of the bukkit version.
If you have the need to include the type of jar. (e.g. CraftBukkit, Paper, ...)
I'll happily change it around.

Add the server's Bukkit version to all three places that report a
failure to resolve NMS/CraftBukkit classes, per review feedback on this
PR: the constructor's ClassNotFoundException log, the constructor's
"server not supported" summary log, and the exception thrown by
queryMaterial when those classes were never resolved. Previously none
of these included the actual version string, so diagnosing a report
required asking the reporter for it separately.
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.

2 participants