Skip to content

fix: Paper 26.2 compatibility for SpigotEntityIdProvider #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TWME-TW wants to merge 1 commit into
base: master
Choose a base branch
from
+52 −7
Open

fix: Paper 26.2 compatibility for SpigotEntityIdProvider #70

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,14 @@ private Supplier<Integer> detectIdSupplier() {
final ServerVersion serverVersion = platform.getAPI().getPacketEvents().getServerManager().getVersion();

if (isPaper() && serverVersion.isNewerThanOrEquals(ServerVersion.V_1_16)) {
return Bukkit.getUnsafe()::nextEntityId; // Paper API
// Paper removed UnsafeValues.nextEntityId() in API 26.2+.
// Verify the method exists via reflection to avoid NoSuchMethodError.
try {
UnsafeValues.class.getMethod("nextEntityId");
return Bukkit.getUnsafe()::nextEntityId; // Paper API (pre-26.x)
} catch (final NoSuchMethodException ignored) {
// Method removed — fall through to AtomicInteger reflection path
}
}

final Class<?> entityClass = getEntityClass();
Expand All @@ -79,7 +86,7 @@ private Supplier<Integer> detectIdSupplier() {

private Supplier<Integer> resolveAtomicSupplier(final Class<?> entityClass) {
final Field entityAtomicField = getStaticFieldOfType(entityClass, AtomicInteger.class,
"entityCount", "d", "c", "counter", "nextEntityId");
"entityCount", "d", "c", "counter", "nextEntityId", "ENTITY_COUNTER");
if (entityAtomicField == null) {
return null;
}
Expand All @@ -97,9 +104,13 @@ private Supplier<Integer> resolveAtomicSupplier(final Class<?> entityClass) {
}

private Supplier<Integer> resolveLegacySupplier(final Class<?> entityClass) {
final Field entityLegacyField = getStaticFieldOfType(entityClass, Integer.TYPE, "entityCount", "b");
// Search for a non-final static int field (entity counters are never final).
final Field entityLegacyField = findMutableStaticIntField(entityClass);
if (entityLegacyField == null) {
throw new IllegalStateException("Could not find legacy entity counter field");
// Last resort: local high-offset counter. Entity ID collision is unlikely
// since the server allocates from 1 upward.
final AtomicInteger fallback = new AtomicInteger(Integer.MAX_VALUE - 100000);
return fallback::incrementAndGet;
Comment on lines +110 to +113

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fallback counter overflows into negative IDs after ~100k entities.

Seeding at Integer.MAX_VALUE - 100000 means incrementAndGet reaches Integer.MAX_VALUE after ~100k calls and then wraps to Integer.MIN_VALUE, producing negative entity IDs that can collide or misbehave on clients. Consider seeding lower (well above the server's allocation range but far from overflow), or documenting/bounding the wrap behavior.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java`
around lines 110 - 113, The fallback counter in the `SpigotEntityIdProvider`
class is seeded at `Integer.MAX_VALUE - 100000`, causing it to overflow into
negative values after approximately 100k entity creations. Replace the seed
value in the `AtomicInteger` initialization to use a lower value that maintains
sufficient distance from the server's normal allocation range (which starts at
1) while leaving adequate headroom before `Integer.MAX_VALUE` to prevent
overflow, such as a value in the range of 1,000,000 or higher depending on
expected entity count limits, or alternatively implement bounds checking or
wrapping logic to handle the overflow gracefully.

}
entityLegacyField.setAccessible(true);
return () -> {
Expand All @@ -113,6 +124,32 @@ private Supplier<Integer> resolveLegacySupplier(final Class<?> entityClass) {
};
}

/**
* Finds a mutable (non-final) static int field in the given class to use as
* a legacy entity counter. Tries known field names first, then falls back to
* any non-final static int field.
*/
private static Field findMutableStaticIntField(final Class<?> clazz) {
for (final String name : new String[]{"entityCount", "b", "c"}) {
final Field field = getField(clazz, name);
if (field != null
&& field.getType() == Integer.TYPE
&& Modifier.isStatic(field.getModifiers())
&& !Modifier.isFinal(field.getModifiers())) {
return field;
}
}
// Wildcard fallback: any non-final static int field
for (final Field field : clazz.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers())
&& field.getType() == Integer.TYPE
&& !Modifier.isFinal(field.getModifiers())) {
return field;
}
}
return null;
Comment on lines +142 to +150

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect how the resolved legacy field is mutated
rg -nP -C3 '\bsetInt\s*\(' platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java

Repository: Tofaa2/EntityLib

Length of output: 552

🏁 Script executed:

#!/bin/bash
# Get the full context of findMutableStaticIntField method
sed -n '100,160p' platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java

Repository: Tofaa2/EntityLib

Length of output: 2937

🏁 Script executed:

#!/bin/bash
# Check if there are name-based filters or constraints in the field resolution
rg -nP -B10 -A10 'findMutableStaticIntField' platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java | head -60

Repository: Tofaa2/EntityLib

Length of output: 2275

🏁 Script executed:

#!/bin/bash
# Search for any additional validation or filtering of the resolved field
rg -nP 'entityLegacyField|resolveLegacySupplier' platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java | head -30

Repository: Tofaa2/EntityLib

Length of output: 620

Wildcard fallback selects the first non-final static int field, risking mutation of an unrelated field.

The method first attempts known field names ("entityCount", "b", "c") matching Minecraft entity counters, but if those fail, the wildcard fallback at lines 142–150 iterates getDeclaredFields() and returns the first non-final static int without name-based validation. Since getDeclaredFields() field ordering is not guaranteed across JVM implementations and obfuscation tools, this selection is non-deterministic. resolveLegacySupplier then calls setInt(null, entityId + 1) on this field (line 119), so selecting the wrong field mutates an unrelated NMS static int, corrupting server state.

While a fallback AtomicInteger exists if no field is found, the wildcard fallback should be restricted (e.g., require exactly one candidate, or log a warning) rather than blindly mutating any match.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@platforms/spigot/src/main/java/me/tofaa/entitylib/spigot/SpigotEntityIdProvider.java`
around lines 142 - 150, The wildcard fallback logic in the field iteration for
non-final static int fields (lines 142-150) blindly returns the first matching
field without validation, risking mutation of unrelated NMS static integers when
resolveLegacySupplier calls setInt on this field. Instead of returning the first
match found by getDeclaredFields(), add validation to ensure you are selecting
the correct field: either collect all matching candidates and verify exactly one
exists before returning it, or add a warning log if the fallback is being used,
or implement additional checks to distinguish legitimate entity counter fields
from unrelated static ints. This prevents silent corruption of server state from
mutating the wrong field.

}

/**
* Resolves server's internal `Entity` class, handling version-specific packages.
* In Minecraft versions 1.17 and later, the `Entity` class resides in `net.minecraft.world.entity.Entity`.
Expand All @@ -125,11 +162,19 @@ private Class<?> getEntityClass() {
final ServerVersion serverVersion = platform.getAPI().getPacketEvents().getServerManager().getVersion();
final boolean isFlattened = serverVersion.isNewerThanOrEquals(ServerVersion.V_1_17);

final String version = Bukkit.getServer().getClass().getPackage().getName().split("\\.")[3];
final String packagePath = isFlattened ? "net.minecraft.world.entity" : "net.minecraft.server." + version;
if (isFlattened) {
try {
return Class.forName("net.minecraft.world.entity.Entity");
} catch (final ClassNotFoundException exception) {
throw new IllegalStateException("Could not find Entity class", exception);
}
}

// Pre-1.17: versioned package (e.g. net.minecraft.server.v1_16_R3.Entity)
final String[] parts = Bukkit.getServer().getClass().getPackage().getName().split("\\.");
final String ver = parts.length > 3 ? parts[3] : "";
try {
return Class.forName(packagePath + ".Entity");
return Class.forName("net.minecraft.server." + ver + ".Entity");
} catch (final ClassNotFoundException exception) {
throw new IllegalStateException("Could not find Entity class", exception);
}
Expand Down