From bf277bbdfecde9d2fd8b3eb7854b52d5a463910d Mon Sep 17 00:00:00 2001 From: Kam Date: Tue, 24 Feb 2026 17:46:52 -0500 Subject: [PATCH] Auction House Space Check Fix --- .../request/auction/ManageAuctionPacket.java | 5 +- .../npcs/controllers/AuctionController.java | 179 +++++++++++++----- .../controllers/data/PlayerTradeData.java | 14 +- 3 files changed, 148 insertions(+), 50 deletions(-) diff --git a/src/main/java/kamkeel/npcs/network/packets/request/auction/ManageAuctionPacket.java b/src/main/java/kamkeel/npcs/network/packets/request/auction/ManageAuctionPacket.java index 10b4504b9..f2ad4c961 100644 --- a/src/main/java/kamkeel/npcs/network/packets/request/auction/ManageAuctionPacket.java +++ b/src/main/java/kamkeel/npcs/network/packets/request/auction/ManageAuctionPacket.java @@ -366,8 +366,9 @@ public class ManageAuctionPacket extends AbstractPacket { String result = controller.createGlobalListing(player, sellerName, item, startingPrice, buyoutPrice, durationHours); if (result != null) { // Failed create, return item (or drop to avoid loss). - if (!player.inventory.addItemStackToInventory(item.copy())) { - player.entityDropItem(item.copy(), 0.5f); + // Use same reference so partial add mutates stackSize, then drop only the remainder. + if (!player.inventory.addItemStackToInventory(item)) { + player.entityDropItem(item, 0.5f); } } if (result == null) { diff --git a/src/main/java/noppes/npcs/controllers/AuctionController.java b/src/main/java/noppes/npcs/controllers/AuctionController.java index 8bf06343f..b754accac 100644 --- a/src/main/java/noppes/npcs/controllers/AuctionController.java +++ b/src/main/java/noppes/npcs/controllers/AuctionController.java @@ -289,7 +289,15 @@ public class AuctionController implements IAuctionHandler { // Global claims are not tied to a single player. claim.playerUUID = null; claim.playerName = "Global"; - globalClaims.add(claim); + synchronized (globalClaims) { + // Prevent duplicate claim IDs + for (AuctionClaim existing : globalClaims) { + if (existing.id.equals(claim.id)) { + return; + } + } + globalClaims.add(claim); + } markDirty(); } @@ -305,19 +313,23 @@ public class AuctionController implements IAuctionHandler { private AuctionClaim getGlobalClaimInternal(String claimId) { if (claimId == null || claimId.isEmpty()) return null; - for (AuctionClaim claim : globalClaims) { - if (claim != null && !claim.claimed && claimId.equals(claim.id)) { - return claim; + synchronized (globalClaims) { + for (AuctionClaim claim : globalClaims) { + if (claim != null && !claim.claimed && claimId.equals(claim.id)) { + return claim; + } } } return null; } private boolean claimAndRemoveGlobal(String claimId) { - AuctionClaim claim = getGlobalClaimInternal(claimId); - if (claim == null) return false; - claim.claimed = true; - return globalClaims.remove(claim); + synchronized (globalClaims) { + AuctionClaim claim = getGlobalClaimInternal(claimId); + if (claim == null) return false; + claim.claimed = true; + return globalClaims.remove(claim); + } } /** @@ -358,15 +370,27 @@ public class AuctionController implements IAuctionHandler { return "This item cannot be listed on the Auction House."; } - // Validate minimum price + // Validate prices + if (startingPrice <= 0) { + return "Starting price must be positive."; + } if (startingPrice < ConfigMarket.MinimumListingPrice) { return "Starting price must be at least " + ConfigMarket.MinimumListingPrice + " " + ConfigMarket.CurrencyName + "."; } + if (startingPrice > ConfigMarket.MaxBalance) { + return "Starting price exceeds maximum allowed value."; + } // Validate buyout price if set + if (buyoutPrice < 0) { + return "Buyout price cannot be negative."; + } if (buyoutPrice > 0 && buyoutPrice < startingPrice) { return "Buyout price must be higher than starting price."; } + if (buyoutPrice > ConfigMarket.MaxBalance) { + return "Buyout price exceeds maximum allowed value."; + } UUID playerUUID = player.getUniqueID(); String playerName = player.getCommandSenderName(); @@ -435,9 +459,16 @@ public class AuctionController implements IAuctionHandler { return "No item provided."; } + // Validate prices + if (startingPrice <= 0) { + return "Starting price must be positive."; + } if (startingPrice < ConfigMarket.MinimumListingPrice) { return "Starting price must be at least " + ConfigMarket.MinimumListingPrice + " " + ConfigMarket.CurrencyName + "."; } + if (buyoutPrice < 0) { + return "Buyout price cannot be negative."; + } if (buyoutPrice > 0 && buyoutPrice < startingPrice) { return "Buyout price must be higher than starting price."; } @@ -514,7 +545,13 @@ public class AuctionController implements IAuctionHandler { if (penalty > 0) { PlayerData sellerData = PlayerDataController.Instance.getData(listing.sellerUUID); if (sellerData != null) { - sellerData.tradeData.withdraw(penalty); + if (!sellerData.tradeData.withdraw(penalty)) { + // Withdraw whatever they have if they can't cover the full penalty + long available = sellerData.tradeData.getBalance(); + if (available > 0) { + sellerData.tradeData.withdraw(available); + } + } sellerData.save(); } } @@ -551,6 +588,11 @@ public class AuctionController implements IAuctionHandler { return "Auction is disabled."; } + // Validate bid amount + if (bidAmount <= 0) { + return "Bid amount must be positive."; + } + UUID playerUUID = player.getUniqueID(); String playerName = player.getCommandSenderName(); @@ -641,14 +683,14 @@ public class AuctionController implements IAuctionHandler { listing.highBidderName = playerName; listing.currentBid = bidAmount; listing.bidCount++; + + // Add to new bidder's active bids + addToPlayerBids(playerUUID, listing.id); + + // Snipe protection (must be inside sync to prevent race with endAuction) + listing.extendForSnipeProtection(ConfigMarket.SnipeProtectionMinutes); } - // Add to new bidder's active bids - addToPlayerBids(playerUUID, listing.id); - - // Snipe protection - listing.extendForSnipeProtection(ConfigMarket.SnipeProtectionMinutes); - markDirty(); logAuction(EnumAuctionLogAction.BID, playerName, listing.item.getDisplayName(), bidAmount, @@ -834,26 +876,36 @@ public class AuctionController implements IAuctionHandler { } public String claimGlobalItem(String claimId, EntityPlayer player) { - AuctionClaim claim = getGlobalClaimInternal(claimId); - if (claim == null) { - return "Global claim not found."; - } - if (!claim.type.isItem()) { - return "This global claim is not an item claim."; - } - if (claim.item == null) { - return "Item data is missing."; - } + String listingId; + String itemName; + synchronized (globalClaims) { + AuctionClaim claim = getGlobalClaimInternal(claimId); + if (claim == null) { + return "Global claim not found."; + } + if (!claim.type.isItem()) { + return "This global claim is not an item claim."; + } + if (claim.item == null) { + return "Item data is missing."; + } - if (!player.inventory.addItemStackToInventory(claim.item.copy())) { - return "Your inventory is full."; - } + // Check inventory space before adding (addItemStackToInventory mutates even on partial add) + if (!canFitInInventory(player, claim.item)) { + return "Not enough inventory space to claim this item."; + } - claimAndRemoveGlobal(claim.id); - cleanupListingIfComplete(claim.listingId); + // Add to inventory (safe - we verified space above) + player.inventory.addItemStackToInventory(claim.item.copy()); + + listingId = claim.listingId; + itemName = claim.item.getDisplayName(); + claimAndRemoveGlobal(claim.id); + } + cleanupListingIfComplete(listingId); markDirty(); - logAuction(EnumAuctionLogAction.CLAIMED, player.getCommandSenderName(), claim.item.getDisplayName(), 0, + logAuction(EnumAuctionLogAction.CLAIMED, player.getCommandSenderName(), itemName, 0, "Global item claim"); return null; } @@ -862,19 +914,25 @@ public class AuctionController implements IAuctionHandler { * Global currency claims are acknowledged/cleared and do not alter player balances. */ public String claimGlobalCurrency(String claimId, EntityPlayer player) { - AuctionClaim claim = getGlobalClaimInternal(claimId); - if (claim == null) { - return "Global claim not found."; - } - if (!claim.type.isCurrency()) { - return "This global claim is not a currency claim."; - } + String listingId; + long currency; + synchronized (globalClaims) { + AuctionClaim claim = getGlobalClaimInternal(claimId); + if (claim == null) { + return "Global claim not found."; + } + if (!claim.type.isCurrency()) { + return "This global claim is not a currency claim."; + } - claimAndRemoveGlobal(claim.id); - cleanupListingIfComplete(claim.listingId); + listingId = claim.listingId; + currency = claim.currency; + claimAndRemoveGlobal(claim.id); + } + cleanupListingIfComplete(listingId); markDirty(); - logAuction(EnumAuctionLogAction.CLAIMED, player.getCommandSenderName(), ConfigMarket.CurrencyName, claim.currency, + logAuction(EnumAuctionLogAction.CLAIMED, player.getCommandSenderName(), ConfigMarket.CurrencyName, currency, "Global currency claim acknowledged (no balance change)"); return null; } @@ -910,11 +968,14 @@ public class AuctionController implements IAuctionHandler { return "Claim was prevented."; } - // Check inventory space - if (!player.inventory.addItemStackToInventory(claim.item.copy())) { - return "Your inventory is full."; + // Check inventory space before adding (addItemStackToInventory mutates even on partial add) + if (!canFitInInventory(player, claim.item)) { + return "Not enough inventory space to claim this item."; } + // Add to inventory (safe - we verified space above) + player.inventory.addItemStackToInventory(claim.item.copy()); + // Remove claim from player's auction data playerData.tradeData.claimAndRemove(claimId); playerData.updateClient = true; @@ -973,6 +1034,34 @@ public class AuctionController implements IAuctionHandler { return null; // Success } + /** + * Check if a player's inventory can fit the entire ItemStack without partial adds. + * Unlike addItemStackToInventory, this does NOT mutate the inventory. + */ + private boolean canFitInInventory(EntityPlayer player, ItemStack stack) { + int remaining = stack.stackSize; + int maxStack = stack.getMaxStackSize(); + ItemStack[] mainInventory = player.inventory.mainInventory; + + for (int i = 0; i < mainInventory.length; i++) { + if (remaining <= 0) return true; + + ItemStack slot = mainInventory[i]; + if (slot == null) { + remaining -= Math.min(remaining, maxStack); + } else if (slot.getItem() == stack.getItem() + && slot.getItemDamage() == stack.getItemDamage() + && ItemStack.areItemStackTagsEqual(slot, stack)) { + int spaceInSlot = maxStack - slot.stackSize; + if (spaceInSlot > 0) { + remaining -= Math.min(remaining, spaceInSlot); + } + } + } + + return remaining <= 0; + } + /** * Clean up a listing if it's ended and no longer needed. * Now just removes ended listings after some delay since claims are in PlayerData. diff --git a/src/main/java/noppes/npcs/controllers/data/PlayerTradeData.java b/src/main/java/noppes/npcs/controllers/data/PlayerTradeData.java index 2850ddb90..f2c0d19d6 100644 --- a/src/main/java/noppes/npcs/controllers/data/PlayerTradeData.java +++ b/src/main/java/noppes/npcs/controllers/data/PlayerTradeData.java @@ -245,8 +245,14 @@ public class PlayerTradeData implements IPlayerTradeData { * Add a claim to this player's pending claims. * Thread-safe - can be called from any thread. */ - public void addClaim(AuctionClaim claim) { + public synchronized void addClaim(AuctionClaim claim) { if (claim != null && !claim.claimed) { + // Prevent duplicate claim IDs + for (AuctionClaim existing : claims) { + if (existing.id.equals(claim.id)) { + return; + } + } claims.add(claim); } } @@ -276,8 +282,9 @@ public class PlayerTradeData implements IPlayerTradeData { /** * Get a specific claim by ID (internal use). + * Synchronized to pair with claimAndRemove for thread safety. */ - public AuctionClaim getClaimInternal(String claimId) { + public synchronized AuctionClaim getClaimInternal(String claimId) { for (AuctionClaim claim : claims) { if (claim.id.equals(claimId) && !claim.claimed) { return claim; @@ -296,8 +303,9 @@ public class PlayerTradeData implements IPlayerTradeData { /** * Mark a claim as claimed and remove it. + * Synchronized to prevent double-claim from concurrent threads. */ - public boolean claimAndRemove(String claimId) { + public synchronized boolean claimAndRemove(String claimId) { for (AuctionClaim claim : claims) { if (claim.id.equals(claimId) && !claim.claimed) { claim.claimed = true;