Better Veinminer

[1.4.0] — 2026-05-14

Fixed

• [CRITICAL] Async save race condition — StatsManager.saveAll() was scheduled as an async task and read PlayerStats fields directly while the main thread was still writing to them. A player could lose stats or receive a corrupted save file depending on thread timing. The auto-save timer now runs on the main thread to take a point-in-time snapshot via the new PlayerStats.copy(), then hands each snapshot to a single-threaded ExecutorService for file I/O. The snapshot is fully consistent; the I/O thread never sees a half-written state.

• [CRITICAL] Concurrent YAML write corruption — The async auto-save timer and the onQuit async save could both be writing the same player's file simultaneously, producing a truncated or structurally invalid YAML file. All disk writes are now submitted to a single-threaded ExecutorService (BVM-StatsIO). Only one write per file can be in progress at any time. StatsManager.shutdown() called from onDisable() blocks up to 5 seconds to guarantee all pending writes complete before the plugin unloads.

• [CRITICAL] Daily reset based on server uptime, not real calendar date — The reset used 24 * 60 * 60 * 20L ticks (24 hours of server uptime). TPS lag caused the timer to drift, and a server restart before 24 hours elapsed reset the timer entirely, effectively disabling the daily limit. Replaced with a LocalDate.now() comparison: the last reset date is persisted to plugins/BetterVeinminer/data.yml and checked on startup (to catch any resets missed while the server was offline) and every 2 minutes (to detect midnight without requiring an exact timer).

• [HIGH] Cooldown cleanup removed entries too aggressively — The cleanup task that evicts stale entries from the cooldowns map used a hardcoded 60-second cutoff (System.currentTimeMillis() - 60_000L). If the configured cooldown exceeded 60 seconds, player entries were evicted while the cooldown was still active. On the next vein break the lookup returned null and the cooldown was silently bypassed. The cutoff now uses config.getCooldownMs().

• [HIGH] Protection plugin bypass — Extra blocks in the vein were broken with block.breakNaturally(tool) without first firing a BlockBreakEvent. WorldGuard, GriefPrevention, Lands, and Residence listen to BlockBreakEvent to enforce region protection; because no event was fired, blocks in protected regions could be destroyed. A BlockBreakEvent is now fired for each extra block before it is broken; if any listener cancels the event, that block is skipped.

• [HIGH] Missing anti-recursion guard — The protection-plugin fix above fires BlockBreakEvent programmatically, which caused the plugin's own onBlockBreak listener to trigger again for each block, scheduling a new VeinmineTask per block and creating an infinite chain. A miningPlayers Set<UUID> marks any player whose veinmine session is currently active; onBlockBreak returns immediately if the player's UUID is already present. The set is always cleared in a finally block to ensure a player can never be permanently locked out.

• [MEDIUM] 26-direction BFS linked separate nearby veins — The BFS scanned all 26 neighbours (3×3×3 cube), allowing diagonal connections to bridge two distinct ore veins that happened to be close together. Vanilla Minecraft generates ores using face-adjacent placement only, so diagonal connections are always false positives. Changed to 6-direction (face-adjacent) BFS. This also reduces the maximum queue size by up to 4×, improving TPS on large veins.

• [MEDIUM] Unbreaking durability formula was deterministic, not vanilla-accurate — The old calcDamage() computed Math.round(hits × multiplier × 1/(unbreaking+1)) and clamped to a minimum of 1. Vanilla Minecraft independently rolls each hit: each has a 1/(unbreaking+1) chance to actually apply damage. The clamped average could never produce 0 damage even with high Unbreaking levels and few hits. calcDamage() now performs a per-hit Bernoulli trial using Math.random(), matching vanilla behaviour including the possibility of 0 damage.

• [MEDIUM] PlayerStats exposed public mutable fields — All four fields (blocksMined, totalOresCollected, currentTier, veinminesUsed) were public, allowing any code to set them to negative or overflowed values without validation. All fields are now private. Setters clamp values to valid ranges (e.g. tier ≥ 1, counts ≥ 0). The new copy() method supports safe async snapshotting.

• [MEDIUM] Config accepted invalid numeric values — Values such as max-blocks: -1 or damage-multiplier: -100 were loaded without validation, producing undefined behaviour (negative BFS limit, inverted durability, server lag). All numeric fields are now clamped on load: max-blocks → [1, 1000], cooldown-ms → [0, 300000], damage-multiplier → [0.1, 10.0], limit-per-day → [1, 100000], base-exp → [0, 10000].

Clarified (not a bug)

• Tool swap exploit (audit report #7) — The report described a tool-swap exploit where a player could switch items during the 1-tick task delay to bypass restrictions. This was already fixed in v1.2.1: VeinmineTask records the inventory slot and tool snapshot at event time, then re-validates with isSimilar() before applying durability. No change needed.

Changed

• Auto-save timer switched from runTaskTimerAsynchronously to runTaskTimer (main thread) so snapshot collection is always synchronised with game state. The actual file I/O is still off the main thread via the BVM-StatsIO executor.
• onDisable() now calls statsManager.shutdown() (blocking flush with 5 s timeout) instead of saveAll() directly, guaranteeing all pending I/O completes before the plugin unloads.

---