fix: a changed trade offer takes both accepts back

A player could change their offer after the other player gave the final
accept, and the trade completed with the new offer on their own accept.
Both clients drop their accepts when an offer changes, the one that
changed it and the one that receives the update, so the trade now does
the same and both players have to accept the offers as they are.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This commit is contained in:
Aaron Kimbrell
2026-11-10 15:39:43 -06:00
parent 6f7fd5f84e
commit 24294561ae
4 changed files with 66 additions and 0 deletions

View File

@@ -160,6 +160,17 @@ void Trade::SetAccepted(LWOOBJID participant, bool value) {
}
}
bool Trade::GetAccepted(const LWOOBJID participant) const {
if (participant == m_ParticipantA) return m_AcceptedA;
if (participant == m_ParticipantB) return m_AcceptedB;
return false;
}
void Trade::ResetAccepted() {
m_AcceptedA = false;
m_AcceptedB = false;
}
void Trade::Complete() {
auto* entityA = GetParticipantAEntity();
auto* entityB = GetParticipantBEntity();

View File

@@ -66,6 +66,16 @@ public:
void SetItems(LWOOBJID participant, std::vector<TradeItem> items);
void SetAccepted(LWOOBJID participant, bool value);
/**
* Whether a participant has given their final accept
*/
bool GetAccepted(LWOOBJID participant) const;
/**
* Takes back both final accepts. A changed offer does this, as both clients do when one arrives.
*/
void ResetAccepted();
void Complete();
void Cancel(const LWOOBJID canceller);

View File

@@ -256,6 +256,8 @@ namespace GameMessages {
trade->SetCoins(entity.GetObjectID(), i64Currency);
trade->SetItems(entity.GetObjectID(), *resolvedItems);
// Both clients take their accept back when an offer changes, so an accept only ever stands for the offers as they are
trade->ResetAccepted();
trade->SendUpdateToOther(entity.GetObjectID());
}

View File

@@ -2,6 +2,7 @@
#include <map>
#include "GameDependencies.h"
#include "TradingManager.h"
namespace {
@@ -12,6 +13,17 @@ namespace {
{ 400, { 1727, 2, false, true } },
};
class TradeAcceptTest : public GameDependenciesTest {
protected:
void SetUp() override {
SetUpDependencies();
}
void TearDown() override {
TearDownDependencies();
}
};
std::optional<HeldTradeItem> FindHeld(const LWOOBJID itemId) {
const auto it = g_Held.find(itemId);
if (it == g_Held.end()) return std::nullopt;
@@ -80,3 +92,34 @@ TEST(TradeOfferTest, AnOfferFillsTheSlotsOfTheTradeWindowAtMost) {
offered.push_back({ Trade::MaxOfferedItems + 1, 1727, 1 });
EXPECT_FALSE(Trade::ResolveOffer(offered, findAny).has_value());
}
TEST_F(TradeAcceptTest, OnlyTheFinalAcceptCounts) {
Trade trade(1, 10, 20);
EXPECT_FALSE(trade.GetAccepted(10));
trade.SetAccepted(10, true);
EXPECT_FALSE(trade.GetAccepted(10));
trade.SetAccepted(10, false);
EXPECT_TRUE(trade.GetAccepted(10));
EXPECT_FALSE(trade.GetAccepted(20));
// Not a participant
EXPECT_FALSE(trade.GetAccepted(30));
}
TEST_F(TradeAcceptTest, AChangedOfferTakesBothAcceptsBack) {
Trade trade(1, 10, 20);
trade.SetAccepted(10, false);
ASSERT_TRUE(trade.GetAccepted(10));
// What the update handler does once the new offer is stored
trade.SetItems(20, { { 100, 1727, 1 } });
trade.ResetAccepted();
EXPECT_FALSE(trade.GetAccepted(10));
EXPECT_FALSE(trade.GetAccepted(20));
// The other player's accept alone does not complete the trade, both have to accept again
trade.SetAccepted(20, false);
EXPECT_TRUE(trade.GetAccepted(20));
EXPECT_FALSE(trade.GetAccepted(10));
}