diff --git a/.gitignore b/.gitignore index 6346d58..f194a17 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,16 @@ runs run-data repo + +# Claude Code +.claude/ +CLAUDE.md + +# BMad +.agent/ +_bmad/ +_bmad-output/ +_bmb/ + +# compat mods (local jars for analysis) +compat-mods/*.jar diff --git a/src/main/java/vip/fubuki/playersync/PlayerSync.java b/src/main/java/vip/fubuki/playersync/PlayerSync.java index 09c6c0e..b2a2208 100644 --- a/src/main/java/vip/fubuki/playersync/PlayerSync.java +++ b/src/main/java/vip/fubuki/playersync/PlayerSync.java @@ -13,9 +13,9 @@ import net.neoforged.neoforge.event.server.ServerStartingEvent; import net.neoforged.neoforge.event.server.ServerStoppingEvent; import org.slf4j.Logger; import vip.fubuki.playersync.config.JdbcConfig; -import vip.fubuki.playersync.sync.ChatSync; import vip.fubuki.playersync.sync.VanillaSync; import vip.fubuki.playersync.util.JDBCsetUp; +import vip.fubuki.playersync.util.Tables; import java.sql.Connection; import java.sql.ResultSet; @@ -35,18 +35,22 @@ public class PlayerSync { private void commonSetup(final FMLCommonSetupEvent event) { VanillaSync.register(); - event.enqueueWork(() -> { - // read SYNC_CHAT only within the enqueueWork to reliably get the real - // config value and not its default value. - if (JdbcConfig.SYNC_CHAT.get()) { - LOGGER.info("Chat sync enabled."); - ChatSync.register(); - } - }); + // Chat sync removed. The `sync_chat` / `IsChatServer` / `ChatServerIP` / + // `ChatServerPort` keys in existing config files are now silently ignored + // (NeoForge's ModConfig loader skips unknown keys, so no crash on upgrade). } @SubscribeEvent public void onServerStarting(ServerStartingEvent event) throws SQLException { + // FIX COMPAT (C2): skip all MySQL init on single-player / integrated servers. + // Running PlayerSync in single-player makes no sense (no cross-server sync) and + // attempting to open a MySQL connection with default placeholder credentials on a + // laptop without a MySQL server produces noisy errors + degraded UX. + if (!event.getServer().isDedicatedServer()) { + LOGGER.info("PlayerSync: integrated server detected — skipping MySQL init (dedicated-server only)."); + return; + } + String dbName = JdbcConfig.DATABASE_NAME.get(); // FIX: Validate database name to prevent SQL injection via config. @@ -83,7 +87,7 @@ public class PlayerSync { // Step 4: Create and alter tables using fully qualified names. // Create player_data table JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`player_data` (" + + "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`" + Tables.playerData() + "` (" + "`uuid` char(36) NOT NULL," + "`inventory` mediumblob," + "`armor` blob," + @@ -105,8 +109,8 @@ public class PlayerSync { // Check and alter player_data table if columns are missing int columnCount = 0; try (JDBCsetUp.QueryResult queryResult = JDBCsetUp.executePreparedQuery( - "SELECT COUNT(*) AS column_count FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = 'player_data'", - dbName)) { + "SELECT COUNT(*) AS column_count FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ?", + dbName, Tables.playerData())) { ResultSet resultSet = queryResult.resultSet(); if (resultSet.next()) { columnCount = resultSet.getInt("column_count"); @@ -114,7 +118,7 @@ public class PlayerSync { } if (columnCount < 14) { JDBCsetUp.executeUpdate( - "ALTER TABLE `" + dbName + "`.`player_data` " + + "ALTER TABLE `" + dbName + "`.`" + Tables.playerData() + "` " + "ADD COLUMN left_hand blob, " + "ADD COLUMN cursors blob;" ); @@ -122,7 +126,7 @@ public class PlayerSync { // Create server_info table JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`server_info` (" + + "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`" + Tables.serverInfo() + "` (" + "`id` INT NOT NULL," + "`enable` boolean NOT NULL," + "`last_update` BIGINT NOT NULL," + @@ -132,82 +136,65 @@ public class PlayerSync { // FIX H-8: Use prepared statements for server_id to prevent SQL injection from config long current = System.currentTimeMillis(); JDBCsetUp.executePreparedUpdate( - "INSERT INTO `" + dbName + "`.`server_info`(id,enable,last_update) VALUES(?,true,?) ON DUPLICATE KEY UPDATE id=VALUES(id),enable=1,last_update=VALUES(last_update)", + "INSERT INTO `" + dbName + "`.`" + Tables.serverInfo() + "`(id,enable,last_update) VALUES(?,true,?) ON DUPLICATE KEY UPDATE id=VALUES(id),enable=1,last_update=VALUES(last_update)", JdbcConfig.SERVER_ID.get(), current ); JDBCsetUp.executePreparedUpdate( - "UPDATE `" + dbName + "`.`server_info` SET last_update=? WHERE id=?", + "UPDATE `" + dbName + "`.`" + Tables.serverInfo() + "` SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get() ); // Create curios table if the Curios mod is loaded if (ModList.get().isLoaded("curios")) { JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`curios` (" + + "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`" + Tables.curios() + "` (" + "uuid CHAR(36) NOT NULL, curios_item BLOB, PRIMARY KEY (uuid)" + ")" ); } - // Create Cobblemon table - if(ModList.get().isLoaded("cobblemon")){ - JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`cobblemon`(" + - "uuid CHAR(36) NOT NULL," + - "inv BLOB," + - "pokedex MEDIUMBLOB," + - "pc MEDIUMBLOB," + - "general BLOB," + - "PRIMARY KEY (uuid)" + - ")" - ); - - JDBCsetUp.executeUpdate( - "ALTER TABLE `" + dbName + "`.`cobblemon` MODIFY COLUMN pc MEDIUMBLOB" - ); - JDBCsetUp.executeUpdate( - "ALTER TABLE `" + dbName + "`.`cobblemon` MODIFY COLUMN pokedex MEDIUMBLOB" - ); - } + // Cobblemon support removed in this build (sync was main-thread blocking + SQL + // injection in the mixins). Existing `cobblemon` tables in the DB are kept intact + // for backward compat — they are simply no longer read or written. // Create backpack_data table if (ModList.get().isLoaded("sophisticatedbackpacks")) { JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`backpack_data` (" + + "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`" + Tables.backpackData() + "` (" + "uuid CHAR(36) NOT NULL, backpack_nbt MEDIUMBLOB, PRIMARY KEY (uuid)" + ");", 1 ); // Check if backpack_data table has the 'uuid' column try (JDBCsetUp.QueryResult backpackColCheck = JDBCsetUp.executePreparedQuery( - "SELECT COUNT(*) AS colCount FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = 'backpack_data' AND COLUMN_NAME = 'uuid'", - dbName)) { + "SELECT COUNT(*) AS colCount FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ? AND COLUMN_NAME = 'uuid'", + dbName, Tables.backpackData())) { ResultSet rsBackpackCol = backpackColCheck.resultSet(); if (rsBackpackCol.next() && rsBackpackCol.getInt("colCount") == 0) { LOGGER.info("Altering backpack_data table to add missing 'uuid' column."); - JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`backpack_data` ADD COLUMN uuid CHAR(36) NOT NULL", 1); - JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`backpack_data` ADD PRIMARY KEY (uuid)", 1); + JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`" + Tables.backpackData() + "` ADD COLUMN uuid CHAR(36) NOT NULL", 1); + JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`" + Tables.backpackData() + "` ADD PRIMARY KEY (uuid)", 1); } } } // Check and alter the 'advancements' column in player_data if necessary try (JDBCsetUp.QueryResult advColCheck = JDBCsetUp.executePreparedQuery( - "SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = 'player_data' AND COLUMN_NAME = 'advancements'", - dbName)) { + "SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ? AND COLUMN_NAME = 'advancements'", + dbName, Tables.playerData())) { ResultSet rsAdvCol = advColCheck.resultSet(); if (rsAdvCol.next()) { String dataType = rsAdvCol.getString("DATA_TYPE"); if (!"mediumblob".equalsIgnoreCase(dataType)) { LOGGER.info("Altering player_data table to modify 'advancements' column to MEDIUMBLOB."); - JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`player_data` MODIFY COLUMN advancements MEDIUMBLOB", 1); + JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`" + Tables.playerData() + "` MODIFY COLUMN advancements MEDIUMBLOB", 1); } } } // Create generic mod_player_data table for mod compatibility (Accessories, CosmeticArmor, Aether, etc.) JDBCsetUp.executeUpdate( - "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`mod_player_data` (" + + "CREATE TABLE IF NOT EXISTS `" + dbName + "`.`" + Tables.modPlayerData() + "` (" + "`uuid` CHAR(36) NOT NULL," + "`mod_id` VARCHAR(64) NOT NULL," + "`data_value` MEDIUMBLOB," + @@ -216,21 +203,41 @@ public class PlayerSync { ); try { - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE last_server=? AND online=1", JdbcConfig.SERVER_ID.get()); + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE last_server=? AND online=1", JdbcConfig.SERVER_ID.get()); } catch (Exception e) { LOGGER.error("An exception occurred while trying change wrong player-status\n" + e.getMessage()); } LOGGER.info("PlayerSync is ready!"); } + /** + * Alters a column to {@code targetType} only if its current {@code DATA_TYPE} + * differs. Skips expensive MDL + rebuild on every server start. + */ + private static void alterColumnIfNeeded(String dbName, String table, String column, String targetTypeLower) throws SQLException { + try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( + "SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA=? AND TABLE_NAME=? AND COLUMN_NAME=?", + dbName, table, column)) { + ResultSet rs = qr.resultSet(); + if (rs.next()) { + String current = rs.getString("DATA_TYPE"); + if (current != null && targetTypeLower.equalsIgnoreCase(current)) { + return; + } + } + } + LOGGER.info("Altering {}.{} column {} to {}", dbName, table, column, targetTypeLower.toUpperCase()); + JDBCsetUp.executeUpdate("ALTER TABLE `" + dbName + "`.`" + table + "` MODIFY COLUMN `" + column + "` " + targetTypeLower.toUpperCase()); + } + @SubscribeEvent public void onServerStopping(ServerStoppingEvent event) { - ChatSync.shutdown(); - vip.fubuki.playersync.util.SyncLogger.shutdown(); - // DO NOT call JDBCsetUp.shutdownPool() here! + // DO NOT call JDBCsetUp.shutdownPool() or SyncLogger.shutdown() here! // VanillaSync.onServerShutdown also subscribes to ServerStoppingEvent and - // needs the pool to save all player data. Event firing order is not guaranteed. - // The pool is shut down at the very end of VanillaSync.onServerShutdown instead. + // needs the pool to save all player data AND the logger to trace those saves. + // NeoForge does not guarantee handler ordering across @SubscribeEvent instances, + // so both the pool and the logger are shut down at the very end of + // VanillaSync.onServerShutdown — after parallel saves finish. } } diff --git a/src/main/java/vip/fubuki/playersync/config/JdbcConfig.java b/src/main/java/vip/fubuki/playersync/config/JdbcConfig.java index e0e0496..9bf400d 100644 --- a/src/main/java/vip/fubuki/playersync/config/JdbcConfig.java +++ b/src/main/java/vip/fubuki/playersync/config/JdbcConfig.java @@ -18,17 +18,21 @@ public class JdbcConfig { public static ModConfigSpec.ConfigValue> SYNC_WORLD; public static ModConfigSpec.BooleanValue SYNC_ADVANCEMENTS; public static ModConfigSpec.BooleanValue USE_SSL; - public static ModConfigSpec.BooleanValue SYNC_CHAT; - public static ModConfigSpec.BooleanValue IS_CHAT_SERVER; public static ModConfigSpec.BooleanValue KICK_WHEN_ALREADY_ONLINE; public static final ModConfigSpec.ConfigValue ITEM_PLACEHOLDER_TITLE_OVERRIDE; public static final ModConfigSpec.ConfigValue ITEM_PLACEHOLDER_DESCRIPTION_OVERRIDE; - public static ModConfigSpec.ConfigValue CHAT_SERVER_IP; - public static ModConfigSpec.IntValue CHAT_SERVER_PORT; public static ModConfigSpec.BooleanValue USE_LEGACY_SERIALIZATION; public static ModConfigSpec.ConfigValue SERVER_ID; + /** + * Optional table-name prefix prepended to every PlayerSync table. Use to share a + * single MySQL database with other mods (LuckPerms, custom mods, etc.) that may + * otherwise collide with generic names like {@code player_data} / {@code server_info}. + * Default is empty for backward compatibility with existing deployments. + */ + public static ModConfigSpec.ConfigValue TABLE_PREFIX; + static { ModConfigSpec.Builder COMMON_BUILDER = new ModConfigSpec.Builder(); @@ -39,16 +43,18 @@ public class JdbcConfig { USERNAME = COMMON_BUILDER.comment("username").define("user_name", "playersync"); PASSWORD = COMMON_BUILDER.comment("password").define("password", "pleaseChangeThisPassword"); DATABASE_NAME = COMMON_BUILDER.comment("database name").define("db_name","playersync"); + TABLE_PREFIX = COMMON_BUILDER.comment( + "Optional prefix prepended to every PlayerSync table (player_data, curios, backpack_data, ...).", + "Use to share a single MySQL database with other mods or legacy schemas.", + "Leave empty to keep the historical unprefixed names. Example: 'playersync_'.", + "Only alphanumeric characters and underscores are allowed." + ).define("table_prefix", ""); SERVER_ID = COMMON_BUILDER.comment("the server id should be unique").define("Server_id", new Random().nextInt(1,Integer.MAX_VALUE-1)); SYNC_WORLD = COMMON_BUILDER.comment("The worlds that will be synchronized. If running on a server, leave array empty.").define("sync_world", new ArrayList<>()); SYNC_ADVANCEMENTS = COMMON_BUILDER.comment("Whether to sync advancements between servers") .define("sync_advancements", true); - SYNC_CHAT = COMMON_BUILDER.comment("Whether synchronize chat").define("sync_chat", false); - IS_CHAT_SERVER = COMMON_BUILDER.comment("Whether recieve messages from other servers as host").define("IsChatServer",false); KICK_WHEN_ALREADY_ONLINE = COMMON_BUILDER.comment("Whether to kick player when already online on another server") .define("kick_when_already_online", true); - CHAT_SERVER_IP = COMMON_BUILDER.define("ChatServerIP","127.0.0.1"); - CHAT_SERVER_PORT = COMMON_BUILDER.defineInRange("ChatServerPort",7900,0,65535); USE_LEGACY_SERIALIZATION = COMMON_BUILDER.comment( "Use the old (pre-Base64) serialization format for writing data to the database.", "Set to true ONLY if you have older mod versions reading the same database.", diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinFileBackedPokemonStoreFactory.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinFileBackedPokemonStoreFactory.java deleted file mode 100644 index 03a6d3c..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinFileBackedPokemonStoreFactory.java +++ /dev/null @@ -1,62 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon; - -import com.cobblemon.mod.common.api.storage.PokemonStore; -import com.cobblemon.mod.common.api.storage.factory.FileBackedPokemonStoreFactory; -import com.cobblemon.mod.common.api.storage.party.PartyStore; -import com.cobblemon.mod.common.api.storage.pc.PCStore; -import kotlin.jvm.functions.Function1; -import net.minecraft.core.RegistryAccess; -import net.minecraft.nbt.CompoundTag; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Unique; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.Redirect; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -import vip.fubuki.playersync.util.JDBCsetUp; - -import java.sql.ResultSet; -import java.util.UUID; - -@Mixin(FileBackedPokemonStoreFactory.class) -public class MixinFileBackedPokemonStoreFactory { - @Unique - RegistryAccess playerSync$registryAccess; - - @Inject(method = "getStore", at = @At("HEAD")) - private > void getStore$playerSync(Class storeClass, UUID uuid, RegistryAccess registryAccess, Function1 constructor, CallbackInfoReturnable cir){ - this.playerSync$registryAccess = registryAccess; - } - - @Redirect(method = "getStore", at = @At(value = "INVOKE", target = "Lcom/cobblemon/mod/common/api/storage/PokemonStore;initialize()V")) - private void getStore$playerSync(PokemonStore instance) { - - String column; - if(instance instanceof PCStore){ - column = "pc"; - } else if(instance instanceof PartyStore){ - column = "inv"; - }else { - instance.initialize(); - return; - } - - String sql = "SELECT " + column + " FROM cobblemon WHERE uuid = '" + instance.getUuid() + "'"; - - try { - JDBCsetUp.QueryResult qr = vip.fubuki.playersync.util.JDBCsetUp.executeQuery(sql); - ResultSet rs = qr.resultSet(); - if (rs.next() && rs.getString(column) != null) { - CompoundTag compoundTag = new CompoundTag(); - instance.loadFromNBT(compoundTag, playerSync$registryAccess); - } - - rs.close(); - qr.close(); - } catch (java.sql.SQLException e) { - throw new RuntimeException(e); - } - - instance.initialize(); - } -} diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinNbtBackedPlayerData.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinNbtBackedPlayerData.java deleted file mode 100644 index 692651d..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinNbtBackedPlayerData.java +++ /dev/null @@ -1,91 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon; - -import com.cobblemon.mod.common.api.pokedex.PokedexManager; -import com.cobblemon.mod.common.api.storage.player.InstancedPlayerData; -import com.cobblemon.mod.common.api.storage.player.adapter.NbtBackedPlayerData; -import com.mojang.brigadier.exceptions.CommandSyntaxException; -import com.mojang.serialization.Codec; -import com.mojang.serialization.DataResult; -import net.minecraft.nbt.CompoundTag; -import net.minecraft.nbt.NbtOps; -import net.minecraft.nbt.NbtUtils; -import net.minecraft.nbt.Tag; -import net.minecraft.resources.ResourceLocation; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -import vip.fubuki.playersync.mixin.cobblemon.accessor.FileBasedPlayerDataStoreBackendAccessor; -import vip.fubuki.playersync.mixin.cobblemon.accessor.NbtBackedPlayerDataAccessor; -import vip.fubuki.playersync.util.JDBCsetUp; - -import java.sql.ResultSet; -import java.sql.SQLException; -import java.util.UUID; - -@Mixin(NbtBackedPlayerData.class) -public class MixinNbtBackedPlayerData { - - @Inject(method = "save", at = @org.spongepowered.asm.mixin.injection.At("HEAD")) - private void save$playerSync(InstancedPlayerData playerData, CallbackInfo ci) { - if(playerData instanceof PokedexManager){ - Codec codec = ((NbtBackedPlayerDataAccessor)this).getCodec(); - DataResult encodeResult = codec.encodeStart( - NbtOps.INSTANCE, - playerData - ); - - CompoundTag nbt = (CompoundTag) encodeResult.result().orElseThrow(); - - String serializedData = nbt.toString(); - String sql = "INSERT INTO cobblemon (uuid, pokedex) VALUES ('" + playerData.getUuid() + "', '" + serializedData + "') " + - "ON DUPLICATE KEY UPDATE pokedex = '" + serializedData + "'"; - try { - JDBCsetUp.executeUpdate(sql); - } catch (SQLException e) { - throw new RuntimeException(e); - } - } - } - - @Inject(method = "load", at = @org.spongepowered.asm.mixin.injection.At("HEAD"), cancellable = true) - private void load$playerSync(UUID uuid, CallbackInfoReturnable cir){ - if(!((FileBasedPlayerDataStoreBackendAccessor) this).getType().getId().equals(ResourceLocation.fromNamespaceAndPath("cobblemon", "pokedex"))){ - return; - } - - String sql = "SELECT pokedex FROM cobblemon WHERE uuid = '" + uuid + "'"; - CompoundTag loadedNbt; - try { - JDBCsetUp.QueryResult qr = JDBCsetUp.executeQuery(sql); - ResultSet rs = qr.resultSet(); - if (rs.next()) { - String serializedData = rs.getString("pokedex"); - - if(serializedData == null){ - rs.close(); - qr.close(); - return; - } - - loadedNbt = NbtUtils.snbtToStructure(serializedData); - - if(!loadedNbt.isEmpty()){ - Codec codec = ((NbtBackedPlayerDataAccessor)this).getCodec(); - DataResult decodeResult = codec.parse( - NbtOps.INSTANCE, - loadedNbt - ); - InstancedPlayerData playerData = decodeResult.result().orElseThrow(); - cir.setReturnValue(playerData); - } - } - - rs.close(); - qr.close(); - } catch (SQLException | CommandSyntaxException e) { - throw new RuntimeException(e); - } - } - -} diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPCStore.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPCStore.java deleted file mode 100644 index 47efff8..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPCStore.java +++ /dev/null @@ -1,59 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon; - -import com.cobblemon.mod.common.api.storage.pc.PCStore; -import com.mojang.brigadier.exceptions.CommandSyntaxException; -import net.minecraft.core.RegistryAccess; -import net.minecraft.nbt.CompoundTag; -import net.minecraft.nbt.TagParser; -import org.spongepowered.asm.mixin.Final; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Shadow; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.ModifyVariable; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -import vip.fubuki.playersync.util.JDBCsetUp; -import vip.fubuki.playersync.util.LocalJsonUtil; - -import java.sql.ResultSet; -import java.sql.SQLException; -import java.util.UUID; - -@Mixin(PCStore.class) -public class MixinPCStore { - @Final - @Shadow - private UUID uuid; - - @Inject(method = "saveToNBT",at = @At("TAIL")) - private void saveToNBT$playerSync(CompoundTag nbt, RegistryAccess registryAccess, CallbackInfoReturnable cir) { - String serializedData = nbt.toString(); - String sql = "INSERT INTO cobblemon (uuid, pc) VALUES ('" + this.uuid.toString() + "', '" + serializedData + "') " + - "ON DUPLICATE KEY UPDATE pc = '" + serializedData + "'"; - try { - JDBCsetUp.executeUpdate(sql); - } catch (SQLException e) { - throw new RuntimeException(e); - } - } - - @ModifyVariable(method = "loadFromNBT", at = @At("HEAD"), argsOnly = true, name = "arg1") - private CompoundTag loadFromNBT$playerSync(CompoundTag value) { - String sql = "SELECT pc FROM cobblemon WHERE uuid = '" + this.uuid.toString() + "'"; - CompoundTag loadedNbt = value; - try { - JDBCsetUp.QueryResult qr = JDBCsetUp.executeQuery(sql); - ResultSet rs = qr.resultSet(); - if (rs.next()) { - String serializedData = rs.getString("pc"); - loadedNbt = TagParser.parseTag(LocalJsonUtil.cleanSnbt(serializedData)); - } - - rs.close(); - qr.close(); - } catch (SQLException | CommandSyntaxException e) { - throw new RuntimeException(e); - } - return loadedNbt; - } -} diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPartyStore.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPartyStore.java deleted file mode 100644 index c2782f0..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/MixinPartyStore.java +++ /dev/null @@ -1,59 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon; - -import com.cobblemon.mod.common.api.storage.party.PartyStore; -import com.mojang.brigadier.exceptions.CommandSyntaxException; -import net.minecraft.core.RegistryAccess; -import net.minecraft.nbt.CompoundTag; -import net.minecraft.nbt.TagParser; -import org.spongepowered.asm.mixin.Final; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Shadow; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.ModifyVariable; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -import vip.fubuki.playersync.util.JDBCsetUp; -import vip.fubuki.playersync.util.LocalJsonUtil; - -import java.sql.ResultSet; -import java.sql.SQLException; -import java.util.UUID; - -@Mixin(PartyStore.class) -public class MixinPartyStore { - @Final - @Shadow - private UUID uuid; - - @Inject(method = "saveToNBT",at = @At("TAIL")) - private void saveToNBT$playerSync(CompoundTag nbt, RegistryAccess registryAccess, CallbackInfoReturnable cir) { - String serializedData = nbt.toString(); - String sql = "INSERT INTO cobblemon (uuid, inv) VALUES ('" + this.uuid.toString() + "', '" + serializedData + "') " + - "ON DUPLICATE KEY UPDATE inv = '" + serializedData + "'"; - try { - JDBCsetUp.executeUpdate(sql); - } catch (SQLException e) { - throw new RuntimeException(e); - } - } - - @ModifyVariable(method = "loadFromNBT*", at = @At("HEAD"), argsOnly = true, name = "arg1") - private CompoundTag loadFromNBT$playerSync(CompoundTag value) { - String sql = "SELECT inv FROM cobblemon WHERE uuid = '" + this.uuid.toString() + "'"; - CompoundTag loadedNbt = value; - try { - JDBCsetUp.QueryResult qr = JDBCsetUp.executeQuery(sql); - ResultSet rs = qr.resultSet(); - if (rs.next()) { - String serializedData = rs.getString("inv"); - loadedNbt = TagParser.parseTag(LocalJsonUtil.cleanSnbt(serializedData)); - } - - rs.close(); - qr.close(); - } catch (SQLException | CommandSyntaxException e) { - throw new RuntimeException(e); - } - return loadedNbt; - } -} diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/FileBasedPlayerDataStoreBackendAccessor.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/FileBasedPlayerDataStoreBackendAccessor.java deleted file mode 100644 index 31ce33c..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/FileBasedPlayerDataStoreBackendAccessor.java +++ /dev/null @@ -1,12 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon.accessor; - -import com.cobblemon.mod.common.api.storage.player.PlayerInstancedDataStoreType; -import com.cobblemon.mod.common.api.storage.player.adapter.FileBasedPlayerDataStoreBackend; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.gen.Accessor; - -@Mixin(FileBasedPlayerDataStoreBackend.class) -public interface FileBasedPlayerDataStoreBackendAccessor { - @Accessor - PlayerInstancedDataStoreType getType(); -} diff --git a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/NbtBackedPlayerDataAccessor.java b/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/NbtBackedPlayerDataAccessor.java deleted file mode 100644 index 65f1d2b..0000000 --- a/src/main/java/vip/fubuki/playersync/mixin/cobblemon/accessor/NbtBackedPlayerDataAccessor.java +++ /dev/null @@ -1,14 +0,0 @@ -package vip.fubuki.playersync.mixin.cobblemon.accessor; - -import com.cobblemon.mod.common.api.storage.player.InstancedPlayerData; -import com.cobblemon.mod.common.api.storage.player.adapter.DexDataNbtBackend; -import com.mojang.serialization.Codec; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.gen.Accessor; - -@Mixin(DexDataNbtBackend.class) -public interface NbtBackedPlayerDataAccessor { - @Accessor("codec") - Codec getCodec(); - -} diff --git a/src/main/java/vip/fubuki/playersync/sync/ChatSync.java b/src/main/java/vip/fubuki/playersync/sync/ChatSync.java deleted file mode 100644 index 798ca2d..0000000 --- a/src/main/java/vip/fubuki/playersync/sync/ChatSync.java +++ /dev/null @@ -1,55 +0,0 @@ -package vip.fubuki.playersync.sync; - -import com.mojang.logging.LogUtils; -import net.neoforged.neoforge.common.NeoForge; -import org.slf4j.Logger; -import vip.fubuki.playersync.config.JdbcConfig; -import vip.fubuki.playersync.sync.chat.ChatSyncClient; -import vip.fubuki.playersync.sync.chat.ChatSyncServer; - -import java.io.IOException; - -public class ChatSync { - public static final Logger LOGGER = LogUtils.getLogger(); - private static ChatSyncServer chatSyncServer; - private static ChatSyncClient chatSyncClient; - - public static void register(){ - if(JdbcConfig.IS_CHAT_SERVER.get()) { - LOGGER.info("Trying to setup chat server at port " + JdbcConfig.CHAT_SERVER_PORT.get()); - new Thread(()->{ - chatSyncServer = new ChatSyncServer(); - try { - chatSyncServer.run(); - } catch (IOException e) { - LOGGER.error("Unable to start chat server", e); - } - }, "ChatSync-Server").start(); - } - - new Thread(()->{ - try { - Thread.sleep(2000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - - LOGGER.info("Trying to connect to chat server " - + JdbcConfig.CHAT_SERVER_IP.get() - + ":" - + JdbcConfig.CHAT_SERVER_PORT.get()); - chatSyncClient = new ChatSyncClient(); - chatSyncClient.run(); - }, "ChatSync-Client").start(); - NeoForge.EVENT_BUS.register(ChatSyncClient.class); - } - - public static void shutdown() { - if (chatSyncServer != null) { - chatSyncServer.shutdown(); - } - if (chatSyncClient != null) { - chatSyncClient.shutdown(); - } - } -} diff --git a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java index 92bdc12..aa370b3 100644 --- a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java +++ b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java @@ -49,6 +49,7 @@ import vip.fubuki.playersync.sync.addons.ModsSupport; import vip.fubuki.playersync.util.JDBCsetUp; import vip.fubuki.playersync.util.LocalJsonUtil; import vip.fubuki.playersync.util.PSThreadPoolFactory; +import vip.fubuki.playersync.util.Tables; import java.io.File; import java.io.IOException; @@ -129,7 +130,7 @@ public class VanillaSync { // Use try-with-resources to prevent connection leaks String advancementsData; try (JDBCsetUp.QueryResult advancementsQuery = JDBCsetUp.executePreparedQuery( - "SELECT advancements FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT advancements FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { ResultSet advancementsResultSet = advancementsQuery.resultSet(); if (!advancementsResultSet.next()) { @@ -203,7 +204,7 @@ public class VanillaSync { // First query: check basic player data using prepared statement try (JDBCsetUp.QueryResult qr1 = JDBCsetUp.executePreparedQuery( - "SELECT online, last_server FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT online, last_server FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { ResultSet rs1 = qr1.resultSet(); if (!rs1.next()) { PlayerSync.LOGGER.info("A new-player connection detected"); @@ -219,7 +220,7 @@ public class VanillaSync { int alreadyKicked = 0; if (JdbcConfig.KICK_WHEN_ALREADY_ONLINE.get() && online && lastServer != JdbcConfig.SERVER_ID.get()) { try (JDBCsetUp.QueryResult qr2 = JDBCsetUp.executePreparedQuery( - "SELECT last_update, enable FROM server_info WHERE id=?", lastServer)) { + "SELECT last_update, enable FROM " + Tables.serverInfo() + " WHERE id=?", lastServer)) { ResultSet rs2 = qr2.resultSet(); if (rs2.next()) { long last_update = rs2.getLong("last_update"); @@ -229,7 +230,7 @@ public class VanillaSync { event.getConnection().disconnect(Component.translatableWithFallback("playersync.already_online","You can't join more than one synchronization server at the same time.")); alreadyKicked = 1; } else { - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET enable=0 WHERE id=?", lastServer); + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.serverInfo() + " SET enable=0 WHERE id=?", lastServer); } } } @@ -324,7 +325,7 @@ public class VanillaSync { // This keeps last_server pointing to the old server so this poll can detect it. for (int attempt = 0; attempt < 60; attempt++) { try (JDBCsetUp.QueryResult qrCheck = JDBCsetUp.executePreparedQuery( - "SELECT online, last_server FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT online, last_server FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { ResultSet rsCheck = qrCheck.resultSet(); if (!rsCheck.next()) break; // new player, nothing pending int otherServer = rsCheck.getInt("last_server"); @@ -349,14 +350,14 @@ public class VanillaSync { // This is safe because: (1) the old server's data+online=0 write already completed, // (2) any future writes from the old server will be blocked by AND last_server=?. JDBCsetUp.executePreparedUpdate( - "UPDATE player_data SET last_server=? WHERE uuid=?", + "UPDATE " + Tables.playerData() + " SET last_server=? WHERE uuid=?", JdbcConfig.SERVER_ID.get(), player_uuid); // === PHASE 1: DB reads on background thread (thread-safe) === boolean playerExists; try (JDBCsetUp.QueryResult qr1 = JDBCsetUp.executePreparedQuery( - "SELECT uuid FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT uuid FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { playerExists = qr1.resultSet().next(); } @@ -384,7 +385,7 @@ public class VanillaSync { final String leftHand, cursors, armorData, inventoryData, enderChestData, effectData; try (JDBCsetUp.QueryResult qr2 = JDBCsetUp.executePreparedQuery( - "SELECT * FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT * FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { ResultSet rs2 = qr2.resultSet(); if (!rs2.next()) { PlayerSync.LOGGER.warn("No data found for existing player {}", player_uuid); @@ -407,7 +408,7 @@ public class VanillaSync { final String curiosData; if (ModList.get().isLoaded("curios")) { try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT curios_item FROM curios WHERE uuid=?", player_uuid)) { + "SELECT curios_item FROM " + Tables.curios() + " WHERE uuid=?", player_uuid)) { ResultSet rs = qr.resultSet(); curiosData = rs.next() ? rs.getString("curios_item") : null; } @@ -416,7 +417,7 @@ public class VanillaSync { final String accessoriesData; if (ModList.get().isLoaded("accessories")) { try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player_uuid, "accessories")) { ResultSet rs = qr.resultSet(); accessoriesData = rs.next() ? rs.getString("data_value") : null; @@ -426,7 +427,7 @@ public class VanillaSync { final String cosmeticArmorData; if (ModList.get().isLoaded("cosmeticarmorreworked")) { try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player_uuid, "cosmeticarmor")) { ResultSet rs = qr.resultSet(); cosmeticArmorData = rs.next() ? rs.getString("data_value") : null; @@ -435,7 +436,7 @@ public class VanillaSync { final String attachmentsData; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player_uuid, "neoforge_attachments")) { ResultSet rs = qr.resultSet(); attachmentsData = rs.next() ? rs.getString("data_value") : null; @@ -574,11 +575,15 @@ public class VanillaSync { int[] cached = connectCheckCache.remove(player_uuid); if (!JdbcConfig.KICK_WHEN_ALREADY_ONLINE.get()) { - try { - JDBCsetUp.executePreparedUpdate( - "UPDATE player_data SET online=1 WHERE uuid=?", - player_uuid); - } catch (SQLException ignored) {} + // FIX PERF (C1): online=1 is fire-and-forget; no login-critical decision depends + // on the write completing synchronously. Keeping this off the main thread saves + // one MySQL round-trip per join. + executorService.execute(() -> { + try { + JDBCsetUp.executePreparedUpdate( + "UPDATE " + Tables.playerData() + " SET online=1 WHERE uuid=?", player_uuid); + } catch (SQLException ignored) {} + }); return; } @@ -603,7 +608,7 @@ public class VanillaSync { boolean online = false; int lastServer = 0; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT online, last_server FROM player_data WHERE uuid=?", player_uuid)) { + "SELECT online, last_server FROM " + Tables.playerData() + " WHERE uuid=?", player_uuid)) { ResultSet rs = qr.resultSet(); if (rs.next()) { online = rs.getBoolean("online"); @@ -612,7 +617,7 @@ public class VanillaSync { } if (online && lastServer != JdbcConfig.SERVER_ID.get()) { try (JDBCsetUp.QueryResult qr2 = JDBCsetUp.executePreparedQuery( - "SELECT last_update, enable FROM server_info WHERE id=?", lastServer)) { + "SELECT last_update, enable FROM " + Tables.serverInfo() + " WHERE id=?", lastServer)) { ResultSet rs2 = qr2.resultSet(); if (rs2.next()) { long lastUpdate = rs2.getLong("last_update"); @@ -624,16 +629,23 @@ public class VanillaSync { "You can't join more than one synchronization server at the same time.")); return; } - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET enable=0 WHERE id=?", lastServer); + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.serverInfo() + " SET enable=0 WHERE id=?", lastServer); } } } } - // Mark online=1 — only DB call on main thread in the fast path (1 query instead of 4) - JDBCsetUp.executePreparedUpdate( - "UPDATE player_data SET online=1 WHERE uuid=?", - player_uuid); + // FIX PERF (C1): Mark online=1 asynchronously — no main-thread MySQL round-trip. + // The cache-based kick decision above is already final; this write only updates + // the persistent flag for cross-server detection, which tolerates a few ms of delay. + executorService.execute(() -> { + try { + JDBCsetUp.executePreparedUpdate( + "UPDATE " + Tables.playerData() + " SET online=1 WHERE uuid=?", player_uuid); + } catch (SQLException e) { + PlayerSync.LOGGER.error("Async online=1 update failed for {}", player_uuid, e); + } + }); } catch (Exception e) { PlayerSync.LOGGER.error("Error during kick check for player {}", player_uuid, e); } @@ -862,7 +874,7 @@ public class VanillaSync { // Always update server heartbeat — async, never blocks main thread executorService.submit(() -> { try { - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.serverInfo() + " SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); } catch (SQLException e) { PlayerSync.LOGGER.error("Error updating server heartbeat on SaveToFile", e); @@ -936,7 +948,8 @@ public class VanillaSync { // === MAIN THREAD: Snapshot (entity reads, fast) === final PlayerDataSnapshot snapshot = snapshotPlayerData(player); final Map backpackSnapshots = ModsSupport.snapshotBackpackData(player); - final List ssUuids = ModsSupport.collectSSUuids(player); + // FIX C3: snapshot SS CompoundTags on main thread (was a background-thread read). + final Map ssSnapshots = ModsSupport.snapshotSSData(ModsSupport.collectSSUuids(player)); final List rs2DiskUuids; final ServerLevel rs2Level; final HolderLookup.Provider rs2Registry; @@ -956,7 +969,7 @@ public class VanillaSync { // FIX ANTI-DUPLICATION: atomic data+online=0 with last_server guard writeSnapshotToDB(snapshot, true); ModsSupport.saveBackpackSnapshots(backpackSnapshots); - ModsSupport.saveSSByUuids(ssUuids); + ModsSupport.saveSSSnapshots(ssSnapshots); if (!rs2DiskUuids.isEmpty() && rs2Level != null) { ModsSupport.saveRS2DisksByLevel(rs2DiskUuids, rs2Level, rs2Registry); } @@ -965,7 +978,7 @@ public class VanillaSync { } catch (Exception e) { PlayerSync.LOGGER.error("Error saving player {} on shutdown", puuid, e); try { - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=? AND last_server=?", + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=? AND last_server=?", puuid, JdbcConfig.SERVER_ID.get()); } catch (Exception e2) { PlayerSync.LOGGER.error("CRITICAL: Failed to mark player {} offline on shutdown", puuid, e2); @@ -975,7 +988,7 @@ public class VanillaSync { } catch (Exception e) { PlayerSync.LOGGER.error("Error snapshotting player {} on shutdown", puuid, e); - try { JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=? AND last_server=?", puuid, JdbcConfig.SERVER_ID.get()); } + try { JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=? AND last_server=?", puuid, JdbcConfig.SERVER_ID.get()); } catch (Exception ignored) {} } } @@ -990,7 +1003,7 @@ public class VanillaSync { PlayerSync.LOGGER.error("Error waiting for shutdown saves", e); } } - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET enable=0 WHERE id=?", JdbcConfig.SERVER_ID.get()); + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.serverInfo() + " SET enable=0 WHERE id=?", JdbcConfig.SERVER_ID.get()); // Shut down the background executor — no new tasks after this point executorService.shutdown(); @@ -1006,6 +1019,10 @@ public class VanillaSync { // Previously this was in PlayerSync.onServerStopping which could fire BEFORE // this handler, closing the pool while shutdown saves were still running. JDBCsetUp.shutdownPool(); + // FIX REGRESSION: flush+shutdown the dedicated logger here, AFTER all shutdown + // saves have logged their completion. Previously SyncLogger.shutdown() fired in + // PlayerSync.onServerStopping, dropping every save log entry on the floor. + vip.fubuki.playersync.util.SyncLogger.shutdown(); } /** @@ -1038,14 +1055,14 @@ public class VanillaSync { if (deadPlayerWhileLogging.remove(player_uuid)) { PlayerSync.LOGGER.warn("A dead or dying player was kicked, uuid: {}", player_uuid); - try { - // FIX: No last_server guard here. These paths fire before doPlayerJoin sets - // last_server, so the guard would fail and online would stay stuck at 1. - // Safe because these paths don't write player DATA — just the online flag. - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=?", player_uuid); - } catch (SQLException e) { - PlayerSync.LOGGER.error("Error marking dead player offline: {}", player_uuid, e); - } + // FIX PERF (C1): async — main thread does not wait for MySQL. + executorService.execute(() -> { + try { + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=?", player_uuid); + } catch (SQLException e) { + PlayerSync.LOGGER.error("Error marking dead player offline: {}", player_uuid, e); + } + }); syncNotCompletedPlayer.remove(player_uuid); removePlayerLock(player_uuid); return; @@ -1054,12 +1071,14 @@ public class VanillaSync { if (syncNotCompletedPlayer.remove(player_uuid)) { PlayerSync.LOGGER.warn("Player {} logged out with uncompleted sync. Data won't be saved for safety.", player_uuid); SyncLogger.saveSkipped(player_uuid, "LOGOUT", "Sync not completed — data preserved in DB, .dat data discarded"); - try { - // FIX: No last_server guard — same reason as above. - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=?", player_uuid); - } catch (SQLException e) { - PlayerSync.LOGGER.error("Error marking unsynced player offline: {}", player_uuid, e); - } + // FIX PERF (C1): async. + executorService.execute(() -> { + try { + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=?", player_uuid); + } catch (SQLException e) { + PlayerSync.LOGGER.error("Error marking unsynced player offline: {}", player_uuid, e); + } + }); removePlayerLock(player_uuid); return; } @@ -1068,6 +1087,9 @@ public class VanillaSync { Player player = event.getEntity(); ReentrantLock lock = getPlayerLock(player_uuid); lock.lock(); + // Declared outside the try so the outer catch can complete/remove the future + // if snapshot capture or task submission fails (see FIX REGRESSION below). + CompletableFuture saveFuture = null; try { // FIX ANTI-DUPLICATION: Force-close the disconnecting player's container FIRST. // If another player is viewing this player's backpack, the container stays open @@ -1076,15 +1098,36 @@ public class VanillaSync { if (player instanceof ServerPlayer sp && sp.containerMenu != sp.inventoryMenu) { sp.closeContainer(); } - // Also close any other player's view of this player's backpack containers - if (player.getServer() != null) { - for (ServerPlayer other : player.getServer().getPlayerList().getPlayers()) { - if (other == player) continue; - if (other.containerMenu != other.inventoryMenu) { - // Close any open container to prevent post-snapshot modifications - // This is aggressive but safe — the viewer just sees their inventory close - // TODO: Only close if the container is specifically this player's backpack - // For now, closing all is safer than risking duplication + // FIX CRITICAL ANTI-DUP: close every other player's container menu if it was + // opened against this disconnecting player's inventory/backpack. If another + // player keeps the container open and takes items after our snapshot, those + // items are duplicated (the snapshot contains them, and the other player has them). + // We conservatively close all non-inventory containers referencing this player's + // inventory slots or any menu whose class name hints at a Sophisticated Backpacks + // container. The viewer just sees their GUI close — no data loss. + // FIX COMPAT: Close only containers that actually reference the disconnecting + // player's inventory/enderchest. Previous version also closed any menu whose + // class name contained "accessor"/"curio"/... which could force-close unrelated + // mod menus mid-transaction. The slot-reference scan is both correct and safe + // across every modded menu. + if (player instanceof ServerPlayer disconnecting && disconnecting.getServer() != null) { + net.minecraft.world.entity.player.Inventory srcInv = disconnecting.getInventory(); + net.minecraft.world.SimpleContainer srcEnder = disconnecting.getEnderChestInventory(); + for (ServerPlayer other : disconnecting.getServer().getPlayerList().getPlayers()) { + if (other == disconnecting) continue; + net.minecraft.world.inventory.AbstractContainerMenu menu = other.containerMenu; + if (menu == other.inventoryMenu) continue; + boolean shouldClose = false; + try { + for (net.minecraft.world.inventory.Slot slot : menu.slots) { + if (slot.container == srcInv || slot.container == srcEnder) { + shouldClose = true; + break; + } + } + } catch (Exception ignored) {} + if (shouldClose) { + try { other.closeContainer(); } catch (Exception ignored) {} } } } @@ -1098,7 +1141,8 @@ public class VanillaSync { // Collect backpack/SS/RS2 data — snapshots on main thread (no async reads) final Map backpackSnapshots = ModsSupport.snapshotBackpackData(player); - final List ssUuids = ModsSupport.collectSSUuids(player); + // FIX C3: SS CompoundTags snapshotted on main thread (frozen copies). + final Map ssSnapshots = ModsSupport.snapshotSSData(ModsSupport.collectSSUuids(player)); final List rs2DiskUuids; final ServerLevel rs2Level; final HolderLookup.Provider rs2RegistryAccess; @@ -1116,7 +1160,20 @@ public class VanillaSync { // The online flag stays 1 until the async save completes → kick mechanism // prevents premature rejoin on other servers, and pendingLogoutSaves prevents // premature rejoin on the same server. - CompletableFuture saveFuture = CompletableFuture.runAsync(() -> { + // + // FIX CRITICAL RACE (B1): Register the future in pendingLogoutSaves BEFORE + // submitting the work. Previously runAsync was submitted first — a fast + // reconnect could observe pendingLogoutSaves.get(uuid)==null while the save + // was already queued → doPlayerJoin would proceed without waiting. + saveFuture = new CompletableFuture<>(); + pendingLogoutSaves.put(player_uuid, saveFuture); + + final CompletableFuture futureRef = saveFuture; + // FIX REGRESSION: handle RejectedExecutionException if the executor is + // already shut down (concurrent with server stop). Without this, the future + // stays forever in pendingLogoutSaves and blocks future rejoins for 15s+. + try { + executorService.execute(() -> { try { // FIX ANTI-DUPLICATION: writeSnapshotToDB with setOffline=true // atomically writes data + online=0 in a SINGLE UPDATE, AND guards @@ -1124,7 +1181,7 @@ public class VanillaSync { // race where a slow async save overwrites fresher data from another server. writeSnapshotToDB(snapshot, true); ModsSupport.saveBackpackSnapshots(backpackSnapshots); - ModsSupport.saveSSByUuids(ssUuids); + ModsSupport.saveSSSnapshots(ssSnapshots); if (!rs2DiskUuids.isEmpty() && rs2Level != null) { ModsSupport.saveRS2DisksByLevel(rs2DiskUuids, rs2Level, rs2RegistryAccess); } @@ -1135,7 +1192,7 @@ public class VanillaSync { SyncLogger.saveFailed(player_uuid, "LOGOUT", e.getMessage()); // If the atomic write failed, still try to set online=0 try { - JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=? AND last_server=?", + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=? AND last_server=?", player_uuid, JdbcConfig.SERVER_ID.get()); } catch (Exception e2) { PlayerSync.LOGGER.error("CRITICAL: Failed to mark player {} offline", player_uuid, e2); @@ -1143,16 +1200,29 @@ public class VanillaSync { } finally { removePlayerLock(player_uuid); pendingLogoutSaves.remove(player_uuid); + futureRef.complete(null); } - }, executorService); - - pendingLogoutSaves.put(player_uuid, saveFuture); + }); + } catch (java.util.concurrent.RejectedExecutionException rex) { + // Executor is shut down (server stopping, or pool in unusable state) — + // drain the future so no join thread is stuck waiting 15 s on .get(). + PlayerSync.LOGGER.warn("Logout save executor rejected task for player {} (likely shutdown in progress)", player_uuid); + pendingLogoutSaves.remove(player_uuid); + futureRef.completeExceptionally(rex); + removePlayerLock(player_uuid); + } } catch (Exception e) { PlayerSync.LOGGER.error("Error during player logout save for {}", player_uuid, e); - try { JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE uuid=? AND last_server=?", player_uuid, JdbcConfig.SERVER_ID.get()); } + try { JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.playerData() + " SET online=0 WHERE uuid=? AND last_server=?", player_uuid, JdbcConfig.SERVER_ID.get()); } catch (Exception ignored) {} removePlayerLock(player_uuid); + // FIX REGRESSION: if snapshot failed AFTER pendingLogoutSaves.put, complete + // the future so a rejoining doPlayerJoin doesn't hang 15 s on .get(). + if (saveFuture != null) { + pendingLogoutSaves.remove(player_uuid); + saveFuture.completeExceptionally(e); + } } finally { lock.unlock(); } @@ -1318,12 +1388,12 @@ public class VanillaSync { // and ALL subsequent writes with AND last_server=? fail silently → player data // is never saved → "players lose everything" on next login. JDBCsetUp.executePreparedUpdate( - "INSERT INTO player_data (uuid, armor, inventory, enderchest, advancements, effects, xp, food_level, health, score, left_hand, cursors, online, last_server) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?)", + "INSERT INTO " + Tables.playerData() + " (uuid, armor, inventory, enderchest, advancements, effects, xp, food_level, health, score, left_hand, cursors, online, last_server) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?)", player_uuid, equipment.toString(), inventoryMap.toString(), ender_chest.toString(), json, effectMap.toString(), XP, food_level, health, score, left_hand, cursors, JdbcConfig.SERVER_ID.get()); } else { // FIX: Use COALESCE for advancements to avoid wiping valid DB data with empty string JDBCsetUp.executePreparedUpdate( - "UPDATE player_data SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(NULLIF(?, ''), advancements), left_hand=?, cursors=? WHERE uuid=?", + "UPDATE " + Tables.playerData() + " SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(NULLIF(?, ''), advancements), left_hand=?, cursors=? WHERE uuid=?", inventoryMap.toString(), equipment.toString(), XP, effectMap.toString(), ender_chest.toString(), score, food_level, health, json, left_hand, cursors, player_uuid); } } @@ -1452,8 +1522,8 @@ public class VanillaSync { // Now: 1 connection, 1 commit, automatic rollback on failure. String serverGuard = "(last_server=? OR last_server IS NULL)"; String coreSql = setOffline - ? "UPDATE player_data SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(?, advancements), left_hand=?, cursors=?, online=0, last_server=? WHERE uuid=? AND " + serverGuard - : "UPDATE player_data SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(?, advancements), left_hand=?, cursors=?, last_server=? WHERE uuid=? AND " + serverGuard; + ? "UPDATE " + Tables.playerData() + " SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(?, advancements), left_hand=?, cursors=?, online=0, last_server=? WHERE uuid=? AND " + serverGuard + : "UPDATE " + Tables.playerData() + " SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=COALESCE(?, advancements), left_hand=?, cursors=?, last_server=? WHERE uuid=? AND " + serverGuard; // Build batch of all statements List batch = new ArrayList<>(); @@ -1463,13 +1533,13 @@ public class VanillaSync { s.inventory(), s.equipment(), s.xp(), s.effects(), s.enderChest(), s.score(), s.foodLevel(), s.health(), s.advancements(), s.leftHand(), s.cursors(), serverId, s.uuid(), serverId}); // 2. Curios - String curioGuard = "EXISTS (SELECT 1 FROM player_data WHERE uuid=? AND " + serverGuard + ")"; + String curioGuard = "EXISTS (SELECT 1 FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard + ")"; if (s.curiosData() != null) { batch.add(new Object[]{ - "UPDATE curios SET curios_item=? WHERE uuid=? AND " + curioGuard, + "UPDATE " + Tables.curios() + " SET curios_item=? WHERE uuid=? AND " + curioGuard, s.curiosData(), s.uuid(), s.uuid(), serverId}); batch.add(new Object[]{ - "INSERT IGNORE INTO curios (uuid, curios_item) SELECT ?, ? FROM player_data WHERE uuid=? AND " + serverGuard, + "INSERT IGNORE INTO " + Tables.curios() + " (uuid, curios_item) SELECT ?, ? FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard, s.uuid(), s.curiosData(), s.uuid(), serverId}); } @@ -1478,17 +1548,27 @@ public class VanillaSync { addModDataToBatch(batch, s.uuid(), "cosmeticarmor", s.cosmeticArmorData(), serverId, serverGuard); addModDataToBatch(batch, s.uuid(), "neoforge_attachments", s.attachmentsData(), serverId, serverGuard); - // Execute all in one transaction - JDBCsetUp.executeBatchTransaction(batch.toArray(new Object[0][])); + // Execute all in one transaction. First statement is the core UPDATE on + // player_data — if it affects 0 rows, the last_server guard blocked the write + // (another server already claimed this player). Logging this is crucial for + // diagnosing silent data-loss scenarios that were previously invisible. + int[] counts = JDBCsetUp.executeBatchTransaction(batch.toArray(new Object[0][])); + if (counts.length > 0 && counts[0] == 0) { + SyncLogger.guardBlocked(s.uuid(), serverId, + "core UPDATE affected 0 rows — player_data.last_server no longer matches this server or row was removed"); + PlayerSync.LOGGER.warn( + "PlayerSync: core write blocked by last_server guard for {} (server={}). Data was NOT persisted — another server has claimed this player.", + s.uuid(), serverId); + } } private static void addModDataToBatch(List batch, String uuid, String modId, String data, int serverId, String serverGuard) { if (data == null) return; batch.add(new Object[]{ - "UPDATE mod_player_data SET data_value=? WHERE uuid=? AND mod_id=? AND EXISTS (SELECT 1 FROM player_data WHERE uuid=? AND " + serverGuard + ")", + "UPDATE " + Tables.modPlayerData() + " SET data_value=? WHERE uuid=? AND mod_id=? AND EXISTS (SELECT 1 FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard + ")", data, uuid, modId, uuid, serverId}); batch.add(new Object[]{ - "INSERT IGNORE INTO mod_player_data (uuid, mod_id, data_value) SELECT ?, ?, ? FROM player_data WHERE uuid=? AND " + serverGuard, + "INSERT IGNORE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) SELECT ?, ?, ? FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard, uuid, modId, data, uuid, serverId}); } @@ -1551,7 +1631,7 @@ public class VanillaSync { heartbeatTickCounter = 0; executorService.submit(() -> { try { - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", + JDBCsetUp.executePreparedUpdate("UPDATE " + Tables.serverInfo() + " SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); } catch (SQLException e) { PlayerSync.LOGGER.error("Error updating server heartbeat", e); @@ -1672,8 +1752,13 @@ public class VanillaSync { return totalXp; } - @SubscribeEvent + // FIX COMPAT (C1): priority=LOW + skip canceled events defends against mods like + // Revive Me / Corail Tombstone / Hardcore Revival that cancel LivingDeathEvent at + // NORMAL/HIGH priority. At LOW we run after them, and the cancel check short-circuits + // the death-save so "fallen" players are not mistakenly treated as dead. + @SubscribeEvent(priority = net.neoforged.bus.api.EventPriority.LOW) public static void onPlayerDeath(LivingDeathEvent event) { + if (event.isCanceled()) return; if (!(event.getEntity() instanceof ServerPlayer player)) return; String puuid = player.getUUID().toString(); if (deadPlayerWhileLogging.contains(puuid)) return; @@ -1696,7 +1781,7 @@ public class VanillaSync { try { final PlayerDataSnapshot snapshot = snapshotPlayerData(player); final Map backpackSnapshots = ModsSupport.snapshotBackpackData(player); - final List ssUuids = ModsSupport.collectSSUuids(player); + final Map ssSnapshots = ModsSupport.snapshotSSData(ModsSupport.collectSSUuids(player)); final List rs2DiskUuids; final ServerLevel rs2Level; final HolderLookup.Provider rs2Registry; @@ -1717,7 +1802,7 @@ public class VanillaSync { try { writeSnapshotToDB(snapshot); ModsSupport.saveBackpackSnapshots(backpackSnapshots); - ModsSupport.saveSSByUuids(ssUuids); + ModsSupport.saveSSSnapshots(ssSnapshots); if (!rs2DiskUuids.isEmpty() && rs2Level != null) { ModsSupport.saveRS2DisksByLevel(rs2DiskUuids, rs2Level, rs2Registry); } diff --git a/src/main/java/vip/fubuki/playersync/sync/addons/CuriosCache.java b/src/main/java/vip/fubuki/playersync/sync/addons/CuriosCache.java index 58f1efc..3954dd3 100644 --- a/src/main/java/vip/fubuki/playersync/sync/addons/CuriosCache.java +++ b/src/main/java/vip/fubuki/playersync/sync/addons/CuriosCache.java @@ -74,8 +74,18 @@ public class CuriosCache { for (int i = 0; i < dynStacks.getSlots(); i++) { ItemStack stack = dynStacks.getStackInSlot(i); if (!stack.isEmpty()) { - String serialized = VanillaSync.getNbtForStorage(stack); - flatMap.put(slotType + ":" + i, serialized); + flatMap.put(slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); + } + } + // FIX A2: capture cosmetic stacks in the death cache, matching the + // snapshot/apply format ("cos:slotType:index"). Without this, a player + // who died with a cosmetic curio would lose it on rejoin because the + // apply path clears cosmetic slots unconditionally. + IDynamicStackHandler cosStacks = stacksHandler.getCosmeticStacks(); + for (int i = 0; i < cosStacks.getSlots(); i++) { + ItemStack stack = cosStacks.getStackInSlot(i); + if (!stack.isEmpty()) { + flatMap.put("cos:" + slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); } } }); diff --git a/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java b/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java index 4b1d93a..d0a4c36 100644 --- a/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java +++ b/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java @@ -8,6 +8,7 @@ import vip.fubuki.playersync.PlayerSync; import vip.fubuki.playersync.sync.VanillaSync; import vip.fubuki.playersync.util.JDBCsetUp; import vip.fubuki.playersync.util.LocalJsonUtil; +import vip.fubuki.playersync.util.Tables; import java.sql.ResultSet; import java.sql.SQLException; @@ -22,6 +23,29 @@ import java.util.Map; */ public class ModCompatSync { + // FIX PERF (C4): Cache reflection Method lookups for NeoForge AttachmentHolder. + // Previously resolved on every snapshot/apply (35 players × auto-save = thousands of + // reflective lookups / hour). Static-init once, reuse forever. + private static final java.lang.reflect.Method SERIALIZE_ATTACHMENTS; + private static final java.lang.reflect.Method DESERIALIZE_ATTACHMENTS; + static { + java.lang.reflect.Method ser = null, des = null; + try { + ser = net.neoforged.neoforge.attachment.AttachmentHolder.class + .getDeclaredMethod("serializeAttachments", net.minecraft.core.HolderLookup.Provider.class); + ser.setAccessible(true); + des = net.neoforged.neoforge.attachment.AttachmentHolder.class + .getDeclaredMethod("deserializeAttachments", + net.minecraft.core.HolderLookup.Provider.class, + net.minecraft.nbt.CompoundTag.class); + des.setAccessible(true); + } catch (NoSuchMethodException e) { + PlayerSync.LOGGER.error("[PlayerSync] Could not cache AttachmentHolder reflection methods; NeoForge attachment sync will be disabled.", e); + } + SERIALIZE_ATTACHMENTS = ser; + DESERIALIZE_ATTACHMENTS = des; + } + // ============================ // Accessories API (Aether slots) // ============================ @@ -58,7 +82,7 @@ public class ModCompatSync { String serializedData = flatMap.toString(); JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", player.getUUID().toString(), "accessories", serializedData); PlayerSync.LOGGER.debug("Saved Accessories data for player {}", player.getUUID()); @@ -84,7 +108,7 @@ public class ModCompatSync { String accessoriesData; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player.getUUID().toString(), "accessories")) { ResultSet rs = qr.resultSet(); if (!rs.next()) { @@ -232,7 +256,7 @@ public class ModCompatSync { String serializedData = flatMap.toString(); JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", player.getUUID().toString(), "cosmeticarmor", serializedData); PlayerSync.LOGGER.debug("Saved CosmeticArmor data for player {}", player.getUUID()); @@ -257,7 +281,7 @@ public class ModCompatSync { String cosmeticData; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player.getUUID().toString(), "cosmeticarmor")) { ResultSet rs = qr.resultSet(); if (!rs.next()) { @@ -364,19 +388,15 @@ public class ModCompatSync { public static void storeNeoForgeAttachments(Player player) { try { if (!(player instanceof net.minecraft.server.level.ServerPlayer serverPlayer)) return; + if (SERIALIZE_ATTACHMENTS == null) return; - // FIX: Use serializeAttachments(Provider) directly instead of saveWithoutId() - // This is the exact method NeoForge uses to save attachments, no full player save needed - java.lang.reflect.Method serializeMethod = net.neoforged.neoforge.attachment.AttachmentHolder.class - .getDeclaredMethod("serializeAttachments", net.minecraft.core.HolderLookup.Provider.class); - serializeMethod.setAccessible(true); net.minecraft.nbt.CompoundTag attachments = (net.minecraft.nbt.CompoundTag) - serializeMethod.invoke(player, serverPlayer.getServer().registryAccess()); + SERIALIZE_ATTACHMENTS.invoke(player, serverPlayer.getServer().registryAccess()); if (attachments != null && !attachments.isEmpty()) { String serialized = VanillaSync.serializeTagToBinaryBase64(attachments); JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", player.getUUID().toString(), "neoforge_attachments", serialized); PlayerSync.LOGGER.debug("Saved NeoForge attachments for player {} ({} keys)", player.getUUID(), attachments.getAllKeys().size()); @@ -398,10 +418,11 @@ public class ModCompatSync { public static void restoreNeoForgeAttachments(Player player) { try { if (!(player instanceof net.minecraft.server.level.ServerPlayer serverPlayer)) return; + if (DESERIALIZE_ATTACHMENTS == null) return; String serialized; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT data_value FROM mod_player_data WHERE uuid=? AND mod_id=?", + "SELECT data_value FROM " + Tables.modPlayerData() + " WHERE uuid=? AND mod_id=?", player.getUUID().toString(), "neoforge_attachments")) { ResultSet rs = qr.resultSet(); if (!rs.next()) return; @@ -413,17 +434,10 @@ public class ModCompatSync { net.minecraft.nbt.CompoundTag attachments = VanillaSync.deserializeBinaryBase64Tag(serialized); if (attachments.isEmpty()) return; - // FIX: Correct method signature is (HolderLookup.Provider, CompoundTag), not (CompoundTag) - // The wrapper must contain the "neoforge:attachments" key for the method to find the data net.minecraft.nbt.CompoundTag wrapper = new net.minecraft.nbt.CompoundTag(); wrapper.put("neoforge:attachments", attachments); - java.lang.reflect.Method deserializeMethod = net.neoforged.neoforge.attachment.AttachmentHolder.class - .getDeclaredMethod("deserializeAttachments", - net.minecraft.core.HolderLookup.Provider.class, - net.minecraft.nbt.CompoundTag.class); - deserializeMethod.setAccessible(true); - deserializeMethod.invoke(player, serverPlayer.getServer().registryAccess(), wrapper); + DESERIALIZE_ATTACHMENTS.invoke(player, serverPlayer.getServer().registryAccess(), wrapper); PlayerSync.LOGGER.info("Restored NeoForge attachments for player {} ({} keys)", player.getUUID(), attachments.getAllKeys().size()); @@ -437,6 +451,7 @@ public class ModCompatSync { */ public static void applyAttachmentsFromData(Player player, String serialized) { if (serialized == null || !serialized.startsWith("BNBT:")) return; + if (DESERIALIZE_ATTACHMENTS == null) return; try { if (!(player instanceof net.minecraft.server.level.ServerPlayer serverPlayer)) return; @@ -446,12 +461,7 @@ public class ModCompatSync { net.minecraft.nbt.CompoundTag wrapper = new net.minecraft.nbt.CompoundTag(); wrapper.put("neoforge:attachments", attachments); - java.lang.reflect.Method deserializeMethod = net.neoforged.neoforge.attachment.AttachmentHolder.class - .getDeclaredMethod("deserializeAttachments", - net.minecraft.core.HolderLookup.Provider.class, - net.minecraft.nbt.CompoundTag.class); - deserializeMethod.setAccessible(true); - deserializeMethod.invoke(player, serverPlayer.getServer().registryAccess(), wrapper); + DESERIALIZE_ATTACHMENTS.invoke(player, serverPlayer.getServer().registryAccess(), wrapper); PlayerSync.LOGGER.info("Applied NeoForge attachments for player {} ({} keys)", player.getUUID(), attachments.getAllKeys().size()); @@ -475,6 +485,9 @@ public class ModCompatSync { try { io.wispforest.accessories.api.AccessoriesCapability cap = io.wispforest.accessories.api.AccessoriesCapability.get(player); + // FIX ANTI-LOSS (A2): cap==null means the capability isn't attached yet — + // return null to SKIP write and preserve DB. Do NOT return "{}" here, as that + // would wipe a legitimate accessories record. if (cap == null) return null; Map flatMap = new HashMap<>(); for (Map.Entry entry : cap.getContainers().entrySet()) { @@ -487,9 +500,7 @@ public class ModCompatSync { } } } - // FIX ANTI-DUPLICATION: Return "{}" for empty slots, NOT null. - // Null causes writeModSnapshot to SKIP the write, keeping stale data in DB. - // "{}" is written to DB, and on restore applyAccessoriesFromData clears slots. + // Cap read OK — "{}" is intentional for truly empty slots so apply clears stale .dat. return flatMap.toString(); } catch (Exception e) { PlayerSync.LOGGER.error("Error snapshotting Accessories for player {}", player.getUUID(), e); @@ -506,6 +517,7 @@ public class ModCompatSync { try { lain.mods.cos.impl.inventory.InventoryCosArmor cosInv = lain.mods.cos.impl.ModObjects.invMan.getCosArmorInventory(player.getUUID()); + // FIX ANTI-LOSS (A2): null manager → cannot read → SKIP write, preserve DB. if (cosInv == null) return null; Map flatMap = new HashMap<>(); for (int i = 0; i < cosInv.getContainerSize(); i++) { @@ -514,9 +526,7 @@ public class ModCompatSync { flatMap.put(i, VanillaSync.getNbtForStorage(stack)); } } - // FIX ANTI-DUPLICATION: Return "{}" for empty slots, NOT null. - // Null causes writeModSnapshot to SKIP the write, keeping stale data in DB. - // "{}" is written to DB, and on restore applyCosmeticArmorFromData clears slots. + // Read OK — "{}" for truly empty slots so apply clears stale .dat. return flatMap.toString(); } catch (Exception e) { PlayerSync.LOGGER.error("Error snapshotting CosmeticArmor for player {}", player.getUUID(), e); @@ -529,13 +539,11 @@ public class ModCompatSync { * Returns BNBT-serialized string or null if no data. */ public static String snapshotAttachments(Player player) { + if (SERIALIZE_ATTACHMENTS == null) return null; try { if (!(player instanceof net.minecraft.server.level.ServerPlayer serverPlayer)) return null; - java.lang.reflect.Method serializeMethod = net.neoforged.neoforge.attachment.AttachmentHolder.class - .getDeclaredMethod("serializeAttachments", net.minecraft.core.HolderLookup.Provider.class); - serializeMethod.setAccessible(true); net.minecraft.nbt.CompoundTag attachments = (net.minecraft.nbt.CompoundTag) - serializeMethod.invoke(player, serverPlayer.getServer().registryAccess()); + SERIALIZE_ATTACHMENTS.invoke(player, serverPlayer.getServer().registryAccess()); if (attachments == null || attachments.isEmpty()) return null; return VanillaSync.serializeTagToBinaryBase64(attachments); } catch (Exception e) { @@ -575,17 +583,17 @@ public class ModCompatSync { public static void writeModSnapshot(String uuid, String accessoriesData, String cosmeticArmor, String attachments) throws SQLException { if (accessoriesData != null) { JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", uuid, "accessories", accessoriesData); } if (cosmeticArmor != null) { JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", uuid, "cosmeticarmor", cosmeticArmor); } if (attachments != null) { JDBCsetUp.executePreparedUpdate( - "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + "REPLACE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) VALUES (?, ?, ?)", uuid, "neoforge_attachments", attachments); } } @@ -595,11 +603,11 @@ public class ModCompatSync { String serverGuard = "(last_server=? OR last_server IS NULL)"; // Update existing row only if this server still owns the player JDBCsetUp.executePreparedUpdate( - "UPDATE mod_player_data SET data_value=? WHERE uuid=? AND mod_id=? AND EXISTS (SELECT 1 FROM player_data WHERE uuid=? AND " + serverGuard + ")", + "UPDATE " + Tables.modPlayerData() + " SET data_value=? WHERE uuid=? AND mod_id=? AND EXISTS (SELECT 1 FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard + ")", data, uuid, modId, uuid, serverId); // Insert if row doesn't exist yet (first save) JDBCsetUp.executePreparedUpdate( - "INSERT IGNORE INTO mod_player_data (uuid, mod_id, data_value) SELECT ?, ?, ? FROM player_data WHERE uuid=? AND " + serverGuard, + "INSERT IGNORE INTO " + Tables.modPlayerData() + " (uuid, mod_id, data_value) SELECT ?, ?, ? FROM " + Tables.playerData() + " WHERE uuid=? AND " + serverGuard, uuid, modId, data, uuid, serverId); } diff --git a/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java b/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java index 656fb4f..00fa286 100644 --- a/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java +++ b/src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java @@ -17,6 +17,7 @@ import vip.fubuki.playersync.PlayerSync; import vip.fubuki.playersync.sync.VanillaSync; import vip.fubuki.playersync.util.JDBCsetUp; import vip.fubuki.playersync.util.LocalJsonUtil; +import vip.fubuki.playersync.util.Tables; import java.io.IOException; import java.sql.ResultSet; @@ -54,7 +55,15 @@ public class ModsSupport { if (uuidOpt.isPresent()) { UUID contentsUuid = uuidOpt.get(); restoreStorageContents(contentsUuid, (nbt) -> { - net.p3pp3rf1y.sophisticatedbackpacks.backpack.BackpackStorage.get().setBackpackContents(contentsUuid, nbt); + // ROOT CAUSE FIX — BackpackStorage.setBackpackContents() upstream is a + // shallow MERGE, not a replace, when the UUID already exists. On any + // server that previously loaded this backpack (re-join, multi-world, + // .dat persisted), old sub-tags survive the "restore" → duplication. + // Removing first guarantees a clean replace. + net.p3pp3rf1y.sophisticatedbackpacks.backpack.BackpackStorage store = + net.p3pp3rf1y.sophisticatedbackpacks.backpack.BackpackStorage.get(); + try { store.removeBackpackContents(contentsUuid); } catch (Throwable ignored) {} + store.setBackpackContents(contentsUuid, nbt); PlayerSync.LOGGER.info("Restored backpack data for UUID {}", contentsUuid); }); } @@ -67,7 +76,7 @@ public class ModsSupport { */ private static void restoreStorageContents(UUID contentsUuid, StorageRestoreCallback callback) { try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT backpack_nbt FROM backpack_data WHERE uuid=?", contentsUuid.toString())) { + "SELECT backpack_nbt FROM " + Tables.backpackData() + " WHERE uuid=?", contentsUuid.toString())) { ResultSet rs = qr.resultSet(); if (rs.next()) { String serialized = rs.getString("backpack_nbt"); @@ -120,7 +129,7 @@ public class ModsSupport { // containers, causing item duplication on the next login. if (nbt == null || nbt.isEmpty()) { try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT LENGTH(backpack_nbt) AS len FROM backpack_data WHERE uuid=?", contentsUuid.toString())) { + "SELECT LENGTH(backpack_nbt) AS len FROM " + Tables.backpackData() + " WHERE uuid=?", contentsUuid.toString())) { java.sql.ResultSet rs = qr.resultSet(); if (rs.next() && rs.getInt("len") > 50) { PlayerSync.LOGGER.debug("Skipping save of empty NBT for UUID {} - DB has {} bytes of real data", @@ -132,8 +141,15 @@ public class ModsSupport { String serialized = VanillaSync.serializeTagToBinaryBase64(nbt); try { + // FIX INTEGRITY (E): REPLACE INTO silently overwrote backpack rows even when + // another server had already claimed the owning player. We cannot easily + // add a last_server guard to backpack_data directly (it is keyed by + // storage UUID, not player UUID — no link to player_data). So we keep the + // REPLACE here but expect upper layers (`saveBackpackSnapshots`) to be called + // only after the player_data transaction commit has run under the last_server + // guard, which is the case in writeSnapshotToDB's caller chain. JDBCsetUp.executePreparedUpdate( - "REPLACE INTO backpack_data (uuid, backpack_nbt) VALUES (?, ?)", + "REPLACE INTO " + Tables.backpackData() + " (uuid, backpack_nbt) VALUES (?, ?)", contentsUuid.toString(), serialized); } catch (SQLException e) { PlayerSync.LOGGER.error("Error saving storage data for UUID {}", contentsUuid, e); @@ -156,7 +172,7 @@ public class ModsSupport { String curiosData; try (JDBCsetUp.QueryResult qr = JDBCsetUp.executePreparedQuery( - "SELECT curios_item FROM curios WHERE uuid=?", player.getUUID().toString())) { + "SELECT curios_item FROM " + Tables.curios() + " WHERE uuid=?", player.getUUID().toString())) { ResultSet rs = qr.resultSet(); if (!rs.next()) { // No stored data; perform an initial save. @@ -168,13 +184,17 @@ public class ModsSupport { ICuriosItemHandler handler = handlerOpt.get(); - // FIX ANTI-DUPLICATION: ALWAYS clear curios slots first to wipe stale data - // loaded from Minecraft's .dat file, then only restore if DB has valid data. + // FIX A2/A3: clear BOTH functional and cosmetic slots first to wipe stale .dat + // data, then restore from DB if valid. handler.getCurios().forEach((slotType, stacksHandler) -> { IDynamicStackHandler dynStacks = stacksHandler.getStacks(); for (int i = 0; i < dynStacks.getSlots(); i++) { dynStacks.setStackInSlot(i, ItemStack.EMPTY); } + IDynamicStackHandler cos = stacksHandler.getCosmeticStacks(); + for (int i = 0; i < cos.getSlots(); i++) { + cos.setStackInSlot(i, ItemStack.EMPTY); + } }); if (curiosData == null || curiosData.length() <= 2) { @@ -188,16 +208,19 @@ public class ModsSupport { return; } - // Restore each saved item + // Restore each saved item. Support both new "cos:slotType:index" cosmetic keys + // and legacy "slotType:index" functional-only keys. for (Map.Entry entry : storedMap.entrySet()) { String compositeKey = entry.getKey(); - int lastColon = compositeKey.lastIndexOf(':'); + boolean cosmetic = compositeKey.startsWith("cos:"); + String remaining = cosmetic ? compositeKey.substring(4) : compositeKey; + int lastColon = remaining.lastIndexOf(':'); if (lastColon < 0) continue; - String slotType = compositeKey.substring(0, lastColon); + String slotType = remaining.substring(0, lastColon); int slotIndex; try { - slotIndex = Integer.parseInt(compositeKey.substring(lastColon + 1)); + slotIndex = Integer.parseInt(remaining.substring(lastColon + 1)); } catch (NumberFormatException ex) { continue; } @@ -207,7 +230,9 @@ public class ModsSupport { ItemStack stack = VanillaSync.deserializeAndCreatePlaceholderIfNeeded(serialized); if (handler.getCurios().containsKey(slotType)) { ICurioStacksHandler stacksHandler = handler.getCurios().get(slotType); - IDynamicStackHandler dynStacks = stacksHandler.getStacks(); + IDynamicStackHandler dynStacks = cosmetic + ? stacksHandler.getCosmeticStacks() + : stacksHandler.getStacks(); if (slotIndex < dynStacks.getSlots()) { dynStacks.setStackInSlot(slotIndex, stack); } @@ -244,7 +269,7 @@ public class ModsSupport { // Use cached data from death event PlayerSync.LOGGER.info("Using cached curios data for dead player {}", playerUuid); JDBCsetUp.executePreparedUpdate( - "REPLACE INTO curios (uuid, curios_item) VALUES (?, ?)", + "REPLACE INTO " + Tables.curios() + " (uuid, curios_item) VALUES (?, ?)", playerUuid.toString(), cached.serializedData); CuriosCache.curiosCache.remove(playerUuid); } else { @@ -260,17 +285,36 @@ public class ModsSupport { public static String snapshotCuriosData(Player player) { if (!ModList.get().isLoaded("curios")) return null; Optional handlerOpt = CuriosApi.getCuriosInventory(player); + // FIX ANTI-LOSS (A2): if the handler could not be resolved (capability not yet + // attached, or Curios mod issue), return null so writeSnapshotToDB SKIPS the write + // and preserves whatever data is already in DB. Returning "{}" here would overwrite + // a legitimate curios record with an empty one and destroy the player's items. + if (handlerOpt.isEmpty()) { + PlayerSync.LOGGER.warn("Curios handler unavailable while snapshotting {} — skipping curios write", player.getUUID()); + return null; + } Map flatMap = new HashMap<>(); - handlerOpt.ifPresent(handler -> { - handler.getCurios().forEach((slotType, stacksHandler) -> { - IDynamicStackHandler dynStacks = stacksHandler.getStacks(); - for (int i = 0; i < dynStacks.getSlots(); i++) { - ItemStack stack = dynStacks.getStackInSlot(i); - if (!stack.isEmpty()) { - flatMap.put(slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); - } + ICuriosItemHandler handler = handlerOpt.get(); + // FIX DATA-LOSS (A2): sync BOTH functional stacks and cosmetic stacks. The prior + // implementation only captured getStacks() → every cosmetic item equipped in a + // Curios cosmetic slot was silently wiped across server transfers. Cosmetic slots + // are identified by the "cos:" prefix in the composite key so apply/clear can + // distinguish them without a schema change. + handler.getCurios().forEach((slotType, stacksHandler) -> { + IDynamicStackHandler dynStacks = stacksHandler.getStacks(); + for (int i = 0; i < dynStacks.getSlots(); i++) { + ItemStack stack = dynStacks.getStackInSlot(i); + if (!stack.isEmpty()) { + flatMap.put(slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); } - }); + } + IDynamicStackHandler cosStacks = stacksHandler.getCosmeticStacks(); + for (int i = 0; i < cosStacks.getSlots(); i++) { + ItemStack stack = cosStacks.getStackInSlot(i); + if (!stack.isEmpty()) { + flatMap.put("cos:" + slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); + } + } }); return flatMap.toString(); } @@ -290,14 +334,19 @@ public class ModsSupport { ICuriosItemHandler handler = handlerOpt.get(); - // FIX ANTI-DUPLICATION: ALWAYS clear curios slots first, even when DB data is - // empty. Without this, stale curios loaded from Minecraft's .dat file (world save) - // persist when the DB has no curios data — causing item duplication across servers. + // FIX ANTI-DUPLICATION (A2+A3): clear BOTH functional and cosmetic stacks first, + // even when DB data is empty. Without this, stale curios loaded from the .dat + // persist when the DB has no entry → dup across servers. Cosmetic stacks also + // needed clearing or cosmetic-dup persisted asymmetrically. for (Map.Entry entry : handler.getCurios().entrySet()) { IDynamicStackHandler stacks = entry.getValue().getStacks(); for (int i = 0; i < stacks.getSlots(); i++) { stacks.setStackInSlot(i, ItemStack.EMPTY); } + IDynamicStackHandler cos = entry.getValue().getCosmeticStacks(); + for (int i = 0; i < cos.getSlots(); i++) { + cos.setStackInSlot(i, ItemStack.EMPTY); + } } // If no data to restore, we're done (slots already cleared above) @@ -306,27 +355,32 @@ public class ModsSupport { Map storedMap = LocalJsonUtil.StringToMap(curiosData); if (storedMap.isEmpty()) return; - // Restore items from pre-read data + // Restore items from pre-read data. Cosmetic slots use the "cos:slotType:index" + // composite key; functional slots use "slotType:index". for (Map.Entry entry : storedMap.entrySet()) { String compositeKey = entry.getKey(); - int lastColon = compositeKey.lastIndexOf(':'); + boolean cosmetic = compositeKey.startsWith("cos:"); + String remaining = cosmetic ? compositeKey.substring(4) : compositeKey; + int lastColon = remaining.lastIndexOf(':'); if (lastColon < 0) continue; - String slotType = compositeKey.substring(0, lastColon); + String slotType = remaining.substring(0, lastColon); int slotIndex; - try { slotIndex = Integer.parseInt(compositeKey.substring(lastColon + 1)); } + try { slotIndex = Integer.parseInt(remaining.substring(lastColon + 1)); } catch (NumberFormatException e) { continue; } try { ItemStack stack = VanillaSync.deserializeAndCreatePlaceholderIfNeeded(entry.getValue()); ICurioStacksHandler stacksHandler = handler.getCurios().get(slotType); if (stacksHandler != null) { - IDynamicStackHandler stacks = stacksHandler.getStacks(); + IDynamicStackHandler stacks = cosmetic + ? stacksHandler.getCosmeticStacks() + : stacksHandler.getStacks(); if (slotIndex < stacks.getSlots()) { stacks.setStackInSlot(slotIndex, stack); } } } catch (Exception e) { - PlayerSync.LOGGER.error("Error applying curios slot {}:{}", slotType, slotIndex, e); + PlayerSync.LOGGER.error("Error applying curios slot {} ({}:{})", compositeKey, slotType, slotIndex, e); } } PlayerSync.LOGGER.info("Applied curios data for player {} from pre-read data", player.getUUID()); @@ -344,8 +398,15 @@ public class ModsSupport { for (int i = 0; i < dynStacks.getSlots(); i++) { ItemStack stack = dynStacks.getStackInSlot(i); if (!stack.isEmpty()) { - String serialized = VanillaSync.getNbtForStorage(stack); - flatMap.put(slotType + ":" + i, serialized); + flatMap.put(slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); + } + } + // FIX A2: cosmetic stacks must be captured symmetrically with snapshotCuriosData. + IDynamicStackHandler cosStacks = stacksHandler.getCosmeticStacks(); + for (int i = 0; i < cosStacks.getSlots(); i++) { + ItemStack stack = cosStacks.getStackInSlot(i); + if (!stack.isEmpty()) { + flatMap.put("cos:" + slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); } } }); @@ -356,7 +417,7 @@ public class ModsSupport { // FIX: Use REPLACE INTO instead of separate INSERT/UPDATE to prevent silent // no-ops when the row doesn't exist yet (e.g. new player who died before first save) JDBCsetUp.executePreparedUpdate( - "REPLACE INTO curios (uuid, curios_item) VALUES (?, ?)", + "REPLACE INTO " + Tables.curios() + " (uuid, curios_item) VALUES (?, ?)", player.getUUID().toString(), serializedData); } @@ -374,13 +435,17 @@ public class ModsSupport { if (uuidOpt.isPresent()) { UUID contentsUuid = uuidOpt.get(); - // FIX: Read the full contents NBT from the wrapper's in-memory state, - // not from BackpackStorage which may have stale data if the wrapper - // hasn't flushed recent changes (e.g. upgrade modifications). - // refreshInventoryForInputOutput triggers an internal save to BackpackStorage. + // FIX: Read the full contents NBT from the wrapper's in-memory state. + // NOTE: despite earlier comments, refreshInventoryForInputOutput() does + // NOT actively flush to BackpackStorage — it resets the IO handler cache + // and runs the change callbacks. The live CompoundTag in BackpackStorage + // is already kept up to date by handler writes, so reading it next is safe. try { backpackWrapper.refreshInventoryForInputOutput(); - } catch (Exception ignored) {} + } catch (Exception e) { + PlayerSync.LOGGER.warn("refreshInventoryForInputOutput failed for backpack {} of player {} — saved NBT may be slightly stale", + contentsUuid, player.getUUID(), e); + } CompoundTag backpackNbt = net.p3pp3rf1y.sophisticatedbackpacks.backpack.BackpackStorage.get().getOrCreateBackpackContents(contentsUuid); saveStorageContents(contentsUuid, backpackNbt); @@ -638,9 +703,54 @@ public class ModsSupport { } /** - * Saves Sophisticated Storage contents by UUID. Reads SavedData and writes to DB. - * Can be called from a background thread (no entity access). + * FIX THREAD-SAFETY (C3): Captures Sophisticated Storage CompoundTags on the MAIN + * thread by copying the SavedData entries. Previously {@link #saveSSByUuids(List)} + * read {@code ItemContentsStorage} directly from a background thread, racing with + * main-thread modifications (non-thread-safe HashMap) and risking torn reads → dup. + * + *

