Enhance documentation and error handling across multiple classes; add configuration file for plugin settings

Author: Omena0

src/main/java/com/pyjavabridge/BridgeInstance.java

@@ -88,6 +88,11 @@
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.player.PlayerQuitEvent;
 
+/**
+ * Represents a single Python script bridge connection.
+ * Manages the IPC protocol (stdin/stdout), message routing,
+ * event subscriptions, and object handle registry for one script.
+ */
 public class BridgeInstance {
     private final PyJavaBridgePlugin plugin;
     private final String name;
@@ -152,7 +157,9 @@ public void onPlayerQuit(PlayerQuitEvent event) {
                 if (attachment != null) {
                     try {
                         event.getPlayer().removeAttachment(attachment);
-                    } catch (Exception ignored) {
+                    } catch (Exception e) {
+                        // Attachment may already be invalid if plugin is disabling
+                        plugin.getLogger().fine("[" + name + "] Could not remove permission attachment: " + e.getMessage());
                     }
                 }
             }
@@ -179,6 +186,7 @@ public boolean hasSubscription(String eventName) {
         return subscriptions.containsKey(eventName);
     }
 
+    /** Starts the Python process and bridge communication thread. */
     void start() {
         running = true;
 
@@ -214,7 +222,9 @@ private void shutdownInternal() {
             if (obj instanceof org.bukkit.boss.BossBar bar) {
                 try {
                     bar.removeAll();
-                } catch (Exception ignored) {}
+                } catch (Exception e) {
+                    plugin.getLogger().fine("[" + name + "] Could not remove boss bar: " + e.getMessage());
+                }
             }
         }
 
@@ -254,7 +264,7 @@ private void bridgeLoop() {
                 } catch (IOException eof) {
                     break;
                 }
-                if (length <= 0 || length > 16_777_216) {
+                if (length <= 0 || length > plugin.getMaxMessageSize()) {
                     plugin.getLogger().severe("[" + name + "] Invalid message length: " + length);
                     break;
                 }
@@ -267,6 +277,10 @@ private void bridgeLoop() {
                     plugin.getLogger().severe("[" + name + "] Failed to parse message: " + e.getMessage());
                     continue;
                 }
+                if (!message.has("type") || !message.get("type").isJsonPrimitive()) {
+                    plugin.getLogger().warning("[" + name + "] Received message without valid 'type' field, skipping");
+                    continue;
+                }
                 handleMessage(message);
             }
 
@@ -406,6 +420,21 @@ private void handleHandshake(JsonObject message) {
 
     private void handleRegisterCommand(JsonObject message) {
         String commandName = message.get("name").getAsString();
+
+        // Validate command name
+        if (commandName == null || commandName.isEmpty()) {
+            plugin.getLogger().warning("[" + name + "] Attempted to register command with empty name");
+            return;
+        }
+        if (commandName.length() > 64) {
+            plugin.getLogger().warning("[" + name + "] Command name too long (max 64): " + commandName);
+            return;
+        }
+        if (!commandName.matches("[a-zA-Z0-9_-]+")) {
+            plugin.getLogger().warning("[" + name + "] Invalid command name (alphanumeric, _ and - only): " + commandName);
+            return;
+        }
+
         String permission = message.has("permission") ? message.get("permission").getAsString() : null;
         boolean hasDynamicTabComplete = message.has("has_tab_complete") && message.get("has_tab_complete").getAsBoolean();
 
@@ -508,7 +537,8 @@ private void handleRemoveEntities(JsonObject message) {
                 if (obj instanceof Entity entity) {
                     try {
                         entity.remove();
-                    } catch (Exception ignored) {
+                    } catch (Exception e) {
+                        plugin.getLogger().fine("[" + name + "] Could not remove entity: " + e.getMessage());
                     }
                 }
                 registry.release(handle);
@@ -644,22 +674,32 @@ void sendShutdownEvent() {
 
     public Object handleKick(Player player, List<Object> args) {
         String reason = args.isEmpty() ? "" : String.valueOf(args.get(0));
+        // Try API variants in order: Component, String, legacy kickPlayer
         try {
             Method kick = player.getClass().getMethod("kick", Component.class);
             kick.invoke(player, Component.text(reason));
             return null;
-        } catch (Exception ignored) {
+        } catch (NoSuchMethodException e) {
+            // API variant not available, try next
+        } catch (Exception e) {
+            plugin.getLogger().fine("[" + name + "] kick(Component) failed: " + e.getMessage());
         }
         try {
             Method kick = player.getClass().getMethod("kick", String.class);
             kick.invoke(player, reason);
             return null;
-        } catch (Exception ignored) {
+        } catch (NoSuchMethodException e) {
+            // API variant not available, try next
+        } catch (Exception e) {
+            plugin.getLogger().fine("[" + name + "] kick(String) failed: " + e.getMessage());
         }
         try {
             Method kickPlayer = player.getClass().getMethod("kickPlayer", String.class);
             kickPlayer.invoke(player, reason);
-        } catch (Exception ignored) {
+        } catch (NoSuchMethodException e) {
+            plugin.getLogger().warning("[" + name + "] No kick method found on player class");
+        } catch (Exception e) {
+            plugin.getLogger().fine("[" + name + "] kickPlayer(String) failed: " + e.getMessage());
         }
         return null;
     }
@@ -1613,7 +1653,8 @@ private Object invokeInventoryMethod(org.bukkit.inventory.Inventory inventory, S
                         viewer.closeInventory();
                     }
                 }
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                plugin.getLogger().fine("[" + name + "] Error closing inventory: " + e.getMessage());
             }
             return null;
         }
@@ -1622,7 +1663,8 @@ private Object invokeInventoryMethod(org.bukkit.inventory.Inventory inventory, S
                 Method getTitle = inventory.getClass().getMethod("getTitle");
                 Object titleObj = getTitle.invoke(inventory);
                 return titleObj != null ? titleObj.toString() : "";
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                // getTitle() may not exist on all inventory types
                 return "";
             }
         }
@@ -2114,7 +2156,9 @@ private Object invokeItemStackMethod(ItemStack itemStack, String method, List<Ob
                         try {
                             Method setType = ItemStack.class.getMethod("setType", Material.class);
                             setType.invoke(itemStack, deserialized.getType());
-                        } catch (Exception ignored) {
+                        } catch (Exception e) {
+                            // setType may be deprecated/removed in newer API versions
+                            plugin.getLogger().fine("[" + name + "] ItemStack.setType fallback failed: " + e.getMessage());
                         }
                         itemStack.setAmount(deserialized.getAmount());
                         if (deserialized.hasItemMeta()) {
@@ -2501,7 +2545,7 @@ private Object invokeDisplayMethod(Object target, String method, List<Object> ar
         return UNHANDLED;
     }
 
-    // #2: Cache getMethods() per class to avoid repeated reflection
+    // Cache getMethods() per class to avoid repeated reflection
     private static final ConcurrentHashMap<Class<?>, Method[]> reflectiveMethodsCache = new ConcurrentHashMap<>();
 
     private Object invokeReflective(Object target, String method, List<Object> args) throws Exception {
@@ -2631,7 +2675,9 @@ private Sound resolveSound(Object arg) {
                             }
                         }
                     }
-                } catch (Exception ignored) {
+                } catch (Exception e) {
+                    // Sound registry lookup may fail depending on server version
+                    plugin.getLogger().fine("Sound registry lookup failed for '" + name + "': " + e.getMessage());
                 }
             }
 
