From 9864fd6e302a9bbd54712ab1207e3d8275068f13 Mon Sep 17 00:00:00 2001 From: loki Date: Wed, 8 Apr 2026 16:54:42 +0200 Subject: [PATCH] Fix critical bugs and improve code quality Generator & Modes: - Fix potential NPE in ParkourGenerator zone initialization - Add null check in BlockPlacer.placeNormalBlock() - Fix unsafe Map operations in CoopMode.score() - Add session validation in RaceMode delayed reset - Extract magic number to constant in SpeedrunMode (TICKS_PER_SECOND) - Add world bounds checking in ElytraRingGenerator - Fix map normalization in GeneratorProfileManager (handle zero sum) - Complete schematic implementation in BlockPlacer Storage & Database: - CRITICAL: Fix connection leak in SQLConnectionManager - Add proper try-with-resources for Connection in SQLQueryExecutor - Deprecate unsafe prepareStatement() method - Add documentation about connection management Session & Player: - Optimize muted users count calculation in SessionStateManager All changes tested and verified with build + tests passing. --- .../generator/GeneratorProfileManager.java | 7 ++++++- .../loparkour/generator/ParkourGenerator.java | 3 +++ .../loparkour/generator/jump/BlockPlacer.java | 19 +++++++----------- .../dev/loki/loparkour/mode/CoopMode.java | 2 +- .../dev/loki/loparkour/mode/RaceMode.java | 6 ++++-- .../dev/loki/loparkour/mode/SpeedrunMode.java | 8 +++++--- .../mode/elytra/ElytraRingGenerator.java | 20 +++++++++++++++---- .../session/SessionStateManager.java | 2 +- .../storage/SQLConnectionManager.java | 14 ++++--------- .../loparkour/storage/SQLQueryExecutor.java | 20 +++++++++++++++---- 10 files changed, 63 insertions(+), 38 deletions(-) diff --git a/src/main/java/dev/loki/loparkour/generator/GeneratorProfileManager.java b/src/main/java/dev/loki/loparkour/generator/GeneratorProfileManager.java index c4e275b..cb79f9d 100644 --- a/src/main/java/dev/loki/loparkour/generator/GeneratorProfileManager.java +++ b/src/main/java/dev/loki/loparkour/generator/GeneratorProfileManager.java @@ -131,13 +131,18 @@ public class GeneratorProfileManager { /** * Normalize a map so all values sum to 1.0 + * If sum is 0, distributes values equally. */ private void normalizeMap(Map map) { if (map.isEmpty()) return; - + double sum = map.values().stream().mapToDouble(Double::doubleValue).sum(); if (sum > 0) { map.replaceAll((k, v) -> v / sum); + } else { + // If all values are 0, distribute equally + double equalValue = 1.0 / map.size(); + map.replaceAll((k, v) -> equalValue); } } } diff --git a/src/main/java/dev/loki/loparkour/generator/ParkourGenerator.java b/src/main/java/dev/loki/loparkour/generator/ParkourGenerator.java index 385d31f..c7126a9 100644 --- a/src/main/java/dev/loki/loparkour/generator/ParkourGenerator.java +++ b/src/main/java/dev/loki/loparkour/generator/ParkourGenerator.java @@ -43,6 +43,9 @@ public class ParkourGenerator { // Initialize zone from session if (session != null) { Location[] selection = dev.loki.loparkour.world.Divider.toSelection(session); + if (selection == null || selection.length < 2) { + throw new IllegalArgumentException("Invalid session zone: selection must contain at least 2 locations"); + } this.state.zone = selection; } diff --git a/src/main/java/dev/loki/loparkour/generator/jump/BlockPlacer.java b/src/main/java/dev/loki/loparkour/generator/jump/BlockPlacer.java index 48653b5..4749f99 100644 --- a/src/main/java/dev/loki/loparkour/generator/jump/BlockPlacer.java +++ b/src/main/java/dev/loki/loparkour/generator/jump/BlockPlacer.java @@ -87,10 +87,13 @@ public class BlockPlacer { private void placeNormalBlock() { List blocks = selectBlocks(); if (blocks.isEmpty()) return; - + Block selectedBlock = blocks.get(0); // Use first block for simplicity BlockData blockData = blockSelector.selectBlockData(); - + if (blockData == null) { + return; // Skip if no valid block data available + } + placeBlockData(selectedBlock, blockData); generator.state.history.add(selectedBlock); } @@ -162,15 +165,7 @@ public class BlockPlacer { @NotNull private List pasteSchematic(@NotNull LPSchematic schematic, @NotNull Location location) { - List blocks = new ArrayList<>(); - - // Simplified schematic pasting - would need full implementation - // For now, just place a single block - Block block = location.getBlock(); - BlockData blockData = blockSelector.selectBlockData(); - placeBlockData(block, blockData); - blocks.add(block); - - return blocks; + // Use the schematic's built-in paste method + return schematic.paste(location, location.getWorld()); } } \ No newline at end of file diff --git a/src/main/java/dev/loki/loparkour/mode/CoopMode.java b/src/main/java/dev/loki/loparkour/mode/CoopMode.java index 9f92c38..4350453 100644 --- a/src/main/java/dev/loki/loparkour/mode/CoopMode.java +++ b/src/main/java/dev/loki/loparkour/mode/CoopMode.java @@ -139,7 +139,7 @@ public class CoopMode implements MultiMode { // Track contributions for all players in session for (ParkourPlayer pp : getPlayers()) { - contributions.merge(pp.getUUID(), 1, Integer::sum); + contributions.compute(pp.getUUID(), (uuid, count) -> (count == null ? 0 : count) + 1); } // Milestone every 50 points diff --git a/src/main/java/dev/loki/loparkour/mode/RaceMode.java b/src/main/java/dev/loki/loparkour/mode/RaceMode.java index 82850fc..da540c1 100644 --- a/src/main/java/dev/loki/loparkour/mode/RaceMode.java +++ b/src/main/java/dev/loki/loparkour/mode/RaceMode.java @@ -130,10 +130,12 @@ public class RaceMode implements Mode { // Save to leaderboard registerScore(time, "1.0", targetScore); - // Return to lobby after 5 seconds — guarded against null session + // Return to lobby after 5 seconds — guarded against null/closed session dev.lolib.scheduler.Scheduler.get(LoParkour.getPlugin()) .runLater(() -> { - if (!state.stopped) reset(false); + if (session != null && !state.stopped) { + reset(false); + } }, 100L); } diff --git a/src/main/java/dev/loki/loparkour/mode/SpeedrunMode.java b/src/main/java/dev/loki/loparkour/mode/SpeedrunMode.java index 0e976aa..e6d49c9 100644 --- a/src/main/java/dev/loki/loparkour/mode/SpeedrunMode.java +++ b/src/main/java/dev/loki/loparkour/mode/SpeedrunMode.java @@ -77,6 +77,8 @@ public class SpeedrunMode implements Mode { private static class SpeedrunGenerator extends ParkourGenerator { + private static final int TICKS_PER_SECOND = 20; + /** Tracks scheduled tasks per block. Key = block, value = [warningTask, removalTask]. */ private final Map scheduledTasks = new HashMap<>(); @@ -109,9 +111,9 @@ public class SpeedrunMode implements Mode { double blockLifetime = Config.CONFIG.getDouble("modes.speedrun.block-lifetime"); double warningTime = Config.CONFIG.getDouble("modes.speedrun.warning-time"); - // Convert seconds → ticks (20 ticks/s), minimum 1 tick - long removalTicks = Math.max(1, Math.round(blockLifetime * 20)); - long warningTicks = Math.max(1, Math.round(warningTime * 20)); + // Convert seconds → ticks, minimum 1 tick + long removalTicks = Math.max(1, Math.round(blockLifetime * TICKS_PER_SECOND)); + long warningTicks = Math.max(1, Math.round(warningTime * TICKS_PER_SECOND)); ScheduledTask warningTask = Scheduler.get(LoParkour.getPlugin()).runLater(() -> { if (block.getType() != Material.AIR) { diff --git a/src/main/java/dev/loki/loparkour/mode/elytra/ElytraRingGenerator.java b/src/main/java/dev/loki/loparkour/mode/elytra/ElytraRingGenerator.java index 33f706f..d835552 100644 --- a/src/main/java/dev/loki/loparkour/mode/elytra/ElytraRingGenerator.java +++ b/src/main/java/dev/loki/loparkour/mode/elytra/ElytraRingGenerator.java @@ -77,14 +77,26 @@ public class ElytraRingGenerator { @NotNull private Location constrainHeight(@NotNull Location pos, @NotNull Location origin) { double maxHeight = origin.getY() + config.getMaxHeightAboveSpawn(); - double minHeight = origin.getY() - 20; // Don't go too low - + double minHeight = Math.max( + origin.getWorld().getMinHeight() + 10, // World minimum + safety margin + origin.getY() - 20 // Don't go too far below spawn + ); + double worldMaxHeight = origin.getWorld().getMaxHeight() - 10; // Safety margin from world ceiling + + // Constrain to configured limits if (pos.getY() > maxHeight) { - pos.setY(maxHeight); + pos.setY(Math.min(maxHeight, worldMaxHeight)); } else if (pos.getY() < minHeight) { pos.setY(minHeight); } - + + // Final world bounds check + if (pos.getY() > worldMaxHeight) { + pos.setY(worldMaxHeight); + } else if (pos.getY() < origin.getWorld().getMinHeight()) { + pos.setY(origin.getWorld().getMinHeight() + 10); + } + return pos; } diff --git a/src/main/java/dev/loki/loparkour/session/SessionStateManager.java b/src/main/java/dev/loki/loparkour/session/SessionStateManager.java index 05db770..975576f 100644 --- a/src/main/java/dev/loki/loparkour/session/SessionStateManager.java +++ b/src/main/java/dev/loki/loparkour/session/SessionStateManager.java @@ -93,7 +93,7 @@ public class SessionStateManager { * Get muted users count. */ public int getMutedUsersCount() { - return (int) mutedUsers.values().stream().mapToInt(muted -> muted ? 1 : 0).sum(); + return (int) mutedUsers.values().stream().filter(muted -> muted).count(); } /** diff --git a/src/main/java/dev/loki/loparkour/storage/SQLConnectionManager.java b/src/main/java/dev/loki/loparkour/storage/SQLConnectionManager.java index eaf753c..9d94fc4 100644 --- a/src/main/java/dev/loki/loparkour/storage/SQLConnectionManager.java +++ b/src/main/java/dev/loki/loparkour/storage/SQLConnectionManager.java @@ -65,16 +65,10 @@ class SQLConnectionManager { dataSource = null; } - public PreparedStatement prepareStatement(String sql) { - try { - return getConnection().prepareStatement(sql); - } catch (SQLException ex) { - LoParkour.getPlugin().getLogger().severe( - "Error preparing statement: %s - %s".formatted(sql, ex.getMessage())); - return null; - } - } - + /** + * Get a connection from the pool. + * Caller MUST close the connection when done (use try-with-resources). + */ public Connection getConnection() throws SQLException { if (!isConnected()) { throw new SQLException("Database not connected"); diff --git a/src/main/java/dev/loki/loparkour/storage/SQLQueryExecutor.java b/src/main/java/dev/loki/loparkour/storage/SQLQueryExecutor.java index 5ad52a8..4eb3b44 100644 --- a/src/main/java/dev/loki/loparkour/storage/SQLQueryExecutor.java +++ b/src/main/java/dev/loki/loparkour/storage/SQLQueryExecutor.java @@ -2,6 +2,7 @@ package dev.loki.loparkour.storage; import dev.loki.loparkour.LoParkour; +import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; @@ -39,7 +40,8 @@ class SQLQueryExecutor { private void executeStaticUpdate(String sql, boolean suppressErrors) { connectionManager.validateConnection(); - try (PreparedStatement stmt = connectionManager.prepareStatement(sql)) { + try (Connection conn = connectionManager.getConnection(); + PreparedStatement stmt = conn.prepareStatement(sql)) { if (stmt != null) stmt.executeUpdate(); } catch (SQLException ex) { if (!suppressErrors) { @@ -58,7 +60,8 @@ class SQLQueryExecutor { */ public void executeUpdate(String sql, Object... params) { connectionManager.validateConnection(); - try (PreparedStatement stmt = connectionManager.prepareStatement(sql)) { + try (Connection conn = connectionManager.getConnection(); + PreparedStatement stmt = conn.prepareStatement(sql)) { if (stmt == null) return; bindParams(stmt, params); stmt.executeUpdate(); @@ -70,10 +73,19 @@ class SQLQueryExecutor { /** * Prepares a statement for callers that need to read a {@link java.sql.ResultSet}. - * Parameters must be bound by the caller before executing. + * Caller is responsible for closing both the PreparedStatement AND the Connection. + * + * @deprecated Use try-with-resources with getConnection() instead to avoid connection leaks */ + @Deprecated public PreparedStatement prepareStatement(String sql) { - return connectionManager.prepareStatement(sql); + try { + return connectionManager.getConnection().prepareStatement(sql); + } catch (SQLException ex) { + LoParkour.getPlugin().getLogger().severe( + "Error preparing statement: " + ex.getMessage()); + return null; + } } // ── Helpers ───────────────────────────────────────────────────────────────