Merge pull request #246 from KAMKEEL/review-and-fix-auction-trader-market-system

Harden auctions and trader flows: validation, UI safety, admin cancel/refund, and config fixes

src/main/java/kamkeel/npcs/network/packets/player/AuctionActionPacket.java

@@ -17,6 +17,7 @@ import noppes.npcs.CustomNpcsPermissions;
 import noppes.npcs.config.ConfigMarket;
 import noppes.npcs.constants.EnumAuctionDuration;
 import noppes.npcs.constants.EnumRoleType;
+import noppes.npcs.containers.ContainerAuction;
 import noppes.npcs.controllers.AuctionController;
 import noppes.npcs.controllers.CurrencyController;
 import noppes.npcs.controllers.data.AuctionFilter;
@@ -290,10 +291,23 @@ public class AuctionActionPacket extends AbstractPacket {
 
         try {
             NBTTagCompound itemNBT = NBTJsonUtil.Convert(itemJson);
-            ItemStack item = ItemStack.loadItemStackFromNBT(itemNBT);
+            ItemStack requestedItem = ItemStack.loadItemStackFromNBT(itemNBT);
 
-            if (item == null) {
-                sendResponse(player, false, "Invalid item");
+            if (!(player.openContainer instanceof ContainerAuction)) {
+                sendResponse(player, false, "Open the auction listing screen to create a listing");
+                return;
+            }
+
+            ContainerAuction container = (ContainerAuction) player.openContainer;
+            ItemStack slotItem = container.getListingItem();
+
+            if (requestedItem == null || slotItem == null) {
+                sendResponse(player, false, "Place an item in the listing slot");
+                return;
+            }
+
+            if (!ItemStack.areItemStacksEqual(slotItem, requestedItem) || slotItem.stackSize != requestedItem.stackSize) {
+                sendResponse(player, false, "Listing item mismatch");
                 return;
             }
 
@@ -318,11 +332,10 @@ public class AuctionActionPacket extends AbstractPacket {
 
             // Create the listing
             AuctionListing listing = AuctionController.Instance.createListing(
-                player, item, startingPrice, buyoutPrice, duration);
+                player, slotItem, startingPrice, buyoutPrice, duration);
 
             if (listing != null) {
-                // Remove item from player's inventory
-                // Note: The GUI should have already collected the item, this is validation
+                container.clearListingSlot();
                 sendResponse(player, true, "Listing created successfully");
             } else {
                 sendResponse(player, false, "Failed to create listing. Check your balance and listing limit.");
@@ -429,7 +442,7 @@ public class AuctionActionPacket extends AbstractPacket {
             return;
         }
 
-        if (AuctionController.Instance.cancelListing(listingId, player)) {
+        if (AuctionController.Instance.cancelListing(listingId, player, isAdmin)) {
             sendResponse(player, true, "Listing cancelled. Claim your item.");
         } else {
             sendResponse(player, false, "Failed to cancel listing");

src/main/java/noppes/npcs/client/gui/player/GuiAuction.java

@@ -215,6 +215,9 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
     private void viewDetails() {
         if (selectedListingIndex >= 0 && selectedListingIndex < listings.size()) {
             AuctionListing listing = listings.get(selectedListingIndex);
+            if (listing.item == null) {
+                return;
+            }
             setSubGui(new SubGuiAuctionDetails(this, listing));
         }
     }
@@ -222,6 +225,9 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
     private void placeBid() {
         if (selectedListingIndex >= 0 && selectedListingIndex < listings.size()) {
             AuctionListing listing = listings.get(selectedListingIndex);
+            if (listing.item == null) {
+                return;
+            }
             setSubGui(new SubGuiAuctionBid(this, listing, playerBalance));
         }
     }
@@ -261,7 +267,7 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
             String line = formatListingLine(listing);
             displayList.add(line);
         }
-        listingScroll.setList(displayList);
+        listingScroll.setList(displayList, true, false);
 
         if (selectedListingIndex >= 0 && selectedListingIndex < displayList.size()) {
             listingScroll.selected = selectedListingIndex;
@@ -269,7 +275,9 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
     }
 
     private String formatListingLine(AuctionListing listing) {
-        String itemName = listing.item.getDisplayName();
+        String itemName = listing.item == null
+            ? "Unknown"
+            : listing.item.getDisplayName();
         if (itemName.length() > 15) {
             itemName = itemName.substring(0, 12) + "...";
         }
@@ -324,12 +332,17 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
 
         // Action buttons based on selection
         boolean hasSelection = selectedListingIndex >= 0 && selectedListingIndex < listings.size();
+        if (hasSelection && listings.get(selectedListingIndex).item == null) {
+            hasSelection = false;
+        }
         if (getButton(50) != null) getButton(50).setEnabled(hasSelection);
         if (getButton(51) != null) getButton(51).setEnabled(hasSelection);
 
         if (hasSelection && getButton(52) != null) {
             AuctionListing listing = listings.get(selectedListingIndex);
-            getButton(52).setEnabled(listing.buyoutPrice > 0 && playerBalance >= listing.buyoutPrice);
+            getButton(52).setEnabled(listing.item != null &&
+                listing.buyoutPrice > 0 &&
+                playerBalance >= listing.buyoutPrice);
         } else if (getButton(52) != null) {
             getButton(52).setEnabled(false);
         }
@@ -392,18 +405,17 @@ public class GuiAuction extends GuiContainerNPCInterface implements IGuiData,
 
     // ICustomScrollListener
     @Override
-    public void scrollClicked(int scrollId, int index, boolean doubleClick, String selection) {
+    public void customScrollClicked(int mouseX, int mouseY, int mouseButton, GuiCustomScroll scroll) {
+        int index = scroll.selected;
         selectedListingIndex = index;
         updateButtonStates();
-
-        if (doubleClick && selectedListingIndex >= 0 && selectedListingIndex < listings.size()) {
-            viewDetails();
-        }
     }
 
     @Override
-    public void scrollDoubleClicked(String selection, GuiCustomScroll scroll) {
-        // Handled above
+    public void customScrollDoubleClicked(String selection, GuiCustomScroll scroll) {
+        if (selectedListingIndex >= 0 && selectedListingIndex < listings.size()) {
+            viewDetails();
+        }
     }
 
     // IGuiData

src/main/java/noppes/npcs/client/gui/player/SubGuiAuctionBid.java

@@ -35,6 +35,12 @@ public class SubGuiAuctionBid extends SubGuiInterface {
         int y = guiTop + 10;
         int labelColor = 0x404040;
 
+        if (listing.item == null) {
+            addLabel(new GuiNpcLabel(0, StatCollector.translateToLocal("auction.noItemSelected"), guiLeft + 40, y, 0xAA0000));
+            addButton(new GuiNpcButton(31, guiLeft + 60, guiTop + 110, 80, 20, StatCollector.translateToLocal("gui.cancel")));
+            return;
+        }
+
         // Title
         addLabel(new GuiNpcLabel(0, StatCollector.translateToLocal("auction.placeBid"), guiLeft + 70, y, labelColor));
         y += 18;
@@ -70,8 +76,9 @@ public class SubGuiAuctionBid extends SubGuiInterface {
         // Bid input
         addLabel(new GuiNpcLabel(10, StatCollector.translateToLocal("auction.yourBid"), guiLeft + 15, y + 3, labelColor));
         bidField = new GuiNpcTextField(11, this, guiLeft + 80, y, 100, 16, String.valueOf(minBid));
-        bidField.setNumbersOnly();
-        bidField.setMinMaxDefault(minBid, 999999999, minBid);
+        bidField.setIntegersOnly();
+        int minBidInt = (int) Math.min(Integer.MAX_VALUE, Math.max(1L, minBid));
+        bidField.setMinMaxDefault(minBidInt, Integer.MAX_VALUE, minBidInt);
         addTextField(bidField);
         y += 24;
 
@@ -179,7 +186,7 @@ public class SubGuiAuctionBid extends SubGuiInterface {
     }
 
     private void placeBid() {
-        long bidAmount = bidField.getInteger();
+        long bidAmount = parseBid(bidField);
         long minBid = listing.getMinimumBid();
 
         if (bidAmount < minBid) {
@@ -199,6 +206,14 @@ public class SubGuiAuctionBid extends SubGuiInterface {
         close();
     }
 
+    private long parseBid(GuiNpcTextField field) {
+        try {
+            return Long.parseLong(field.getText());
+        } catch (NumberFormatException e) {
+            return 0L;
+        }
+    }
+
     @Override
     public void save() {
         // Nothing to save

src/main/java/noppes/npcs/client/gui/player/SubGuiAuctionCreate.java

@@ -51,14 +51,14 @@ public class SubGuiAuctionCreate extends SubGuiInterface {
         // Starting price
         addLabel(new GuiNpcLabel(10, StatCollector.translateToLocal("auction.startingPrice"), guiLeft + 20, guiTop + 70, 0x404040));
         startingPriceField = new GuiNpcTextField(11, this, guiLeft + 100, guiTop + 68, 80, 14, "100");
-        startingPriceField.setNumbersOnly();
+        startingPriceField.setIntegersOnly();
         startingPriceField.setMinMaxDefault(1, 999999999, 100);
         addTextField(startingPriceField);
 
         // Buyout price (optional)
         addLabel(new GuiNpcLabel(20, StatCollector.translateToLocal("auction.buyoutPrice"), guiLeft + 20, guiTop + 90, 0x404040));
         buyoutPriceField = new GuiNpcTextField(21, this, guiLeft + 100, guiTop + 88, 80, 14, "0");
-        buyoutPriceField.setNumbersOnly();
+        buyoutPriceField.setIntegersOnly();
         buyoutPriceField.setMinMaxDefault(0, 999999999, 0);
         addTextField(buyoutPriceField);
         addLabel(new GuiNpcLabel(22, StatCollector.translateToLocal("auction.noBuyoutHint"), guiLeft + 185, guiTop + 90, 0x888888));
@@ -87,7 +87,7 @@ public class SubGuiAuctionCreate extends SubGuiInterface {
 
     private long getListingFee() {
         EnumAuctionDuration duration = EnumAuctionDuration.values()[selectedDuration];
-        return duration.getFee();
+        return duration.getListingFee();
     }
 
     @Override
@@ -113,8 +113,8 @@ public class SubGuiAuctionCreate extends SubGuiInterface {
             return;
         }
 
-        long startingPrice = startingPriceField.getInteger();
-        long buyoutPrice = buyoutPriceField.getInteger();
+        long startingPrice = parsePrice(startingPriceField, 1);
+        long buyoutPrice = parsePrice(buyoutPriceField, 0);
 
         if (startingPrice < 1) {
             startingPrice = 1;
@@ -135,6 +135,14 @@ public class SubGuiAuctionCreate extends SubGuiInterface {
         close();
     }
 
+    private long parsePrice(GuiNpcTextField field, long defaultValue) {
+        try {
+            return Long.parseLong(field.getText());
+        } catch (NumberFormatException e) {
+            return defaultValue;
+        }
+    }
+
     @Override
     public void updateScreen() {
         super.updateScreen();

src/main/java/noppes/npcs/client/gui/player/SubGuiAuctionDetails.java

@@ -34,6 +34,12 @@ public class SubGuiAuctionDetails extends SubGuiInterface {
         int labelColor = 0x404040;
         int valueColor = 0x000000;
 
+        if (listing.item == null) {
+            addLabel(new GuiNpcLabel(0, StatCollector.translateToLocal("auction.noItemSelected"), guiLeft + 60, y, 0xAA0000));
+            addButton(new GuiNpcButton(50, guiLeft + 80, guiTop + 175, 80, 20, StatCollector.translateToLocal("gui.close")));
+            return;
+        }
+
         // Title
         addLabel(new GuiNpcLabel(0, StatCollector.translateToLocal("auction.auctionDetails"), guiLeft + 80, y, labelColor));
         y += 16;

src/main/java/noppes/npcs/config/ConfigMarket.java

@@ -95,8 +95,16 @@ public class ConfigMarket {
             StartingBalance = config.get(CURRENCY, "Starting Balance", 1000,
                 "Starting balance for new players (only used when Vault is not available)").getInt(1000);
 
-            MaxBalance = config.get(CURRENCY, "Maximum Balance", 999999999999L,
-                "Maximum balance a player can have (only used when Vault is not available)").getLong(999999999999L);
+            Property maxBalanceProp = config.get(CURRENCY, "Maximum Balance", String.valueOf(MaxBalance),
+                "Maximum balance a player can have (only used when Vault is not available)");
+            try {
+                MaxBalance = Long.parseLong(maxBalanceProp.getString());
+            } catch (NumberFormatException e) {
+                MaxBalance = 999999999999L;
+            }
+            if (MaxBalance < 0) {
+                MaxBalance = 0;
+            }
 
             // =========================================
             // Auction Settings

src/main/java/noppes/npcs/controllers/AuctionController.java

@@ -132,6 +132,7 @@ public class AuctionController {
     public List<AuctionListing> searchListings(String query) {
         String lowerQuery = query.toLowerCase();
         return getActiveListings().stream()
+            .filter(l -> l.item != null)
             .filter(l -> l.item.getDisplayName().toLowerCase().contains(lowerQuery))
             .collect(Collectors.toList());
     }
@@ -157,8 +158,14 @@ public class AuctionController {
 
         // Calculate pagination
         int totalResults = results.size();
-        int totalPages = Math.max(1, (int) Math.ceil((double) totalResults / filter.pageSize));
-        int startIndex = filter.page * filter.pageSize;
+        int totalPages = totalResults == 0
+            ? 0
+            : Math.max(1, (int) Math.ceil((double) totalResults / filter.pageSize));
+        int currentPage = Math.max(0, filter.page);
+        if (totalPages > 0 && currentPage >= totalPages) {
+            currentPage = totalPages - 1;
+        }
+        int startIndex = currentPage * filter.pageSize;
         int endIndex = Math.min(startIndex + filter.pageSize, totalResults);
 
         // Get the page slice
@@ -169,7 +176,7 @@ public class AuctionController {
             pageResults = new ArrayList<>(results.subList(startIndex, endIndex));
         }
 
-        return new FilterResult(pageResults, totalResults, filter.page, totalPages);
+        return new FilterResult(pageResults, totalResults, currentPage, totalPages);
     }
 
     /**
@@ -204,11 +211,11 @@ public class AuctionController {
                 comparator = Comparator.comparingLong(l -> -l.expirationTime);
                 break;
             case NAME_A_TO_Z:
-                comparator = Comparator.comparing(l -> l.item.getDisplayName().toLowerCase());
+                comparator = Comparator.comparing(l -> getListingName(l).toLowerCase());
                 break;
             case NAME_Z_TO_A:
                 comparator = Comparator.comparing((AuctionListing l) ->
-                    l.item.getDisplayName().toLowerCase()).reversed();
+                    getListingName(l).toLowerCase()).reversed();
                 break;
             case MOST_BIDS:
                 comparator = Comparator.comparingInt(l -> -l.bidCount);
@@ -287,6 +294,7 @@ public class AuctionController {
         return ids.stream()
             .map(listings::get)
             .filter(Objects::nonNull)
+            .filter(AuctionListing::isActive)
             .filter(l -> bidderUUID.equals(l.highBidderUUID))
             .collect(Collectors.toList());
     }
@@ -433,17 +441,33 @@ public class AuctionController {
     /**
      * Cancel an auction listing
      */
-    public boolean cancelListing(int listingId, EntityPlayer requester) {
+    public boolean cancelListing(int listingId, EntityPlayer requester, boolean forceCancel) {
         AuctionListing listing = listings.get(listingId);
         if (listing == null) {
             return false;
         }
 
         boolean byAdmin = !listing.sellerUUID.equals(requester.getUniqueID());
-
-        if (!listing.cancel(requester.getUniqueID())) {
+        if (byAdmin && !forceCancel) {
             return false;
         }
+        if (!forceCancel && listing.hasBids()) {
+            return false;
+        }
+        if (listing.status != EnumAuctionStatus.ACTIVE) {
+            return false;
+        }
+
+        if (forceCancel && listing.hasBids() && listing.highBidderUUID != null) {
+            CurrencyController.Instance.addClaim(listing.highBidderUUID, listing.currentBid,
+                "Auction Cancelled", String.valueOf(listingId));
+            notifyPlayer(listing.highBidderUUID, EnumChatFormatting.RED +
+                "Your bid on " + EnumChatFormatting.WHITE + listing.item.getDisplayName() +
+                EnumChatFormatting.RED + " was cancelled. Your " +
+                formatCurrency(listing.currentBid) + " has been refunded.");
+        }
+
+        listing.status = EnumAuctionStatus.CANCELLED;
 
         save();
 
@@ -718,6 +742,13 @@ public class AuctionController {
         LogWriter.info(LOG_PREFIX + event + ": " + details);
     }
 
+    private String getListingName(AuctionListing listing) {
+        if (listing == null || listing.item == null) {
+            return "";
+        }
+        return listing.item.getDisplayName();
+    }
+
     // ==================== Persistence ====================
 
     public void save() {

src/main/java/noppes/npcs/controllers/VaultHelper.java

@@ -4,7 +4,7 @@ import net.minecraft.entity.player.EntityPlayer;
 import noppes.npcs.CustomNpcs;
 import noppes.npcs.config.ConfigMarket;
 import noppes.npcs.controllers.data.PlayerData;
-import noppes.npcs.controllers.data.PlayerDataController;
+import noppes.npcs.controllers.PlayerDataController;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 

src/main/java/noppes/npcs/controllers/data/AuctionFilter.java

@@ -107,6 +107,9 @@ public class AuctionFilter {
         if (listing == null) {
             return false;
         }
+        if (listing.item == null) {
+            return false;
+        }
 
         // Search text filter (case-insensitive)
         if (!searchText.isEmpty()) {
@@ -199,9 +202,13 @@ public class AuctionFilter {
         maxPrice = compound.getLong("MaxPrice");
         buyoutOnly = compound.getBoolean("BuyoutOnly");
         noBidsOnly = compound.getBoolean("NoBidsOnly");
-        page = compound.getInteger("Page");
+        page = Math.max(0, compound.getInteger("Page"));
         pageSize = compound.getInteger("PageSize");
-        if (pageSize <= 0) pageSize = 20;
+        if (pageSize <= 0) {
+            pageSize = 20;
+        } else {
+            pageSize = Math.min(100, pageSize);
+        }
         sellerUUID = compound.getString("SellerUUID");
         sellerName = compound.getString("SellerName");
     }

src/main/java/noppes/npcs/controllers/data/TraderStock.java

@@ -58,6 +58,9 @@ public class TraderStock {
             return pStock.getStock(slot, maxStock[slot]);
         }
 
+        if (currentStock[slot] < 0) {
+            return maxStock[slot];
+        }
         return Math.max(0, currentStock[slot]);
     }
 
@@ -83,6 +86,9 @@ public class TraderStock {
             return pStock.consumeStock(slot, maxStock[slot], amount);
         }
 
+        if (currentStock[slot] < 0) {
+            currentStock[slot] = maxStock[slot];
+        }
         if (currentStock[slot] >= amount) {
             currentStock[slot] -= amount;
             return true;

src/main/java/noppes/npcs/roles/RoleAuctioneer.java

@@ -6,6 +6,7 @@ import noppes.npcs.NoppesUtilServer;
 import noppes.npcs.config.ConfigMarket;
 import noppes.npcs.constants.EnumGuiType;
 import noppes.npcs.controllers.AuctionController;
+import noppes.npcs.controllers.data.Line;
 import noppes.npcs.entity.EntityNPCInterface;
 
 /**
@@ -69,9 +70,9 @@ public class RoleAuctioneer extends RoleInterface {
         // Check if auction system is enabled
         if (!ConfigMarket.AuctionEnabled) {
             if (!noAuctionMessage.isEmpty()) {
-                npc.say(player, noAuctionMessage);
+                npc.say(player, new Line(noAuctionMessage));
             } else {
-                npc.say(player, "The auction house is currently closed.");
+                npc.say(player, new Line("The auction house is currently closed."));
             }
             return;
         }
@@ -83,7 +84,7 @@ public class RoleAuctioneer extends RoleInterface {
 
         // Say welcome message
         if (!welcomeMessage.isEmpty()) {
-            npc.say(player, welcomeMessage);
+            npc.say(player, new Line(welcomeMessage));
         } else {
             npc.say(player, npc.advanced.getInteractLine());
         }