add "trade modified" warning if items are removed/reduced, configurable

Add config for requiring second confirmation screen, enabled by default
More resilience to adding items to trade after moving to confirmation screen
This commit is contained in:
trunksbomb
2026-03-25 12:27:20 -04:00
parent 4daed48f0e
commit 89d6545533
8 changed files with 242 additions and 38 deletions

View File

@@ -12,6 +12,7 @@ Players can request a trade, review both offers in a shared GUI, accept once to
- Shared trade screen with separate offer areas for both players
- Two-step confirmation flow
- Quantity-based item selection with quick amounts and `Trade X`
- Warning if items are removed/modified
- Inventory-safe finalization: nothing changes until safety and security checks pass and the trade succeeds
- Configurable trade safety checks to prevent dangerous mid-combat or mid-movement trading
- Per-player trade toggles and ignore list support
@@ -49,6 +50,12 @@ They can also respond manually:
- Once both players accept, the screen enters the confirmation stage
- Click `Confirm` to finalize
If a player reduces or removes items from an offer:
- any pending accepts / confirms are invalidated
- the changed offer slot gets a blinking red border and `!`
- the trade window shows a red `Trade Modified` warning
## Commands
### Player commands
@@ -109,6 +116,7 @@ The trade verifies:
- both players are still within configured trade distance, if enabled
- both live inventories still match the snapshots taken when the trade started
- both players can receive the incoming items
- both players have re-accepted after any offer changes
If any check fails, the trade is cancelled and both players receive an explicit chat message explaining why.
@@ -133,6 +141,16 @@ Server config values live under the `trade` section.
- default: `false`
- enables debug commands and debug UI/testing tools
### Trade flow
- `requireSecondConfirmation`
- default: `true`
- requires the second confirm step after both players accept the offer
- `showTradeModifiedWarnings`
- default: `true`
- shows the `Trade Modified` warning and changed-slot highlights when offers are reduced or removed
### Safety
- `requireOnGround`
@@ -196,4 +214,4 @@ Debug tools are disabled by default and only available when:
- `enableDebugFeatures = true`
When enabled, the mod exposes `/trade debug ...` commands and on-screen debug controls for single-client testing.
When enabled, the mod exposes `/trade debug ...` commands and on-screen debug controls for single-client testing.

View File

