Skip to content

fix: NPE importing GriefPrevention data with Bedrock (Geyser/Floodgate) players - #503

Open
noaskers wants to merge 1 commit into
WiIIiam278:masterfrom
noaskers:fix/gp-importer-null-username
Open

fix: NPE importing GriefPrevention data with Bedrock (Geyser/Floodgate) players#503
noaskers wants to merge 1 commit into
WiIIiam278:masterfrom
noaskers:fix/gp-importer-null-username

Conversation

@noaskers

@noaskers noaskers commented Jul 22, 2026

Copy link
Copy Markdown

Problem

Importing from GriefPrevention on a server with Bedrock players fails partway through, aborting the whole claim import:

[Importer] ❌ Failed to import Claims: java.lang.NullPointerException: name is marked non-null but is null
	at net.william278.huskclaims.user.User.<init>(User.java:32)
	at net.william278.huskclaims.user.User.of(User.java:46)
	at ...BukkitGriefPreventionImporter$GriefPreventionUser.toUser(BukkitGriefPreventionImporter.java:437)
	at ...BukkitGriefPreventionImporter$GriefPreventionUser.toSavedUser(BukkitGriefPreventionImporter.java:442)
	at ...BukkitGriefPreventionImporter.lambda$importClaims$4(BukkitGriefPreventionImporter.java:179)

Cause

GriefPreventionUser resolves usernames with:

if (player.hasPlayedBefore()) {
    this.name = player.getName();
} else {
    this.name = uuid.toString().substring(0, 8);
}

This assumes getName() is non-null whenever hasPlayedBefore() is true, but those come from different sources — playerdata on disk versus a resolvable account name.

Geyser/Floodgate players break that assumption. Their UUIDs are synthesised by Floodgate rather than issued by Mojang, so Bukkit has no name to return for them, while they do have playerdata from having joined. The result is hasPlayedBefore() == true alongside getName() == null, so the existing UUID fallback is skipped and the null reaches User.of, tripping its Lombok @NonNull check.

Because this happens inside the importClaims future, one Bedrock player is enough to fail the import for the entire server. The failure surfaces during CLAIMS rather than USERS because importUsers() only constructs the objects — importClaims() is the first caller of toUser()/toSavedUser().

Fix

Treat a missing name the same as a never-seen player, so the existing UUID fallback covers both cases.

Tested on a local build against a GriefPrevention database containing Bedrock players; the import runs to completion instead of aborting.

Note

This gets Bedrock players through the importer rather than naming them well: they fall back to the shortened UUID, and since Floodgate UUIDs begin with a long run of zeroes, they will share the same 00000000 placeholder until each player next logs in.

Resolving those through the Floodgate API would be the more complete fix, but it adds a dependency and needs testing against a real Floodgate setup, so I've kept this change to stopping the import from breaking. Happy to look at the fuller version if you'd prefer it.

OfflinePlayer#hasPlayedBefore only indicates that playerdata exists for
a UUID; it does not guarantee the server has a cached username for it.
Names come from usercache.json, which is size-capped and expires
entries, so getName may return null for players who have played before.

That null reached User.of, tripping its non-null check and failing the
whole claim import with a NullPointerException.

Fall back to the shortened UUID whenever no name is available, not just
when the player has never played.
@noaskers

Copy link
Copy Markdown
Author

Closing this — on further investigation the null names on my server are Geyser/Floodgate Bedrock players rather than usercache expiry, so the explanation in this PR is not accurate. I'd rather not have a fix upstream with the wrong rationale attached. May reopen with a corrected diagnosis.

@noaskers noaskers closed this Jul 22, 2026
@WiIIiam278

Copy link
Copy Markdown
Owner

Appreciate your detailed diagnostics nonetheless.

@noaskers

Copy link
Copy Markdown
Author

Claude was doing something idk what XD, but yes while I still think the convert can benift from this for servers that have geyser/floodgate and would want to move over. Not sure.

image

@noaskers noaskers reopened this Jul 22, 2026
@noaskers

noaskers commented Jul 22, 2026

Copy link
Copy Markdown
Author

Its mostly up to you what you do with it, but as shown above I got that long error due to Bedrock players. I check the code out and in the current way it does not allow bedrock players to be converted due to there UUID not existing at all.

The screenshot I provied in the comment above is with my local build with this PR in it making it so it does not fully break the converter.

@noaskers

Copy link
Copy Markdown
Author

Appreciate your detailed diagnostics nonetheless.

And I did not mean to close it I just wanted to edit it, but claude was cracking up some GITHUB shit. idk what he's on I ask him to push but he just don't understand and does everything for me.

@noaskers noaskers changed the title fix: NPE importing GriefPrevention users with uncached names fix: NPE importing GriefPrevention data with Bedrock (Geyser/Floodgate) players Jul 22, 2026
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