@@ -2644,7 +2690,9 @@ private Sound resolveSound(Object arg) {
                 if (soundObj instanceof Sound sound) {
                     return sound;
                 }
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                // Sound.valueOf not available or name doesn't match
+                plugin.getLogger().fine("Sound.valueOf failed for '" + enumName + "': " + e.getMessage());
             }
         }
         return null;
@@ -2669,7 +2717,8 @@ private String getTabListValue(Player player, boolean header) {
                 Method method = player.getClass().getMethod(name);
                 value = method.invoke(player);
                 break;
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                // API variant not available; try next method name
             }
         }
 
@@ -2744,12 +2793,14 @@ private boolean tryTabListSetter(Player player, Object header, Object footer, Bo
                 method.invoke(player, header);
                 return true;
 
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                // Try next API method variant
             }
         }
         return false;
     }
 
+    /** Serializes a value using the bridge serializer. */
     public JsonElement serialize(Object value) {
         return serializer.serialize(value);
     }
@@ -2915,6 +2966,7 @@ private JsonObject deserializePayload(byte[] payload) {
         return JsonParser.parseString(new String(payload, StandardCharsets.UTF_8)).getAsJsonObject();
     }
 
+    /** Sends a JSON message to the Python process over the bridge. */
     public void send(JsonObject response) {
         if (writer == null || !running) {
             return;
@@ -2936,8 +2988,8 @@ public void send(JsonObject response) {
     }
 
     /**
-     * Write multiple responses under a single lock acquisition + single flush.
-     * Much more efficient than calling send() in a loop for batch responses.
+     * Write multiple responses under a single lock acquisition and single flush.
+     * More efficient than calling send() in a loop for batch responses.
      */
     private void sendAll(List<JsonObject> responses, long startNano) {
         if (writer == null || !running || responses.isEmpty()) {
@@ -3091,6 +3143,15 @@ private void startPythonProcess() {
     }
 
     private String resolvePythonExecutable() {
+        String override = plugin.getPythonExecutableOverride();
+        if (override != null && !"auto".equalsIgnoreCase(override)) {
+            Path overridePath = Path.of(override);
+            if (Files.exists(overridePath)) {
+                return overridePath.toAbsolutePath().toString();
+            }
+            plugin.getLogger().warning("[" + name + "] Configured python-executable not found: " + override + ", falling back to auto-detect");
+        }
+
         boolean isWindows = System.getProperty("os.name", "").toLowerCase().contains("win");
 
         Path venvDir = scriptsDir.resolve(".venv");
@@ -3113,7 +3174,8 @@ private void closeQuietly(AutoCloseable closable) {
         }
         try {
             closable.close();
-        } catch (Exception ignored) {
+        } catch (Exception e) {
+            plugin.getLogger().fine("[" + name + "] Error during close: " + e.getMessage());
         }
     }
 }

src/main/java/com/pyjavabridge/BridgeSerializer.java

@@ -44,20 +44,25 @@
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 
+/**
+ * Serializes and deserializes Java/Bukkit objects to and from JSON
+ * for transmission over the Python bridge. Handles object registration,
+ * cyclic reference detection, and type-specific field extraction.
+ */
 public class BridgeSerializer {
     private final ObjectRegistry registry;
     private final Gson gson;
     private final PyJavaBridgePlugin plugin;
 
-    // #9: Single cache map for tryInvokeNoArg — Optional.empty() means known-missing
+    // Cache for tryInvokeNoArg — Optional.empty() means known-missing
     private final ConcurrentHashMap<String, Optional<Method>> noArgMethodCache = new ConcurrentHashMap<>();
     // Cache the PlainTextComponentSerializer instance instead of calling .plainText() repeatedly
     private static final PlainTextComponentSerializer PLAIN_TEXT = PlainTextComponentSerializer.plainText();
 
-    // #8: Cache getLogicalTypeName() per concrete class to avoid repeated instanceof chains
+    // Cache getLogicalTypeName() per concrete class to avoid repeated instanceof chains
     private static final ConcurrentHashMap<Class<?>, String> typeNameCache = new ConcurrentHashMap<>();
 
-    // #3: ThreadLocal for cyclic reference detection set — reuse across serialize() calls
+    // ThreadLocal for cyclic reference detection set — reuse across serialize() calls
     private static final ThreadLocal<Set<Object>> SEEN_SET = ThreadLocal.withInitial(
             () -> Collections.newSetFromMap(new java.util.IdentityHashMap<>()));
 
@@ -100,6 +105,7 @@ public BridgeSerializer(ObjectRegistry registry, Gson gson, PyJavaBridgePlugin p
         this.plugin = plugin;
     }
 
+    /** Serializes a Java/Bukkit object to a JSON representation. */
     public JsonElement serialize(Object value) {
         Set<Object> seen = SEEN_SET.get();
         try {
@@ -316,7 +322,8 @@ private JsonElement serialize(Object value, Set<Object> seen) {
                 if (titleObj != null) {
                     fields.addProperty("title", titleObj.toString());
                 }
-            } catch (Exception ignored) {
+            } catch (Exception e) {
+                // getTitle() may not exist on all inventory implementations
             }
         }
 
@@ -446,11 +453,13 @@ private Object tryInvokeNoArg(Object target, String methodName) {
         if (cached.isEmpty()) return null;
         try {
             return cached.get().invoke(target);
-        } catch (Exception ignored) {
+        } catch (Exception e) {
+            // Invocation failed on a known method — likely transient entity state
             return null;
         }
     }
 
+    /** Deserializes a JSON element back to a Java object (primitives, lists, handles, refs, values). */
     public Object deserialize(JsonElement element) {
         if (element == null || element.isJsonNull()) {
             return null;