Callers should invoke this on the main thread, then pass the returned map to + * {@link #saveSSSnapshots(Map)} on a background thread. */ + public static Map snapshotSSData(List uuids) { + Map out = new HashMap<>(); + if (uuids == null || uuids.isEmpty() || !ModList.get().isLoaded("sophisticatedstorage")) return out; + try { + net.p3pp3rf1y.sophisticatedstorage.block.ItemContentsStorage store = + net.p3pp3rf1y.sophisticatedstorage.block.ItemContentsStorage.get(); + for (UUID uuid : uuids) { + try { + CompoundTag live = store.getOrCreateStorageContents(uuid); + if (live != null && !live.isEmpty()) { + out.put(uuid, live.copy()); + } + } catch (Exception e) { + PlayerSync.LOGGER.error("Error snapshotting SS contents for UUID {}", uuid, e); + } + } + } catch (Exception e) { + PlayerSync.LOGGER.error("Error reading ItemContentsStorage for snapshot", e); + } + return out; + } + + /** Background-thread writer for the frozen snapshot produced by {@link #snapshotSSData(List)}. */ + public static void saveSSSnapshots(Map snapshots) { + if (snapshots == null || snapshots.isEmpty()) return; + for (Map.Entry e : snapshots.entrySet()) { + try { + saveStorageContents(e.getKey(), e.getValue()); + } catch (Exception ex) { + PlayerSync.LOGGER.error("Error saving SS snapshot for UUID {}", e.getKey(), ex); + } + } + } + + /** + * @deprecated unsafe — reads ItemContentsStorage from the calling thread (possibly + * background), racing with main-thread modifications. Use {@link #snapshotSSData(List)} + * on main thread followed by {@link #saveSSSnapshots(Map)} on background thread. + */ + @Deprecated public static void saveSSByUuids(List uuids) { for (UUID uuid : uuids) { try { diff --git a/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncClient.java b/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncClient.java deleted file mode 100644 index cb76a6b..0000000 --- a/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncClient.java +++ /dev/null @@ -1,134 +0,0 @@ -package vip.fubuki.playersync.sync.chat; - -import net.minecraft.network.chat.Component; -import net.minecraft.server.players.PlayerList; -import net.neoforged.bus.api.SubscribeEvent; -import net.neoforged.neoforge.event.ServerChatEvent; -import net.neoforged.neoforge.event.entity.player.PlayerEvent; -import vip.fubuki.playersync.PlayerSync; -import vip.fubuki.playersync.config.JdbcConfig; - -import java.io.*; -import java.net.ConnectException; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.net.SocketTimeoutException; -import java.util.Objects; - -public class ChatSyncClient { - static PlayerList playerList; - static Socket clientSocket; - static PrintWriter out; - - private static volatile boolean running = true; - private static final int RECONNECT_DELAY = 5000; - private static final int MAX_RECONNECT_ATTEMPTS = 10; - - public void run() { - int reconnectAttempts = 0; - - while (running && reconnectAttempts < MAX_RECONNECT_ATTEMPTS) { - try { - PlayerSync.LOGGER.info("Connecting to chat server {}:{}", - JdbcConfig.CHAT_SERVER_IP.get(), - JdbcConfig.CHAT_SERVER_PORT.get()); - - clientSocket = new Socket(); - clientSocket.setReuseAddress(true); - clientSocket.setKeepAlive(true); - clientSocket.setTcpNoDelay(true); - - clientSocket.connect( - new InetSocketAddress( - JdbcConfig.CHAT_SERVER_IP.get(), - JdbcConfig.CHAT_SERVER_PORT.get() - ), - 15000 - ); - - clientSocket.setSoTimeout(0); - - out = new PrintWriter(new BufferedWriter( - new OutputStreamWriter(clientSocket.getOutputStream())), true); - - PlayerSync.LOGGER.info("Successfully connected to chat server"); - reconnectAttempts = 0; - - BufferedReader in = new BufferedReader( - new InputStreamReader(clientSocket.getInputStream())); - - String serverMessage; - while (running && (serverMessage = in.readLine()) != null) { - Component textComponents = Component.nullToEmpty(serverMessage); - if(playerList != null){ - playerList.getServer().execute(() -> - playerList.broadcastSystemMessage(textComponents, false)); - }else { - PlayerSync.LOGGER.info("Received message from chat server: " + serverMessage); - } - } - - } catch (SocketTimeoutException e) { - PlayerSync.LOGGER.warn("Chat server read timeout, reconnecting..."); - } catch (ConnectException e) { - PlayerSync.LOGGER.warn("Cannot connect to chat server: {}", e.getMessage()); - } catch (IOException e) { - PlayerSync.LOGGER.error("Chat client connection error: {}", e.getMessage()); - } finally { - closeConnection(); - } - - if (running && reconnectAttempts < MAX_RECONNECT_ATTEMPTS) { - reconnectAttempts++; - PlayerSync.LOGGER.warn("Attempting to reconnect to chat server ({}/{})", - reconnectAttempts, MAX_RECONNECT_ATTEMPTS); - - try { - long delay = Math.min(RECONNECT_DELAY * (long)Math.pow(2, reconnectAttempts-1), 60000); - Thread.sleep(delay); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - break; - } - } - } - } - - private void closeConnection() { - try { - if (out != null) { - out.close(); - out = null; - } - if (clientSocket != null && !clientSocket.isClosed()) { - clientSocket.close(); - clientSocket = null; - } - } catch (IOException e) { - PlayerSync.LOGGER.error("Error closing connection: {}", e.getMessage()); - } - } - - public void shutdown() { - running = false; - closeConnection(); - } - - @SubscribeEvent - public static void onPlayerChat(ServerChatEvent event) { - String message= "<"+event.getUsername()+"> "+event.getMessage().getString(); - if (out != null) { - out.println(message); - } - } - - @SubscribeEvent - public static void onPlayerJoin(PlayerEvent.PlayerLoggedInEvent event){ - playerList = Objects.requireNonNull(event.getEntity().getServer()).getPlayerList(); - } - - @SubscribeEvent - public static void onPlayerLeave(PlayerEvent.PlayerLoggedOutEvent event){ - playerList = Objects.requireNonNull(event.getEntity().getServer()).getPlayerList(); - } -} diff --git a/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncServer.java b/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncServer.java deleted file mode 100644 index 8f3326d..0000000 --- a/src/main/java/vip/fubuki/playersync/sync/chat/ChatSyncServer.java +++ /dev/null @@ -1,130 +0,0 @@ -package vip.fubuki.playersync.sync.chat; - -import vip.fubuki.playersync.PlayerSync; -import vip.fubuki.playersync.config.JdbcConfig; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.net.ServerSocket; -import java.net.Socket; -import java.net.SocketTimeoutException; -import java.util.Iterator; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; - -public class ChatSyncServer { - static ServerSocket serverSocket; - static final Set SocketList = ConcurrentHashMap.newKeySet(); - static final ExecutorService executorService = Executors.newCachedThreadPool(); - private volatile boolean running = true; - - public void run() throws IOException { - try { - serverSocket = new ServerSocket(JdbcConfig.CHAT_SERVER_PORT.get()); - serverSocket.setReuseAddress(true); - PlayerSync.LOGGER.info("Chat server started successfully on port {}", JdbcConfig.CHAT_SERVER_PORT.get()); - - while (running && !Thread.currentThread().isInterrupted()) { - try { - Socket newSocket = serverSocket.accept(); - newSocket.setSoTimeout(0); - SocketList.add(newSocket); - executorService.submit(() -> handleClient(newSocket)); - PlayerSync.LOGGER.info("New client connected, total clients: {}", SocketList.size()); - } catch (IOException e) { - if (running) { - PlayerSync.LOGGER.error("Error accepting client connection: {}", e.getMessage()); - } - } - } - } finally { - shutdown(); - } - } - - private void handleClient(Socket socket) { - String clientInfo = socket.getInetAddress() + ":" + socket.getPort(); - - try (BufferedReader reader = new BufferedReader( - new InputStreamReader(socket.getInputStream()))) { - - String message; - while (running && (message = reader.readLine()) != null) { - broadcastMessage(socket, message); - } - - } catch (SocketTimeoutException e) { - PlayerSync.LOGGER.warn("Client {} timeout", clientInfo); - } catch (IOException e) { - PlayerSync.LOGGER.error("Error handling client {}: {}", clientInfo, e.getMessage()); - } finally { - SocketList.remove(socket); - try { - if (!socket.isClosed()) { - socket.close(); - } - } catch (IOException e) { - PlayerSync.LOGGER.error("Error closing client socket: {}", e.getMessage()); - } - PlayerSync.LOGGER.info("Client disconnected, remaining clients: {}", SocketList.size()); - } - } - - private void broadcastMessage(Socket sender, String message) { - Iterator iterator = SocketList.iterator(); - while (iterator.hasNext()) { - Socket socket = iterator.next(); - if (!socket.equals(sender) && !socket.isClosed()) { - try { - PrintWriter writer = new PrintWriter(socket.getOutputStream(), true); - writer.println(message); - } catch (IOException e) { - PlayerSync.LOGGER.error("Error broadcasting to client, removing: {}", e.getMessage()); - iterator.remove(); - try { - socket.close(); - } catch (IOException ex) { - // Ignore - } - } - } - } - } - - public void shutdown() { - running = false; - try { - if (serverSocket != null && !serverSocket.isClosed()) { - serverSocket.close(); - } - } catch (IOException e) { - PlayerSync.LOGGER.error("Error closing server socket: {}", e.getMessage()); - } - - for (Socket socket : SocketList) { - try { - if (!socket.isClosed()) { - socket.close(); - } - } catch (IOException e) { - // Ignore - } - } - SocketList.clear(); - - executorService.shutdown(); - try { - if (!executorService.awaitTermination(5, TimeUnit.SECONDS)) { - executorService.shutdownNow(); - } - } catch (InterruptedException e) { - executorService.shutdownNow(); - Thread.currentThread().interrupt(); - } - } -} diff --git a/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java b/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java index 0071e27..732602b 100644 --- a/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java +++ b/src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java @@ -43,23 +43,24 @@ public class JDBCsetUp { cfg.setUsername(JdbcConfig.USERNAME.get()); cfg.setPassword(JdbcConfig.PASSWORD.get()); - // FIX PERF: Increased pool for 35+ player servers. - // Old: 10 max / 2 idle → 35 concurrent saves queued on 10 connections → 250ms+ wait. - // New: 25 max / 4 idle → handles peak load without connection starvation. - cfg.setMaximumPoolSize(25); + // FIX PERF (C9): right-sized pool. 25 was oversized; empirical HikariCP rule is + // ~ cores*2 + spindles. 15 handles 35 concurrent players comfortably and reduces + // MySQL server-side context switching. + cfg.setMaximumPoolSize(15); cfg.setMinimumIdle(4); // Connection lifecycle - cfg.setConnectionTimeout(30_000L); // 30 s – how long to wait for a free slot - cfg.setIdleTimeout(600_000L); // 10 min – evict idle connections + cfg.setConnectionTimeout(10_000L); // 10 s – fail fast on MySQL outage + cfg.setIdleTimeout(300_000L); // 5 min – evict idle connections sooner cfg.setMaxLifetime(1_800_000L); // 30 min – recycle before MySQL wait_timeout cfg.setKeepaliveTime(300_000L); // 5 min – ping idle connections (NOT hot path) cfg.setAutoCommit(true); cfg.setPoolName("PlayerSync"); - // FIX PERF: Detect connection leaks (connections held > 10s without being returned) - cfg.setLeakDetectionThreshold(10000); + // FIX PERF (C9): 25s threshold — covers worst-case doPlayerJoin poll bursts without + // flooding logs with false positives. Previous 10s fired during legitimate 15-30s polls. + cfg.setLeakDetectionThreshold(25_000L); dataSource = new HikariDataSource(cfg); LOGGER.info("[PlayerSync] HikariCP pool ready (maxPool={}, minIdle={})", @@ -203,29 +204,38 @@ public class JDBCsetUp { * * Each entry is {sql, params...}. All execute in order within one transaction. * If any fails, the entire batch is rolled back. + * + * @return array of per-statement affected-row counts (parallel to {@code statements}). + * Callers can inspect the first entry to detect silent no-ops caused by + * {@code AND last_server=?} guards blocking a stale write. */ - public static void executeBatchTransaction(Object[]... statements) throws SQLException { + public static int[] executeBatchTransaction(Object[]... statements) throws SQLException { + int[] counts = new int[statements.length]; try (Connection conn = getConnection()) { conn.setAutoCommit(false); try { - for (Object[] entry : statements) { + for (int idx = 0; idx < statements.length; idx++) { + Object[] entry = statements[idx]; String sql = (String) entry[0]; LOGGER.trace(sql); try (PreparedStatement stmt = conn.prepareStatement(sql)) { for (int i = 1; i < entry.length; i++) { stmt.setObject(i, entry[i]); } - stmt.executeUpdate(); + counts[idx] = stmt.executeUpdate(); } } conn.commit(); } catch (SQLException e) { - try { conn.rollback(); } catch (SQLException ignored) {} + try { conn.rollback(); } catch (SQLException rbEx) { + LOGGER.error("[PlayerSync] Rollback failed while handling batch transaction error", rbEx); + } throw e; } finally { conn.setAutoCommit(true); } } + return counts; } public static QueryResult executePreparedQuery(String sql, Object... params) throws SQLException { diff --git a/src/main/java/vip/fubuki/playersync/util/SyncLogger.java b/src/main/java/vip/fubuki/playersync/util/SyncLogger.java index 2d3f876..911cda6 100644 --- a/src/main/java/vip/fubuki/playersync/util/SyncLogger.java +++ b/src/main/java/vip/fubuki/playersync/util/SyncLogger.java @@ -7,6 +7,9 @@ import java.nio.file.*; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -33,6 +36,16 @@ public class SyncLogger { private static final AtomicBoolean initialized = new AtomicBoolean(false); private static Path logPath; + // FIX PERF (C3): Dedicated daemon scheduler so log() never opens/closes the file on + // the caller thread. Previous impl called flushQueue() inline → every log call from + // the main thread opened a FileWriter, wrote, and closed synchronously. + private static final ScheduledExecutorService FLUSH_EXEC = Executors.newSingleThreadScheduledExecutor(r -> { + Thread t = new Thread(r, "PlayerSync-logflush"); + t.setDaemon(true); + t.setPriority(Thread.MIN_PRIORITY); + return t; + }); + // ------------------------------------------------------------------------- // Initialization // ------------------------------------------------------------------------- @@ -47,6 +60,8 @@ public class SyncLogger { writeRaw("=".repeat(80)); writeRaw("PlayerSync Log — Server ID: " + JdbcConfig.SERVER_ID.get() + " — Started: " + LocalDateTime.now().format(TIME_FMT)); writeRaw("=".repeat(80)); + // FIX PERF (C3): single background flush every 500ms — no file I/O on hot path. + FLUSH_EXEC.scheduleWithFixedDelay(SyncLogger::flushQueue, 500, 500, TimeUnit.MILLISECONDS); } catch (Exception e) { System.err.println("[PlayerSync] Failed to initialize SyncLogger: " + e.getMessage()); } @@ -153,8 +168,7 @@ public class SyncLogger { level, formatted); writeQueue.add(line); - // Flush async to avoid blocking caller - flushQueue(); + // FIX PERF (C3): no inline flush — background scheduler drains the queue. } catch (Exception ignored) {} } @@ -191,7 +205,6 @@ public class SyncLogger { private static void writeRaw(String line) { writeQueue.add(line); - flushQueue(); } private static void rotateIfNeeded() { @@ -215,8 +228,10 @@ public class SyncLogger { } catch (IOException ignored) {} } - /** Call on server shutdown to flush remaining entries */ + /** Call on server shutdown to flush remaining entries and stop the background writer. */ public static void shutdown() { + try { FLUSH_EXEC.shutdown(); } catch (Exception ignored) {} + try { FLUSH_EXEC.awaitTermination(2, TimeUnit.SECONDS); } catch (InterruptedException ignored) {} flushQueue(); } } diff --git a/src/main/java/vip/fubuki/playersync/util/Tables.java b/src/main/java/vip/fubuki/playersync/util/Tables.java new file mode 100644 index 0000000..a7b16d9 --- /dev/null +++ b/src/main/java/vip/fubuki/playersync/util/Tables.java @@ -0,0 +1,56 @@ +package vip.fubuki.playersync.util; + +import vip.fubuki.playersync.config.JdbcConfig; + +import java.util.regex.Pattern; + +/** + * Central source of truth for PlayerSync table names. + * + *

Reads the optional {@code table_prefix} config at every call so that + * administrators can safely share a single MySQL database with other mods + * without colliding on generic names such as {@code player_data} or + * {@code server_info}. The prefix defaults to an empty string to preserve + * backward compatibility with existing installations. + * + *

Only the table identifier is prefixed. The database schema + * qualifier (if any) must be added by the caller, e.g. via + * {@code "`" + dbName + "`." + Tables.playerData()}. + * + * @author vyrriox + */ +public final class Tables { + + private Tables() {} + + // FIX PERF: precompile the validation pattern and cache the validated prefix. + // String.matches() recompiles the regex on every call; this was invoked from + // every SQL statement the mod issues (heartbeat, auto-save, join, logout, ...). + // The config value cannot change at runtime, so a lazy singleton cache is safe. + private static final Pattern VALID_PREFIX = Pattern.compile("[A-Za-z0-9_]*"); + private static volatile String cachedPrefix; + private static volatile String cachedRaw; + + private static String prefix() { + String raw; + try { raw = JdbcConfig.TABLE_PREFIX.get(); } + catch (Exception e) { return ""; } + if (raw == null) raw = ""; + // Fast path: same raw value as last call → return cached validated prefix. + String lastRaw = cachedRaw; + if (lastRaw != null && lastRaw.equals(raw)) { + return cachedPrefix; + } + // Validate and cache. + String validated = VALID_PREFIX.matcher(raw).matches() ? raw : ""; + cachedPrefix = validated; + cachedRaw = raw; + return validated; + } + + public static String playerData() { return prefix() + "player_data"; } + public static String serverInfo() { return prefix() + "server_info"; } + public static String curios() { return prefix() + "curios"; } + public static String backpackData() { return prefix() + "backpack_data"; } + public static String modPlayerData() { return prefix() + "mod_player_data"; } +} diff --git a/src/main/resources/playersync.mixins.json b/src/main/resources/playersync.mixins.json index c5db030..6a723cb 100644 --- a/src/main/resources/playersync.mixins.json +++ b/src/main/resources/playersync.mixins.json @@ -3,16 +3,8 @@ "package": "vip.fubuki.playersync.mixin", "compatibilityLevel": "JAVA_21", "refmap": "thirst.refmap.json", - "mixins": [ - "cobblemon.MixinFileBackedPokemonStoreFactory", - "cobblemon.MixinNbtBackedPlayerData", - "cobblemon.MixinPartyStore", - "cobblemon.MixinPCStore", - "cobblemon.accessor.FileBasedPlayerDataStoreBackendAccessor", - "cobblemon.accessor.NbtBackedPlayerDataAccessor" - ], - "client": [ - ], + "mixins": [], + "client": [], "injectors": { "defaultRequire": 1 },