@@ -37,8 +37,9 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
private static final int BANNER_X = 58;
private static final int BANNER_Y = 6;
private static final int CENTER_COLUMN_X = 115;
private static final int STATUS_LABEL_Y = 54;
private static final int CONFIRM_LABEL_Y = 34;
private static final int MODIFIED_LABEL_Y = 17;
private static final int STATUS_LABEL_Y = 51;
private static final int CONFIRM_LABEL_Y = 26;
private static final int ACCEPT_BUTTON_Y = 74;
private static final int CANCEL_BUTTON_Y = 98;
private static final int ACTION_BUTTON_WIDTH = 48;
@@ -218,6 +219,9 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
protected void renderLabels(GuiGraphics guiGraphics, int mouseX, int mouseY) {
guiGraphics.drawString(font, "Trading with " + menu.view().otherName(), titleLabelX, titleLabelY, 0x404040, false);
guiGraphics.drawString(font, playerInventoryTitle, inventoryLabelX, inventoryLabelY, 0x404040, false);
if (menu.view().itemsChanged()) {
drawScaledCenteredColumnText(guiGraphics, "Trade Modified", MODIFIED_LABEL_Y, 0xB02020, 0.7F);
}
if (menu.view().stage() == TradeStage.CONFIRMING) {
drawScaledCenteredColumnText(guiGraphics, "Are you", CONFIRM_LABEL_Y, 0xB02020, 0.7F);
drawScaledCenteredColumnText(guiGraphics, "sure you want", CONFIRM_LABEL_Y + 7, 0xB02020, 0.7F);
@@ -245,6 +249,7 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
acceptButton.setMessage(acceptButtonLabel());
}
super.render(guiGraphics, mouseX, mouseY, partialTick);
renderChangedSlotWarnings(guiGraphics);
renderContextMenu(guiGraphics, mouseX, mouseY);
renderAmountPrompt(guiGraphics);
renderTooltip(guiGraphics, mouseX, mouseY);
@@ -254,6 +259,36 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
return Component.literal(menu.view().stage() == TradeStage.CONFIRMING ? "Confirm" : "Accept");
}
private void renderChangedSlotWarnings(GuiGraphics guiGraphics) {
if (!menu.view().itemsChanged()) {
return;
}
boolean blinkOn = ((System.currentTimeMillis() / 300L) & 1L) == 0L;
int color = blinkOn ? 0xFFFF4040 : 0xFF7A1010;
for (Slot slot : menu.slots) {
if (slot instanceof SelfOfferSlot selfOfferSlot) {
if (menu.view().selfChangedSlots().get(selfOfferSlot.offerIndex())) {
renderChangedSlotWarning(guiGraphics, slot, color);
}
} else if (slot instanceof OtherOfferSlot otherOfferSlot) {
if (menu.view().otherChangedSlots().get(otherOfferSlot.offerIndex())) {
renderChangedSlotWarning(guiGraphics, slot, color);
}
}
}
}
private void renderChangedSlotWarning(GuiGraphics guiGraphics, Slot slot, int color) {
int x = leftPos + slot.x;
int y = topPos + slot.y;
guiGraphics.fill(x - 1, y - 1, x + 17, y, color);
guiGraphics.fill(x - 1, y + 16, x + 17, y + 17, color);
guiGraphics.fill(x - 1, y, x, y + 16, color);
guiGraphics.fill(x + 16, y, x + 17, y + 16, color);
guiGraphics.drawString(font, "!", x + 11, y - 2, color, false);
}
private void sendAction(TradeAction action, int slot, int amount) {
if (minecraft == null || minecraft.getConnection() == null) {
return;
@@ -575,6 +610,10 @@ public class TradeScreen extends AbstractContainerScreen<TradeScreen.TradeMenu>
super(container, slot, x, y);
}
public int offerIndex() {
return getSlotIndex();
}
@Override
public boolean mayPickup(Player player) {
return false;

View File

@@ -260,7 +260,7 @@ public final class TradeCommand {
try {
if (!TradeManager.get(source.getServer()).setDebugOffer(player, DebugTradeSession.parseOfferSpec(spec))) {
source.sendFailure(Component.literal("Start a debug trade first with /trade debug init."));
source.sendFailure(Component.literal("Start a debug trade first with /trade debug init, and only change offers before confirmation."));
return 0;
}
} catch (IllegalArgumentException exception) {
@@ -285,6 +285,10 @@ public final class TradeCommand {
try {
int result = TradeManager.get(source.getServer()).removeDebugOffer(player, spec);
if (result < 0) {
if (result == -2) {
source.sendFailure(Component.literal("Trade offers cannot be changed during confirmation."));
return 0;
}
source.sendFailure(Component.literal("Start a debug trade first with /trade debug init."));
return 0;
}

View File

@@ -7,6 +7,8 @@ public final class TradeConfig {
private static final ModConfigSpec.IntValue TRADE_COMMAND_PROXIMITY;
private static final ModConfigSpec.IntValue REQUEST_TIMEOUT_SECONDS;
private static final ModConfigSpec.BooleanValue ENABLE_DEBUG_FEATURES;
private static final ModConfigSpec.BooleanValue REQUIRE_SECOND_CONFIRMATION;
private static final ModConfigSpec.BooleanValue SHOW_TRADE_MODIFIED_WARNINGS;
private static final ModConfigSpec.BooleanValue REQUIRE_ON_GROUND;
private static final ModConfigSpec.BooleanValue REQUIRE_STATIONARY;
private static final ModConfigSpec.DoubleValue STATIONARY_SPEED_THRESHOLD;
@@ -30,6 +32,10 @@ public final class TradeConfig {
.defineInRange("requestTimeoutSeconds", 30, 1, 3600);
ENABLE_DEBUG_FEATURES = builder.comment("Enable debug trade commands and debug UI/testing tools.")
.define("enableDebugFeatures", false);
REQUIRE_SECOND_CONFIRMATION = builder.comment("Require a second confirmation step after both players accept the initial offer.")
.define("requireSecondConfirmation", true);
SHOW_TRADE_MODIFIED_WARNINGS = builder.comment("Show Trade Modified warnings and changed-slot highlights when offers are reduced or removed.")
.define("showTradeModifiedWarnings", true);
REQUIRE_ON_GROUND = builder.comment("Require players to be on solid ground before requesting or accepting a trade.")
.define("requireOnGround", true);
REQUIRE_STATIONARY = builder.comment("Require players to be stationary before requesting or accepting a trade.")
@@ -78,6 +84,14 @@ public final class TradeConfig {
}
}
public static boolean requireSecondConfirmation() {
return REQUIRE_SECOND_CONFIRMATION.get();
}
public static boolean showTradeModifiedWarnings() {
return SHOW_TRADE_MODIFIED_WARNINGS.get();
}
public static boolean requireOnGround() {
return REQUIRE_ON_GROUND.get();
}

View File

@@ -2,6 +2,7 @@ package com.trunksbomb.trade.trade;
import com.trunksbomb.trade.network.TradeClosePayload;
import com.trunksbomb.trade.network.TradeStatePayload;
import com.trunksbomb.trade.mod.TradeConfig;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
@@ -26,6 +27,8 @@ public class DebugTradeSession {
private final List<ItemStack> inventorySnapshot;
private final List<TradeEntry> selfOffer = blankOffer();
private final List<ItemStack> otherOffer = blankStacks();
private final List<Boolean> selfChangedSlots = blankChangedSlots();
private final List<Boolean> otherChangedSlots = blankChangedSlots();
private final EnumSet<DebugUnsafeState> unsafeStates = EnumSet.noneOf(DebugUnsafeState.class);
private boolean selfAccepted;
private boolean otherAccepted;
@@ -90,7 +93,7 @@ public class DebugTradeSession {
ItemStack merged = entry.stack().copy();
merged.grow(moveAmount);
selfOffer.set(i, new TradeEntry(inventorySlot, merged));
clearAccepts();
clearAccepts(false, false, -1);
return true;
}
}
@@ -101,7 +104,7 @@ public class DebugTradeSession {
}
selfOffer.set(freeSlot, new TradeEntry(inventorySlot, sourceStack.copyWithCount(moveAmount)));
clearAccepts();
clearAccepts(false, false, -1);
return true;
}
@@ -124,7 +127,7 @@ public class DebugTradeSession {
selfOffer.set(offerSlot, null);
}
clearAccepts();
clearAccepts(true, false, offerSlot);
return true;
}
@@ -140,7 +143,7 @@ public class DebugTradeSession {
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
otherOffer.set(i, i < offer.size() ? offer.get(i).copy() : ItemStack.EMPTY);
}
clearAccepts();
clearAccepts(false, false, -1);
}
public boolean appendOtherOffer(List<ItemStack> offer) {
@@ -156,18 +159,25 @@ public class DebugTradeSession {
}
if (changed) {
clearAccepts();
clearAccepts(false, false, -1);
}
return changed;
}
public boolean removeOtherOffer(String spec) {
if ("all".equalsIgnoreCase(spec)) {
boolean changed = false;
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
if (!otherOffer.get(i).isEmpty()) {
otherChangedSlots.set(i, true);
changed = true;
}
otherOffer.set(i, ItemStack.EMPTY);
}
clearAccepts();
return true;
if (changed) {
clearAccepts(false, false, -1);
}
return changed;
}
int split = spec.indexOf(':');
@@ -194,7 +204,7 @@ public class DebugTradeSession {
otherOffer.set(slot, updated);
}
clearAccepts();
clearAccepts(false, true, slot);
return true;
}
@@ -204,7 +214,7 @@ public class DebugTradeSession {
continue;
}
otherOffer.set(i, ItemStack.EMPTY);
clearAccepts();
clearAccepts(false, true, i);
return true;
}
return false;
@@ -263,6 +273,7 @@ public class DebugTradeSession {
}
public TradeView view() {
boolean showModifiedWarnings = TradeConfig.showTradeModifiedWarnings();
return new TradeView(
id,
player.getGameProfile().getName(),
@@ -271,10 +282,13 @@ public class DebugTradeSession {
stage,
selfAccepted,
otherAccepted,
showModifiedWarnings && hasChangedSlots(),
inventoryDisplay(),
emptyReservedSnapshot(),
selfOfferSnapshot(),
otherOfferSnapshot());
otherOfferSnapshot(),
showModifiedWarnings ? changedSlotSnapshot(selfChangedSlots) : blankChangedSlots(),
showModifiedWarnings ? changedSlotSnapshot(otherChangedSlots) : blankChangedSlots());
}
public static List<ItemStack> parseOfferSpec(String spec) {
@@ -418,9 +432,26 @@ public class DebugTradeSession {
return count;
}
private void clearAccepts() {
private void clearAccepts(boolean markSelfChanged, boolean markOtherChanged, int changedSlot) {
selfAccepted = false;
otherAccepted = false;
if (changedSlot >= 0) {
if (markSelfChanged) {
selfChangedSlots.set(changedSlot, true);
}
if (markOtherChanged) {
otherChangedSlots.set(changedSlot, true);
}
}
}
private boolean hasChangedSlots() {
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
if (selfChangedSlots.get(i) || otherChangedSlots.get(i)) {
return true;
}
}
return false;
}
private List<ItemStack> inventorySnapshotCopy() {
@@ -459,6 +490,18 @@ public class DebugTradeSession {
return result;
}
private static List<Boolean> blankChangedSlots() {
List<Boolean> result = new ArrayList<>(OFFER_SLOT_COUNT);
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
result.add(false);
}
return result;
}
private static List<Boolean> changedSlotSnapshot(List<Boolean> changedSlots) {
return new ArrayList<>(changedSlots);
}
private static List<ItemStack> inventorySnapshot(ServerPlayer player) {
List<ItemStack> result = new ArrayList<>(INVENTORY_SLOT_COUNT);
Inventory inventory = player.getInventory();

View File

@@ -233,15 +233,17 @@ public class TradeManager {
return;
}
if (!session.isConfirmationStage()) {
session.advanceToConfirmation();
if (!session.bothAccepted()) {
session.syncToPlayers();
return;
}
if (!session.bothAccepted()) {
session.syncToPlayers();
return;
if (!session.isConfirmationStage()) {
if (TradeConfig.requireSecondConfirmation()) {
session.advanceToConfirmation();
session.syncToPlayers();
return;
}
}
TradeSession.CompletionResult completion = session.completeTrade();
@@ -278,15 +280,17 @@ public class TradeManager {
return;
}
if (!session.isConfirmationStage()) {
session.advanceToConfirmation();
if (!session.bothAccepted()) {
session.sync();
return;
}
if (!session.bothAccepted()) {
session.sync();
return;
if (!session.isConfirmationStage()) {
if (TradeConfig.requireSecondConfirmation()) {
session.advanceToConfirmation();
session.sync();
return;
}
}
if (session.completeTrade()) {
@@ -404,7 +408,7 @@ public class TradeManager {
return false;
}
DebugTradeSession session = getDebugSession(player);
if (session == null) {
if (session == null || session.isConfirmationStage()) {
return false;
}
@@ -421,6 +425,9 @@ public class TradeManager {
if (session == null) {
return -1;
}
if (session.isConfirmationStage()) {
return -2;
}
boolean changed = session.removeOtherOffer(spec);
if (changed) {
@@ -447,20 +454,25 @@ public class TradeManager {
}
session.acceptOther();
if (!session.isConfirmationStage()) {
session.advanceToConfirmation();
if (!session.bothAccepted()) {
session.sync();
return true;
}
if (!session.isConfirmationStage()) {
if (TradeConfig.requireSecondConfirmation()) {
session.advanceToConfirmation();
session.sync();
return true;
}
}
if (session.bothAccepted()) {
if (session.completeTrade()) {
finishDebug(session, Component.literal("Debug trade completed."));
} else {
closeDebug(session, Component.literal("Debug trade cancelled because the items would not fit."));
}
} else {
session.sync();
}
return true;
}
@@ -531,20 +543,36 @@ public class TradeManager {
try {
switch (action) {
case SET_OFFER -> {
if (session.isConfirmationStage()) {
player.sendSystemMessage(Component.literal("Trade offers cannot be changed during confirmation."));
return;
}
session.setOtherOffer(DebugTradeSession.parseOfferSpec(spec));
session.sync();
}
case APPEND_RANDOM -> {
if (session.isConfirmationStage()) {
player.sendSystemMessage(Component.literal("Trade offers cannot be changed during confirmation."));
return;
}
if (session.appendOtherOffer(DebugTradeSession.randomSingleStackOffer())) {
session.sync();
}
}
case REMOVE_OFFER -> {
if (session.isConfirmationStage()) {
player.sendSystemMessage(Component.literal("Trade offers cannot be changed during confirmation."));
return;
}
if (session.removeOtherOffer(spec)) {
session.sync();
}
}
case REMOVE_LAST -> {
if (session.isConfirmationStage()) {
player.sendSystemMessage(Component.literal("Trade offers cannot be changed during confirmation."));
return;
}
if (session.removeLastOtherOffer()) {
session.sync();
}

View File

@@ -1,5 +1,6 @@
package com.trunksbomb.trade.trade;
import com.trunksbomb.trade.mod.TradeConfig;
import com.trunksbomb.trade.network.TradeClosePayload;
import com.trunksbomb.trade.network.TradeStatePayload;
import java.util.ArrayList;
@@ -10,7 +11,6 @@ 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;
@@ -23,6 +23,8 @@ public class TradeSession {
private final List<ItemStack> secondInventory;
private final List<TradeEntry> firstOffer = blankOffer();
private final List<TradeEntry> secondOffer = blankOffer();
private final List<Boolean> firstChangedSlots = blankChangedSlots();
private final List<Boolean> secondChangedSlots = blankChangedSlots();
private boolean firstAccepted;
private boolean secondAccepted;
private TradeStage stage = TradeStage.OFFERING;
@@ -95,7 +97,7 @@ public class TradeSession {
ItemStack merged = entry.stack().copy();
merged.grow(moveAmount);
offer.set(i, new TradeEntry(inventorySlot, merged));
clearAccepts();
clearAccepts(false, false, -1);
return true;
}
}
@@ -106,7 +108,7 @@ public class TradeSession {
}
offer.set(freeSlot, new TradeEntry(inventorySlot, sourceStack.copyWithCount(moveAmount)));
clearAccepts();
clearAccepts(false, false, -1);
return true;
}
@@ -130,7 +132,7 @@ public class TradeSession {
offer.set(offerSlot, null);
}
clearAccepts();
clearAccepts(player == first, player == second, offerSlot);
return true;
}
@@ -193,6 +195,7 @@ public class TradeSession {
public TradeView viewFor(ServerPlayer player) {
boolean isFirst = player == first;
ServerPlayer other = isFirst ? second : first;
boolean showModifiedWarnings = TradeConfig.showTradeModifiedWarnings();
return new TradeView(
id,
player.getGameProfile().getName(),
@@ -201,10 +204,13 @@ public class TradeSession {
stage,
isFirst ? firstAccepted : secondAccepted,
isFirst ? secondAccepted : firstAccepted,
showModifiedWarnings && hasChangedSlots(),
inventoryDisplayFor(player),
emptyReservedSnapshot(),
offerSnapshot(isFirst ? firstOffer : secondOffer),
offerSnapshot(isFirst ? secondOffer : firstOffer));
offerSnapshot(isFirst ? secondOffer : firstOffer),
showModifiedWarnings ? changedSlotSnapshot(isFirst ? firstChangedSlots : secondChangedSlots) : blankChangedSlots(),
showModifiedWarnings ? changedSlotSnapshot(isFirst ? secondChangedSlots : firstChangedSlots) : blankChangedSlots());
}
public void sendState(ServerPlayer player) {
@@ -318,9 +324,26 @@ public class TradeSession {
return -1;
}
private void clearAccepts() {
private void clearAccepts(boolean markFirstChanged, boolean markSecondChanged, int changedSlot) {
firstAccepted = false;
secondAccepted = false;
if (changedSlot >= 0) {
if (markFirstChanged) {
firstChangedSlots.set(changedSlot, true);
}
if (markSecondChanged) {
secondChangedSlots.set(changedSlot, true);
}
}
}
private boolean hasChangedSlots() {
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
if (firstChangedSlots.get(i) || secondChangedSlots.get(i)) {
return true;
}
}
return false;
}
private int reservedCount(ServerPlayer player, int inventorySlot) {
@@ -377,6 +400,18 @@ public class TradeSession {
return result;
}
private static List<Boolean> blankChangedSlots() {
List<Boolean> result = new ArrayList<>(OFFER_SLOT_COUNT);
for (int i = 0; i < OFFER_SLOT_COUNT; i++) {
result.add(false);
}
return result;
}
private static List<Boolean> changedSlotSnapshot(List<Boolean> changedSlots) {
return new ArrayList<>(changedSlots);
}
private static List<ItemStack> inventorySnapshot(ServerPlayer player) {
List<ItemStack> result = new ArrayList<>(INVENTORY_SLOT_COUNT);
Inventory inventory = player.getInventory();

View File

@@ -15,10 +15,13 @@ public record TradeView(
TradeStage stage,
boolean selfAccepted,
boolean otherAccepted,
boolean itemsChanged,
List<ItemStack> inventory,
List<Integer> reservedCounts,
List<ItemStack> selfOffer,
List<ItemStack> otherOffer) {
List<ItemStack> otherOffer,
List<Boolean> selfChangedSlots,
List<Boolean> otherChangedSlots) {
public static final int INVENTORY_SLOT_COUNT = 36;
public static final int OFFER_SLOT_COUNT = 36;
@@ -33,10 +36,13 @@ public record TradeView(
buf.writeEnum(value.stage);
buf.writeBoolean(value.selfAccepted);
buf.writeBoolean(value.otherAccepted);
buf.writeBoolean(value.itemsChanged);
writeStacks(buf, value.inventory, INVENTORY_SLOT_COUNT);
writeInts(buf, value.reservedCounts, INVENTORY_SLOT_COUNT);
writeStacks(buf, value.selfOffer, OFFER_SLOT_COUNT);
writeStacks(buf, value.otherOffer, OFFER_SLOT_COUNT);
writeBooleans(buf, value.selfChangedSlots, OFFER_SLOT_COUNT);
writeBooleans(buf, value.otherChangedSlots, OFFER_SLOT_COUNT);
}
@Override
@@ -49,10 +55,13 @@ public record TradeView(
buf.readEnum(TradeStage.class),
buf.readBoolean(),
buf.readBoolean(),
buf.readBoolean(),
readStacks(buf, INVENTORY_SLOT_COUNT),
readInts(buf, INVENTORY_SLOT_COUNT),
readStacks(buf, OFFER_SLOT_COUNT),
readStacks(buf, OFFER_SLOT_COUNT));
readStacks(buf, OFFER_SLOT_COUNT),
readBooleans(buf, OFFER_SLOT_COUNT),
readBooleans(buf, OFFER_SLOT_COUNT));
}
};
@@ -83,4 +92,18 @@ public record TradeView(
}
return values;
}
private static void writeBooleans(RegistryFriendlyByteBuf buf, List<Boolean> values, int expectedSize) {
for (int i = 0; i < expectedSize; i++) {
buf.writeBoolean(values.get(i));
}
}
private static List<Boolean> readBooleans(RegistryFriendlyByteBuf buf, int expectedSize) {
List<Boolean> values = new ArrayList<>(expectedSize);
for (int i = 0; i < expectedSize; i++) {
values.add(buf.readBoolean());
}
return values;
}
}