Code health (#288)

* Fix some missing nullity/deprecation annotations * Remove unnecessary Paper checks * Respect title changes from InventoryOpenEvent
Jikoo · Feb 13, 2025 · 4de2c2f · 4de2c2f
1 parent e4fdd61
commit 4de2c2f
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 74 deletions.
diff --git a/internal/common/build.gradle.kts b/internal/common/build.gradle.kts
@@ -5,6 +5,16 @@ plugins {
   alias(libs.plugins.paperweight)
 }
 
+//tasks {
+//  withType<JavaCompile> {
+//    // OpenPlayer unchecked warning is due to superclass' messy inheritance and legacy methods.
+//    options.compilerArgs.add("-Xlint:unchecked")
+//    // PlayerManager uses "deprecated" method matching vanilla to support legacy save data.
+//    // While vanilla still feels that it is appropriate to use in the load process, we will too.
+//    options.compilerArgs.add("-Xlint:deprecation")
+//  }
+//}
+
 configurations.all {
   resolutionStrategy.capabilitiesResolution.withCapability("org.spigotmc:spigot-api") {
     val paper = candidates.firstOrNull {

diff --git a/...src/main/java/com/lishid/openinv/internal/common/container/bukkit/OpenDummyInventory.java b/...src/main/java/com/lishid/openinv/internal/common/container/bukkit/OpenDummyInventory.java
@@ -62,7 +62,7 @@ public void setItem(int index, @Nullable ItemStack item) {
   }
 
   @Override
-  public ItemStack[] getContents() {
+  public ItemStack @NotNull [] getContents() {
     return new ItemStack[getSize()];
   }
 
@@ -72,7 +72,7 @@ public void setContents(@NotNull ItemStack[] items) throws IllegalArgumentExcept
   }
 
   @Override
-  public @NotNull ItemStack[] getStorageContents() {
+  public @NotNull ItemStack @NotNull [] getStorageContents() {
     return new ItemStack[getSize()];
   }
 

diff --git a/...in/java/com/lishid/openinv/internal/common/container/bukkit/OpenDummyPlayerInventory.java b/...in/java/com/lishid/openinv/internal/common/container/bukkit/OpenDummyPlayerInventory.java
@@ -111,11 +111,13 @@ public void setItemInOffHand(@Nullable ItemStack item) {
 
   }
 
+  @Deprecated
   @Override
   public @NotNull ItemStack getItemInHand() {
     return new ItemStack(Material.AIR);
   }
 
+  @Deprecated
   @Override
   public void setItemInHand(@Nullable ItemStack stack) {
 

diff --git a/...rc/main/java/com/lishid/openinv/internal/common/container/bukkit/OpenPlayerInventory.java b/...rc/main/java/com/lishid/openinv/internal/common/container/bukkit/OpenPlayerInventory.java
@@ -170,11 +170,13 @@ public void setItemInOffHand(@Nullable ItemStack item) {
     getInventory().getOwnerHandle().getInventory().offhand.set(0, CraftItemStack.asNMSCopy(item));
   }
 
+  @Deprecated
   @Override
   public @NotNull ItemStack getItemInHand() {
     return getItemInMainHand();
   }
 
+  @Deprecated
   @Override
   public void setItemInHand(@Nullable ItemStack stack) {
     setItemInMainHand(stack);

diff --git a/internal/common/src/main/java/com/lishid/openinv/internal/common/player/PlayerManager.java b/internal/common/src/main/java/com/lishid/openinv/internal/common/player/PlayerManager.java
@@ -5,6 +5,7 @@
 import com.lishid.openinv.internal.common.container.OpenInventory;
 import com.mojang.authlib.GameProfile;
 import com.mojang.serialization.Dynamic;
+import io.papermc.paper.adventure.PaperAdventure;
 import net.minecraft.nbt.CompoundTag;
 import net.minecraft.nbt.NbtOps;
 import net.minecraft.network.chat.Component;
@@ -38,17 +39,6 @@
 
 public class PlayerManager implements com.lishid.openinv.internal.PlayerManager {
 
-  private static boolean paper;
-
-  static {
-    try {
-      Class.forName("io.papermc.paper.configuration.Configuration");
-      paper = true;
-    } catch (ClassNotFoundException ignored) {
-      paper = false;
-    }
-  }
-
   protected final @NotNull Logger logger;
   private @Nullable Field bukkitEntity;
 
@@ -158,10 +148,8 @@ boolean loadData(@NotNull ServerPlayer player) {
     // Game type settings are also loaded separately.
     player.loadGameTypes(loadedData);
 
-    if (paper) {
-      // Paper: world is not loaded by ServerPlayer#load(CompoundTag).
-      parseWorld(player, loadedData);
-    }
+    // World is not loaded by ServerPlayer#load(CompoundTag) on Paper.
+    parseWorld(player, loadedData);
 
     return true;
   }
@@ -267,8 +255,13 @@ protected void injectPlayer(ServerPlayer player) throws IllegalAccessException {
       return null;
     }
 
+    var newTitle = pair.getFirst();
+    if (newTitle != null) {
+      title = PaperAdventure.asVanilla(newTitle);
+    }
+
     player.containerMenu = menu;
-    player.connection.send(new ClientboundOpenScreenPacket(menu.containerId, menu.getType(), menu.getTitle()));
+    player.connection.send(new ClientboundOpenScreenPacket(menu.containerId, menu.getType(), title));
     player.initMenu(menu);
 
     return menu.getBukkitView();

diff --git a/internal/spigot/src/main/java/com/lishid/openinv/internal/reobf/player/PlayerManager.java b/internal/spigot/src/main/java/com/lishid/openinv/internal/reobf/player/PlayerManager.java
@@ -4,9 +4,7 @@
 import com.lishid.openinv.internal.reobf.container.OpenEnderChest;
 import com.lishid.openinv.internal.reobf.container.OpenInventory;
 import com.mojang.authlib.GameProfile;
-import com.mojang.serialization.Dynamic;
 import net.minecraft.nbt.CompoundTag;
-import net.minecraft.nbt.NbtOps;
 import net.minecraft.network.chat.Component;
 import net.minecraft.network.protocol.game.ClientboundOpenScreenPacket;
 import net.minecraft.server.MinecraftServer;
@@ -18,13 +16,10 @@
 import net.minecraft.world.entity.player.ChatVisiblity;
 import net.minecraft.world.inventory.AbstractContainerMenu;
 import net.minecraft.world.level.Level;
-import net.minecraft.world.level.dimension.DimensionType;
 import org.bukkit.Bukkit;
 import org.bukkit.OfflinePlayer;
 import org.bukkit.Server;
-import org.bukkit.World;
 import org.bukkit.craftbukkit.v1_21_R3.CraftServer;
-import org.bukkit.craftbukkit.v1_21_R3.CraftWorld;
 import org.bukkit.craftbukkit.v1_21_R3.entity.CraftPlayer;
 import org.bukkit.craftbukkit.v1_21_R3.event.CraftEventFactory;
 import org.bukkit.entity.Player;
@@ -33,22 +28,10 @@
 import org.jetbrains.annotations.Nullable;
 
 import java.lang.reflect.Field;
-import java.util.UUID;
 import java.util.logging.Logger;
 
 public class PlayerManager implements com.lishid.openinv.internal.PlayerManager {
 
-  private static boolean paper;
-
-  static {
-    try {
-      Class.forName("io.papermc.paper.configuration.Configuration");
-      paper = true;
-    } catch (ClassNotFoundException ignored) {
-      paper = false;
-    }
-  }
-
   private final @NotNull Logger logger;
   private @Nullable Field bukkitEntity;
 
@@ -158,48 +141,9 @@ boolean loadData(@NotNull ServerPlayer player) {
     // Game type settings are also loaded separately.
     player.loadGameTypes(loadedData);
 
-    if (paper) {
-      // Paper: world is not loaded by ServerPlayer#load(CompoundTag).
-      parseWorld(player, loadedData);
-    }
-
     return true;
   }
 
-  private void parseWorld(@NotNull ServerPlayer player, @NotNull CompoundTag loadedData) {
-    // See PlayerList#placeNewPlayer
-    World bukkitWorld;
-    if (loadedData.contains("WorldUUIDMost") && loadedData.contains("WorldUUIDLeast")) {
-      // Modern Bukkit world.
-      bukkitWorld = Bukkit.getServer().getWorld(new UUID(loadedData.getLong("WorldUUIDMost"), loadedData.getLong("WorldUUIDLeast")));
-    } else if (loadedData.contains("world", net.minecraft.nbt.Tag.TAG_STRING)) {
-      // Legacy Bukkit world.
-      bukkitWorld = Bukkit.getServer().getWorld(loadedData.getString("world"));
-    } else {
-      // Vanilla player data.
-      DimensionType.parseLegacy(new Dynamic<>(NbtOps.INSTANCE, loadedData.get("Dimension")))
-          .resultOrPartial(logger::warning)
-          .map(player.server::getLevel)
-          // If ServerLevel exists, set, otherwise move to spawn.
-          .ifPresentOrElse(player::setServerLevel, () -> spawnInDefaultWorld(player));
-      return;
-    }
-    if (bukkitWorld == null) {
-      spawnInDefaultWorld(player);
-      return;
-    }
-    player.setServerLevel(((CraftWorld) bukkitWorld).getHandle());
-  }
-
-  private void spawnInDefaultWorld(ServerPlayer player) {
-    ServerLevel level = player.server.getLevel(Level.OVERWORLD);
-    if (level != null) {
-      player.spawnIn(level);
-    } else {
-      logger.warning("Tried to load player with invalid world when no fallback was available!");
-    }
-  }
-
   private void injectPlayer(ServerPlayer player) throws IllegalAccessException {
     if (bukkitEntity == null) {
       return;