Skip to content

Commit ee01247

Browse files
committed
Make trade commit atomic instead of dropping undeliverable items
trade_tradecommit() transferred each staged item independently: it called pc_additem() per item and, on failure, handed that one item back to the giver and carried on, still completing the trade. The staging path (trade_tradeadditem) validates the receiver's weight and free slots but never the per-item MAX_AMOUNT stack cap, so a deal that pushes a stack past 30000 was accepted into the window, then silently lost that item at commit while the rest of the trade went through. Pre-validate both directions before touching any inventory: mirror the npc_buylist() checks (per-item pc_checkadditem for the stack cap, plus cumulative weight and free-slot counts) and cancel the entire trade if either side cannot receive everything the other staged. Nothing transfers unless all of it can, so partners keep exactly what they had on a failed deal rather than losing items to a partial commit. Tell both players why the trade was canceled: the client ignores the fail field on the trade-complete packet (0x00f0), so send the reason as a plain message before cancelling. The blocked player is told they can't hold everything offered; their partner is told by name. The per-item give-back branch in the commit loop is now unreachable on the cap path but is left in place as a defensive fallback.
1 parent 54ff3dc commit ee01247

1 file changed

Lines changed: 65 additions & 0 deletions

File tree

src/map/trade.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,48 @@ void trade_tradecancel(dumb_ptr<map_session_data> sd)
351351
}
352352
}
353353

354+
// Would `receiver` be able to hold everything `giver` has staged in the trade
355+
// window? Checks without mutating anything, mirroring the up-front validation
356+
// npc_buylist() does: accumulate the added weight and the number of fresh
357+
// inventory slots required, and refuse if any stack would pass MAX_AMOUNT, the
358+
// receiver would end up overweight, or there aren't enough free slots. This
359+
// lets trade_tradecommit() stay all-or-nothing rather than committing the
360+
// trade while silently dropping the items pc_additem() can't place.
361+
static
362+
bool trade_can_receive(dumb_ptr<map_session_data> giver,
363+
dumb_ptr<map_session_data> receiver)
364+
{
365+
int new_stacks = 0;
366+
int weight = 0;
367+
for (int trade_i = 0; trade_i < TRADE_MAX; trade_i++)
368+
{
369+
if (giver->deal_item_amount[trade_i] == 0)
370+
continue;
371+
IOff0 n = giver->deal_item_index[trade_i].unshift();
372+
ItemNameId nameid = giver->status.inventory[n].nameid;
373+
int amount = giver->deal_item_amount[trade_i];
374+
switch (pc_checkadditem(receiver, nameid, amount))
375+
{
376+
case ADDITEM::EXIST:
377+
break;
378+
case ADDITEM::NEW:
379+
if (itemdb_isequip(nameid))
380+
new_stacks += amount;
381+
else
382+
new_stacks++;
383+
break;
384+
case ADDITEM::OVERAMOUNT:
385+
return false;
386+
}
387+
weight += itemdb_weight(nameid) * amount;
388+
}
389+
if (weight + receiver->weight > receiver->max_weight)
390+
return false;
391+
if (pc_inventoryblank(receiver) < new_stacks)
392+
return false;
393+
return true;
394+
}
395+
354396
/*==========================================
355397
* 取引許諾(trade押し)
356398
*------------------------------------------
@@ -389,6 +431,29 @@ void trade_tradecommit(dumb_ptr<map_session_data> sd)
389431
MAP_LOG_PC(sd, " TRADECANCEL"_fmt);
390432
return;
391433
}
434+
// Commit is all-or-nothing: if either side couldn't receive
435+
// everything the other staged (a stack would pass MAX_AMOUNT,
436+
// overweight, or no free slot), cancel the whole trade instead
437+
// of transferring part of it and dropping the rest. The client
438+
// ignores the fail field on the trade-complete packet, so the
439+
// reason has to be sent as a plain message before we cancel.
440+
dumb_ptr<map_session_data> full = nullptr;
441+
if (!trade_can_receive(sd, target_sd))
442+
full = target_sd;
443+
else if (!trade_can_receive(target_sd, sd))
444+
full = sd;
445+
if (full != nullptr)
446+
{
447+
dumb_ptr<map_session_data> other = (full == sd) ? target_sd : sd;
448+
clif_displaymessage(full->sess,
449+
"Trade canceled: you can't hold all of the offered items."_s);
450+
clif_displaymessage(other->sess, STRPRINTF(
451+
"Trade canceled: %s can't hold all of the offered items."_fmt,
452+
full->status_key.name));
453+
trade_tradecancel(sd);
454+
MAP_LOG_PC(sd, " TRADECANCEL"_fmt);
455+
return;
456+
}
392457
sd->trade_partner = AccountId();
393458
target_sd->trade_partner = AccountId();
394459
for (trade_i = 0; trade_i < TRADE_MAX; trade_i++)

0 commit comments

Comments
 (0)