update to 26.1 (well snapshots at least)#15
Conversation
Co-authored-by: GalvinPython <77013913+GalvinPython@users.noreply.github.com>
Co-authored-by: GalvinPython <77013913+GalvinPython@users.noreply.github.com>
gets me started on this at least
changed Formatting to ChatFormatting and Command arguments has its own built-in entities suggestions
changed Formatting to ChatFormatting and Command arguments has its own built-in entities suggestions
There was a problem hiding this comment.
Pull request overview
This PR attempts to update the mod’s build configuration and code to target a newer Minecraft snapshot/version line (including raising the Java target), and adjusts command/dimension APIs accordingly.
Changes:
- Bump declared Minecraft/Java/Fabric API versions in
fabric.mod.jsonandgradle.properties. - Update Gradle/Loom configuration and Java toolchain target to 25.
- Refactor
/findplayercommand + utility code to newer (Mojang-named) Minecraft APIs and remove the custom suggestion provider.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/resources/fabric.mod.json |
Updates declared dependency constraints (Minecraft/Java). |
src/main/java/me/imgalvin/playerfinder/PlayerFinder.java |
Refactors command registration/execution and message formatting; adds logging. |
src/main/java/me/imgalvin/playerfinder/PlayerFinderUtils.java |
Migrates dimension/color utilities to new key types and adds debug output. |
src/main/java/me/imgalvin/playerfinder/PlayerSuggestionProvider.java |
Removes custom suggestion provider (no longer used with entity argument). |
gradle.properties |
Updates version properties for Minecraft/Fabric API and mod version. |
build.gradle |
Changes Loom plugin id, dependency configurations, and Java release/version. |
.github/workflows/build.yml |
Restricts workflow triggers via paths and bumps CI Java version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // note: this function only works for vanilla dimensions. custom dimensions will have a slight issue | ||
| return playerDimension.getValue().toString().split(":")[1].replace("the_", ""); | ||
| public String getDimensionText(@NotNull ResourceKey<Level> playerDimension) { | ||
| return playerDimension.identifier().getPath().replace("the_", ""); |
There was a problem hiding this comment.
ResourceKey does not typically expose identifier(); the standard accessor for the underlying id is usually location() (returning a ResourceLocation). As written, this is likely a compile error; use the appropriate accessor and then take the path for display.
| return playerDimension.identifier().getPath().replace("the_", ""); | |
| return playerDimension.location().getPath().replace("the_", ""); |
| System.out.println("Calculating distance between " + playerPos + " and " + targetPos); | ||
| return (int) Math.sqrt(Math.pow(playerPos.getX() - targetPos.getX(), 2) + Math.pow(playerPos.getY() - targetPos.getY(), 2) + Math.pow(playerPos.getZ() - targetPos.getZ(), 2)); |
There was a problem hiding this comment.
The System.out.println in getDistance will spam stdout on every /findplayer call. Remove this debug print or route it through the mod logger at a debug level.
| System.out.println("Calculating distance between " + playerPos + " and " + targetPos); | |
| return (int) Math.sqrt(Math.pow(playerPos.getX() - targetPos.getX(), 2) + Math.pow(playerPos.getY() - targetPos.getY(), 2) + Math.pow(playerPos.getZ() - targetPos.getZ(), 2)); | |
| return (int) Math.sqrt( | |
| Math.pow(playerPos.getX() - targetPos.getX(), 2) | |
| + Math.pow(playerPos.getY() - targetPos.getY(), 2) | |
| + Math.pow(playerPos.getZ() - targetPos.getZ(), 2) | |
| ); |
| ServerPlayer targetPlayerName = EntityArgument.getPlayer(context, "player"); | ||
| String playerName = targetPlayerName.getGameProfile().name(); | ||
|
|
||
| ServerPlayer targetPlayer = context.getSource().getServer().getPlayerList().getPlayer(playerName); | ||
| ServerPlayer sourcePlayer = context.getSource().getServer().getPlayerList().getPlayer(context.getSource().getTextName()); | ||
|
|
||
| if (targetPlayer == null) { | ||
| context.getSource().sendSystemMessage(Component.literal("[PlayerFinder ERROR] Player " + playerName + " not found").withStyle(ChatFormatting.RED)); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
EntityArgument.getPlayer(context, "player") already returns the resolved ServerPlayer, but the code then extracts a name and does a second lookup via getPlayerList().getPlayer(...). This is redundant and can produce null/incorrect results if the player state changes; use the ServerPlayer returned by the argument directly.
| ServerPlayer targetPlayerName = EntityArgument.getPlayer(context, "player"); | |
| String playerName = targetPlayerName.getGameProfile().name(); | |
| ServerPlayer targetPlayer = context.getSource().getServer().getPlayerList().getPlayer(playerName); | |
| ServerPlayer sourcePlayer = context.getSource().getServer().getPlayerList().getPlayer(context.getSource().getTextName()); | |
| if (targetPlayer == null) { | |
| context.getSource().sendSystemMessage(Component.literal("[PlayerFinder ERROR] Player " + playerName + " not found").withStyle(ChatFormatting.RED)); | |
| return 0; | |
| } | |
| ServerPlayer targetPlayer = EntityArgument.getPlayer(context, "player"); | |
| String playerName = targetPlayer.getGameProfile().getName(); | |
| ServerPlayer sourcePlayer = context.getSource().getServer().getPlayerList().getPlayer(context.getSource().getTextName()); |
| ResourceKey<Level> playerDimension = targetPlayer.level().getLevel().dimension(); | ||
| ResourceKey<Level> sourceDimension = sourcePlayer.level().getLevel().dimension(); |
There was a problem hiding this comment.
The dimension key retrieval uses targetPlayer.level().getLevel().dimension(). For a ServerPlayer, level() already returns the level instance; the extra getLevel() call is likely invalid and will not compile. Use the level API directly to get the dimension key (e.g., targetPlayer.level().dimension()).
| ResourceKey<Level> playerDimension = targetPlayer.level().getLevel().dimension(); | |
| ResourceKey<Level> sourceDimension = sourcePlayer.level().getLevel().dimension(); | |
| ResourceKey<Level> playerDimension = targetPlayer.level().dimension(); | |
| ResourceKey<Level> sourceDimension = sourcePlayer.level().dimension(); |
| LOGGER.info("Target player dimension: {}", playerDimension); | ||
| LOGGER.info("Source player dimension: {}", sourceDimension); | ||
|
|
||
| boolean isSameDimension = sourceDimension == playerDimension; |
There was a problem hiding this comment.
isSameDimension compares ResourceKey instances with ==. Use .equals(...) to compare keys reliably, since reference equality is not guaranteed.
| boolean isSameDimension = sourceDimension == playerDimension; | |
| boolean isSameDimension = sourceDimension.equals(playerDimension); |
| }); | ||
| LOGGER.info("PlayerFinder initialized!"); | ||
| // _ previously registryAccess, environment | ||
| CommandRegistrationCallback.EVENT.register((dispatcher, _, _) -> dispatcher.register(Commands.literal("findplayer") |
There was a problem hiding this comment.
The lambda parameters are named _, but a single underscore is a reserved keyword in Java (since Java 9), so this will not compile. Rename these unused parameters to something like registryAccess/environment or ignoredRegistryAccess/ignoredEnvironment.
| CommandRegistrationCallback.EVENT.register((dispatcher, _, _) -> dispatcher.register(Commands.literal("findplayer") | |
| CommandRegistrationCallback.EVENT.register((dispatcher, ignoredRegistryAccess, ignoredEnvironment) -> dispatcher.register(Commands.literal("findplayer") |
| ServerPlayer targetPlayer = context.getSource().getServer().getPlayerList().getPlayer(playerName); | ||
| ServerPlayer sourcePlayer = context.getSource().getServer().getPlayerList().getPlayer(context.getSource().getTextName()); | ||
|
|
There was a problem hiding this comment.
Looking up sourcePlayer by context.getSource().getTextName() is fragile and also breaks for non-player command sources. Prefer using the command source API to obtain the executing player directly (e.g., getPlayer() / getPlayerOrException() depending on mappings), and handle the exception or allow console execution explicitly.
| public ChatFormatting getDimensionColor(@NotNull ResourceKey<Level> playerDimension) { | ||
| return playerDimension.equals(ServerLevel.OVERWORLD) ? ChatFormatting.GREEN : | ||
| playerDimension.equals(ServerLevel.NETHER) ? ChatFormatting.RED : | ||
| playerDimension.equals(ServerLevel.END) ? ChatFormatting.LIGHT_PURPLE : | ||
| ChatFormatting.GRAY; // Fallback colour for custom or unknown dimensions |
There was a problem hiding this comment.
getDimensionColor compares a ResourceKey<Level> against ServerLevel.OVERWORLD/NETHER/END. These constants are typically defined on Level (the key type) rather than ServerLevel; align the constants/types so the comparisons are valid for the key you accept.
|
yolo |
No description provided.