Use reflection-based tick lookup across Paperweight FAWE adapters#3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces direct MinecraftServer.currentTick access in FAWE Paperweight adapters with a reflective tick lookup intended to be more robust across server variants/obfuscation.
Changes:
- Add
MinecraftReflectionutility that reflectively discovers a tick field/method and caches it. - Update each
PaperweightFaweWorldNativeAccessto useMinecraftReflection.getCurrentTick(MinecraftServer.class)for tick comparisons and initialization.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| worldedit-bukkit/adapters/adapter-1_20_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R2/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_20_2/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_20_4/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R3/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_20_4/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_20_5/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R4/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_20_5/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_R1/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21_4/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_4/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21_4/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21_5/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_5/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21_5/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21_6/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_6/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21_6/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21_9/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_9/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21_9/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
| worldedit-bukkit/adapters/adapter-1_21_11/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_11/PaperweightFaweWorldNativeAccess.java | Switch tick reads from direct field access to reflective lookup |
| worldedit-bukkit/adapters/adapter-1_21_11/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/MinecraftReflection.java | New reflective tick lookup + caching utility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| Object cached = CACHE.get(clazz); | ||
| if (cached instanceof Field) { |
There was a problem hiding this comment.
If no compatible tick member is found (or reflection is blocked), this method will re-scan all candidates on every call. Since getCurrentTick() is called in hot paths (e.g., per setBlockState), consider caching a negative/sentinel value per class after the first failed lookup to avoid repeated reflective lookups and logging overhead.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| for (String name : FIELD_CANDIDATES) { | ||
| try { | ||
| Field field = clazz.getDeclaredField(name); | ||
| field.setAccessible(true); | ||
| CACHE.put(clazz, field); | ||
| return readField(field, serverOrClass); |
There was a problem hiding this comment.
When getCurrentTick() is called with a Class (as in the adapters: getCurrentTick(MinecraftServer.class)), this loop can discover a non-static candidate field (e.g. tickCount) and cache it before verifying it can be read without an instance. readField() will then return 0, and because the unusable Field is cached, future calls will keep returning 0 even if a later static candidate (field or method) exists. Only consider/cache static members when the input is a Class (or only cache after a successful read), and continue searching if the member isn’t usable with the provided target.
| package com.sk89q.worldedit.bukkit.adapter.impl.fawe; | ||
|
|
||
| import java.lang.reflect.Field; |
There was a problem hiding this comment.
This utility class is added with the same fully-qualified name (com.sk89q.worldedit.bukkit.adapter.impl.fawe.MinecraftReflection) in every adapter subproject. Since the Bukkit build shades all adapter subprojects into the final jar, this creates duplicate class entries where whichever adapter is packaged last “wins”, which is brittle if the implementations ever diverge. Consider moving this class to :worldedit-bukkit (adapters already depend on it) or to a shared adapters-common module so it exists exactly once.
Motivation
MinecraftServer.currentTick.Description
MinecraftReflectionutility undercom.sk89q.worldedit.bukkit.adapter.impl.fawein each Paperweight adapter module (1_20_2, 1_20_4, 1_20_5, 1_21, 1_21_4, 1_21_5, 1_21_6, 1_21_9, 1_21_11) that searches for tick fields/methods, caches the discoveredField/Method, and returns0on failure.PaperweightFaweWorldNativeAccessimplementations to import and useMinecraftReflection.getCurrentTick(MinecraftServer.class)for initiallastTickand per-callcurrentTickchecks instead of directly accessingMinecraftServer.currentTick.intwith clamping.Testing
rg "MinecraftServer\.currentTick" worldedit-bukkit/adapters -nwhich returned no matches../gradlew :worldedit-bukkit:adapters:adapter-1_21_6:compileJava :worldedit-bukkit:adapters:adapter-1_20_2:compileJava --no-daemonbut the build run failed in this environment due to an unrelated multi-project configuration error (Task with name 'build' not found in project ':worldedit-libs:cli').rgand file diffs; source-level changes are consistent and compile-ready when the overall project configuration is healthy.Codex Task