some more safety checks upon trade confirmation

src/main/java/com/trunksbomb/trade/client/TradeScreen.java

@@ -53,6 +53,7 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
     private ContextMenu contextMenu;
     private EditBox amountInput;
     private AmountPrompt amountPrompt;
+    private Button acceptButton;
 
     public TradeScreen(TradeMenu menu, Inventory inventory, Component title) {
         super(menu, inventory, title);
@@ -82,7 +83,7 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
         amountInput.setFilter(value -> value.isEmpty() || value.chars().allMatch(Character::isDigit));
         addRenderableWidget(amountInput);
 
-        addRenderableWidget(Button.builder(Component.literal("Accept"), button -> sendAction(TradeAction.ACCEPT, -1, 1))
+        acceptButton = addRenderableWidget(Button.builder(acceptButtonLabel(), button -> sendAction(TradeAction.ACCEPT, -1, 1))
                 .bounds(leftPos + CENTER_COLUMN_X + 3, topPos + ACCEPT_BUTTON_Y, ACTION_BUTTON_WIDTH, ACTION_BUTTON_HEIGHT)
                 .build());
         addRenderableWidget(Button.builder(Component.literal("Cancel"), button -> sendAction(TradeAction.DECLINE, -1, 1))
@@ -225,17 +226,34 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
         if (menu.view().otherAccepted()) {
             drawScaledCenteredColumnText(guiGraphics, "Other player", STATUS_LABEL_Y, 0x2E7D32, 0.7F);
             drawScaledCenteredColumnText(guiGraphics, "has accepted", STATUS_LABEL_Y + 7, 0x2E7D32, 0.7F);
+        } else if (menu.view().selfAccepted()) {
+            if (menu.view().stage() == TradeStage.CONFIRMING) {
+                drawScaledCenteredColumnText(guiGraphics, "Waiting for", STATUS_LABEL_Y, 0x2E7D32, 0.7F);
+                drawScaledCenteredColumnText(guiGraphics, "other player", STATUS_LABEL_Y + 7, 0x2E7D32, 0.7F);
+                drawScaledCenteredColumnText(guiGraphics, "to confirm", STATUS_LABEL_Y + 14, 0x2E7D32, 0.7F);
+            } else {
+                drawScaledCenteredColumnText(guiGraphics, "Waiting for", STATUS_LABEL_Y, 0x2E7D32, 0.7F);
+                drawScaledCenteredColumnText(guiGraphics, "other player", STATUS_LABEL_Y + 7, 0x2E7D32, 0.7F);
+                drawScaledCenteredColumnText(guiGraphics, "to accept", STATUS_LABEL_Y + 14, 0x2E7D32, 0.7F);
+            }
         }
     }
 
     @Override
     public void render(GuiGraphics guiGraphics, int mouseX, int mouseY, float partialTick) {
+        if (acceptButton != null) {
+            acceptButton.setMessage(acceptButtonLabel());
+        }
         super.render(guiGraphics, mouseX, mouseY, partialTick);
         renderContextMenu(guiGraphics, mouseX, mouseY);
         renderAmountPrompt(guiGraphics);
         renderTooltip(guiGraphics, mouseX, mouseY);
     }
 
+    private Component acceptButtonLabel() {
+        return Component.literal(menu.view().stage() == TradeStage.CONFIRMING ? "Confirm" : "Accept");
+    }
+
     private void sendAction(TradeAction action, int slot, int amount) {
         if (minecraft == null || minecraft.getConnection() == null) {
             return;

src/main/java/com/trunksbomb/trade/trade/TradeManager.java

@@ -48,7 +48,7 @@ public class TradeManager {
         sessionByPlayer.put(first.getUUID(), session.id());
         sessionByPlayer.put(second.getUUID(), session.id());
         session.syncToPlayers();
-        TradeAuditLog.log(first.server, "OPEN " + playerName(first) + " <-> " + playerName(second));
+        TradeAuditLog.log(first.server, "OPEN " + playerAudit(first) + " <-> " + playerAudit(second));
         first.sendSystemMessage(Component.literal("Trade opened with " + second.getGameProfile().getName() + "."));
         second.sendSystemMessage(Component.literal("Trade opened with " + first.getGameProfile().getName() + "."));
         return true;
@@ -75,8 +75,9 @@ public class TradeManager {
             return false;
         }
 
-        if (!withinTradeRange(requester, target)) {
+        Component rangeFailure = tradeRangeFailure(requester, target);
-            requester.sendSystemMessage(Component.literal("That player is too far away to trade."));
+        if (rangeFailure != null) {
+            requester.sendSystemMessage(rangeFailure);
             return false;
         }
 
@@ -99,7 +100,7 @@ public class TradeManager {
         }
 
         pendingRequestsByTarget.put(target.getUUID(), new TradeRequest(requester.getUUID(), target.getUUID(), target.server.getTickCount()));
-        TradeAuditLog.log(requester.server, "REQUEST " + playerName(requester) + " -> " + playerName(target));
+        TradeAuditLog.log(requester.server, "REQUEST " + playerAudit(requester) + " -> " + playerAudit(target));
         requester.sendSystemMessage(Component.literal("Trade request sent to " + target.getGameProfile().getName() + "."));
         target.sendSystemMessage(Component.literal(requester.getGameProfile().getName() + " would like to trade with you: ")
                 .append(Component.literal("click to accept")
@@ -130,9 +131,10 @@ public class TradeManager {
             return false;
         }
 
-        if (!withinTradeRange(requester, target)) {
+        Component rangeFailure = tradeRangeFailure(requester, target);
-            requester.sendSystemMessage(Component.literal(target.getGameProfile().getName() + " is too far away to trade."));
+        if (rangeFailure != null) {
-            target.sendSystemMessage(Component.literal(requester.getGameProfile().getName() + " is too far away to trade."));
+            requester.sendSystemMessage(Component.literal("Trade request cancelled because ").append(rangeFailure));
+            target.sendSystemMessage(Component.literal("Trade request cancelled because ").append(rangeFailure));
             return false;
         }
 
@@ -140,7 +142,7 @@ public class TradeManager {
         List<Component> requesterUnsafe = tradeSafetyFailures(requester, currentTick);
         if (!requesterUnsafe.isEmpty()) {
             if (canDelayTradeAcceptance(requester, currentTick)) {
-                TradeAuditLog.log(target.server, "REQUEST ACCEPT DELAYED " + playerName(target) + " -> " + playerName(requester));
+                TradeAuditLog.log(target.server, "REQUEST ACCEPT DELAYED " + playerAudit(target) + " -> " + playerAudit(requester));
                 beginPendingAcceptance(requester, target, currentTick);
                 return true;
             }
@@ -161,7 +163,7 @@ public class TradeManager {
             return false;
         }
 
-        TradeAuditLog.log(target.server, "REQUEST ACCEPTED " + playerName(target) + " accepted " + playerName(requester));
+        TradeAuditLog.log(target.server, "REQUEST ACCEPTED " + playerAudit(target) + " accepted " + playerAudit(requester));
         return true;
     }
 
@@ -179,7 +181,7 @@ public class TradeManager {
     public boolean declinePendingTrade(ServerPlayer target) {
         DebugTradeRequest debugRequest = pendingDebugRequestsByTarget.remove(target.getUUID());
         if (debugRequest != null) {
-            TradeAuditLog.log(target.server, "DEBUG REQUEST DECLINED by " + playerName(target));
+            TradeAuditLog.log(target.server, "DEBUG REQUEST DECLINED by " + playerAudit(target));
             target.sendSystemMessage(Component.literal("Debug trade request declined."));
             return true;
         }
@@ -192,7 +194,7 @@ public class TradeManager {
 
         ServerPlayer requester = target.server.getPlayerList().getPlayer(request.requester());
         if (requester != null) {
-            TradeAuditLog.log(target.server, "REQUEST DECLINED " + playerName(target) + " declined " + playerName(requester));
+            TradeAuditLog.log(target.server, "REQUEST DECLINED " + playerAudit(target) + " declined " + playerAudit(requester));
             requester.sendSystemMessage(Component.literal(target.getGameProfile().getName() + " declined your trade request."));
         }
         target.sendSystemMessage(Component.literal("Trade request declined."));
@@ -242,10 +244,11 @@ public class TradeManager {
             return;
         }
 
-        if (session.completeTrade()) {
+        TradeSession.CompletionResult completion = session.completeTrade();
+        if (completion.successful()) {
             finish(session, Component.literal("Trade completed."));
         } else {
-            cancel(session, Component.literal("Trade cancelled because one player could not fit all traded items."));
+            cancel(session, completion.failureReason());
         }
     }
 
@@ -577,7 +580,7 @@ public class TradeManager {
     private void finish(TradeSession session, Component reason) {
         TradeAuditLog.log(
                 session.firstPlayer().server,
-                "COMPLETE " + playerName(session.firstPlayer()) + " <-> " + playerName(session.secondPlayer()) + " | "
+                "COMPLETE " + playerAudit(session.firstPlayer()) + " <-> " + playerAudit(session.secondPlayer()) + " | "
                         + offerSummary(playerName(session.firstPlayer()), session.firstOfferSnapshot()) + " | "
                         + offerSummary(playerName(session.secondPlayer()), session.secondOfferSnapshot()) + " | "
                         + reason.getString());
@@ -590,7 +593,7 @@ public class TradeManager {
     private void cancel(TradeSession session, Component reason) {
         TradeAuditLog.log(
                 session.firstPlayer().server,
-                "CANCEL " + playerName(session.firstPlayer()) + " <-> " + playerName(session.secondPlayer()) + " | "
+                "CANCEL " + playerAudit(session.firstPlayer()) + " <-> " + playerAudit(session.secondPlayer()) + " | "
                         + offerSummary(playerName(session.firstPlayer()), session.firstOfferSnapshot()) + " | "
                         + offerSummary(playerName(session.secondPlayer()), session.secondOfferSnapshot()) + " | "
                         + reason.getString());
@@ -609,7 +612,7 @@ public class TradeManager {
     private void finishDebug(DebugTradeSession session, Component reason) {
         TradeAuditLog.log(
                 session.player().server,
-                "DEBUG COMPLETE " + playerName(session.player()) + " | "
+                "DEBUG COMPLETE " + playerAudit(session.player()) + " | "
                         + offerSummary(playerName(session.player()), session.selfOfferSnapshot()) + " | "
                         + offerSummary("Debug Trader", session.otherOfferSnapshot()) + " | "
                         + reason.getString());
@@ -621,7 +624,7 @@ public class TradeManager {
     private void closeDebug(DebugTradeSession session, Component reason) {
         TradeAuditLog.log(
                 session.player().server,
-                "DEBUG CLOSE " + playerName(session.player()) + " | "
+                "DEBUG CLOSE " + playerAudit(session.player()) + " | "
                         + offerSummary(playerName(session.player()), session.selfOfferSnapshot()) + " | "
                         + offerSummary("Debug Trader", session.otherOfferSnapshot()) + " | "
                         + reason.getString());
@@ -636,15 +639,7 @@ public class TradeManager {
     }
 
     private boolean withinTradeRange(ServerPlayer first, ServerPlayer second) {
-        if (TradeConfig.requireSameDimension() && first.level() != second.level()) {
+        return tradeRangeFailure(first, second) == null;
-            return false;
-        }
-        int maxDistance = TradeConfig.tradeCommandProximity();
-        if (maxDistance <= 0) {
-            return true;
-        }
-        double maxDistanceSquared = maxDistance * maxDistance;
-        return first.distanceToSqr(second) <= maxDistanceSquared;
     }
 
     private void clearPendingRequests(ServerPlayer player) {
@@ -652,7 +647,7 @@ public class TradeManager {
         if (direct != null) {
             ServerPlayer requester = player.server.getPlayerList().getPlayer(direct.requester());
             if (requester != null) {
-                TradeAuditLog.log(player.server, "REQUEST EXPIRED " + playerName(requester) + " -> " + playerName(player));
+                TradeAuditLog.log(player.server, "REQUEST EXPIRED " + playerAudit(requester) + " -> " + playerAudit(player));
                 requester.sendSystemMessage(Component.literal("Your trade request to " + player.getGameProfile().getName() + " expired."));
             }
         }
@@ -663,7 +658,7 @@ public class TradeManager {
             }
             ServerPlayer target = player.server.getPlayerList().getPlayer(entry.getValue().target());
             if (target != null) {
-                TradeAuditLog.log(player.server, "REQUEST EXPIRED " + playerName(player) + " -> " + playerName(target));
+                TradeAuditLog.log(player.server, "REQUEST EXPIRED " + playerAudit(player) + " -> " + playerAudit(target));
                 target.sendSystemMessage(Component.literal("The trade request from " + player.getGameProfile().getName() + " expired."));
             }
             return true;
@@ -690,7 +685,7 @@ public class TradeManager {
             ServerPlayer requester = server.getPlayerList().getPlayer(request.requester());
             ServerPlayer target = server.getPlayerList().getPlayer(request.target());
             if (requester != null) {
-                TradeAuditLog.log(server, "REQUEST EXPIRED " + playerName(requester) + " -> " + nameFor(request.target(), server));
+                TradeAuditLog.log(server, "REQUEST EXPIRED " + playerAudit(requester) + " -> " + nameFor(request.target(), server));
                 requester.sendSystemMessage(Component.literal("Your trade request expired."));
             }
             if (target != null) {
@@ -714,13 +709,13 @@ public class TradeManager {
             }
 
             if (scheduled.autoAccept()) {
-                TradeAuditLog.log(server, "DEBUG INIT ACCEPT " + playerName(target));
+                TradeAuditLog.log(server, "DEBUG INIT ACCEPT " + playerAudit(target));
                 startOrDelayDebugTrade(target);
                 continue;
             }
 
             pendingDebugRequestsByTarget.put(target.getUUID(), new DebugTradeRequest(target.getUUID(), server.getTickCount()));
-            TradeAuditLog.log(server, "DEBUG REQUEST " + playerName(target));
+            TradeAuditLog.log(server, "DEBUG REQUEST " + playerAudit(target));
             target.sendSystemMessage(Component.literal("Debug Trader would like to trade with you: ")
                     .append(Component.literal("click to accept")
                             .withStyle(Style.EMPTY
@@ -741,8 +736,9 @@ public class TradeManager {
                 continue;
             }
 
-            if (!withinTradeRange(first, second)) {
+            Component rangeFailure = tradeRangeFailure(first, second);
-                cancel(session, Component.literal("Trade cancelled because players moved too far apart."));
+            if (rangeFailure != null) {
+                cancel(session, Component.literal("Trade cancelled because ").append(rangeFailure));
                 continue;
             }
 
@@ -788,11 +784,12 @@ public class TradeManager {
                 continue;
             }
 
-            if (target != null && !withinTradeRange(requester, target)) {
+            Component rangeFailure = target == null ? null : tradeRangeFailure(requester, target);
+            if (rangeFailure != null) {
                 clearPendingAcceptance(
                         requester,
-                        Component.literal("Trade request cancelled because " + target.getGameProfile().getName() + " is too far away to trade."),
+                        Component.literal("Trade request cancelled because ").append(rangeFailure),
-                        Component.literal("Trade request cancelled because players moved too far apart."));
+                        Component.literal("Trade request cancelled because ").append(rangeFailure));
                 continue;
             }
 
@@ -1073,11 +1070,35 @@ public class TradeManager {
         return player.getGameProfile().getName() + " [" + player.getUUID() + "]";
     }
 
+    private String playerAudit(ServerPlayer player) {
+        return playerName(player) + " @ " + locationSummary(player);
+    }
+
     private String nameFor(UUID playerId, MinecraftServer server) {
         ServerPlayer player = server.getPlayerList().getPlayer(playerId);
         return player != null ? playerName(player) : playerId.toString();
     }
 
+    private String locationSummary(ServerPlayer player) {
+        String dimension = player.level().dimension().location().toString();
+        return dimension + " (" + player.blockPosition().getX() + ", " + player.blockPosition().getY() + ", " + player.blockPosition().getZ() + ")";
+    }
+
+    private Component tradeRangeFailure(ServerPlayer first, ServerPlayer second) {
+        if (TradeConfig.requireSameDimension() && first.level() != second.level()) {
+            return Component.literal("players must be in the same dimension to trade.");
+        }
+        int maxDistance = TradeConfig.tradeCommandProximity();
+        if (maxDistance <= 0) {
+            return null;
+        }
+        double maxDistanceSquared = maxDistance * maxDistance;
+        if (first.distanceToSqr(second) > maxDistanceSquared) {
+            return Component.literal("players are too far apart to trade.");
+        }
+        return null;
+    }
+
     private String offerSummary(String trader, List<ItemStack> offer) {
         List<String> entries = new ArrayList<>();
         for (ItemStack stack : offer) {

src/main/java/com/trunksbomb/trade/trade/TradeSession.java

@@ -10,6 +10,7 @@ import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.world.entity.player.Inventory;
 import net.minecraft.world.item.ItemStack;
 import net.neoforged.neoforge.network.PacketDistributor;
+import com.trunksbomb.trade.mod.TradeConfig;
 
 public class TradeSession {
     private static final int INVENTORY_SLOT_COUNT = TradeView.INVENTORY_SLOT_COUNT;
@@ -157,16 +158,36 @@ public class TradeSession {
         return stage == TradeStage.CONFIRMING;
     }
 
-    public boolean completeTrade() {
+    public CompletionResult completeTrade() {
+        if (first.level() != second.level()) {
+            return CompletionResult.failure(Component.literal("Trade cancelled because both players must remain in the same dimension."));
+        }
+
+        int maxDistance = TradeConfig.tradeCommandProximity();
+        if (maxDistance > 0) {
+            double maxDistanceSquared = maxDistance * maxDistance;
+            if (first.distanceToSqr(second) > maxDistanceSquared) {
+                return CompletionResult.failure(Component.literal("Trade cancelled because players moved too far apart before it was finalized."));
+            }
+        }
+
+        if (!inventoryMatchesSnapshot(first, firstInventory)) {
+            return CompletionResult.failure(Component.literal("Trade cancelled because " + first.getGameProfile().getName() + "'s inventory changed during the trade."));
+        }
+
+        if (!inventoryMatchesSnapshot(second, secondInventory)) {
+            return CompletionResult.failure(Component.literal("Trade cancelled because " + second.getGameProfile().getName() + "'s inventory changed during the trade."));
+        }
+
         List<ItemStack> firstResult = simulateResult(first, firstOffer, secondOffer);
         List<ItemStack> secondResult = simulateResult(second, secondOffer, firstOffer);
         if (firstResult == null || secondResult == null) {
-            return false;
+            return CompletionResult.failure(Component.literal("Trade cancelled because one player could not fit all traded items."));
         }
 
         applyInventory(first, firstResult);
         applyInventory(second, secondResult);
-        return true;
+        return CompletionResult.success();
     }
 
     public TradeView viewFor(ServerPlayer player) {
@@ -270,6 +291,16 @@ public class TradeSession {
         inventory.setChanged();
     }
 
+    private static boolean inventoryMatchesSnapshot(ServerPlayer player, List<ItemStack> snapshot) {
+        Inventory inventory = player.getInventory();
+        for (int i = 0; i < INVENTORY_SLOT_COUNT; i++) {
+            if (!ItemStack.matches(inventory.getItem(i), snapshot.get(i))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     private static List<TradeEntry> blankOffer() {
         List<TradeEntry> result = new ArrayList<>(OFFER_SLOT_COUNT);
         for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
@@ -362,4 +393,14 @@ public class TradeSession {
         }
         return result;
     }
+
+    public record CompletionResult(boolean successful, Component failureReason) {
+        public static CompletionResult success() {
+            return new CompletionResult(true, null);
+        }
+
+        public static CompletionResult failure(Component failureReason) {
+            return new CompletionResult(false, failureReason);
+        }
+    }
 }