diff --git a/dGame/TradingManager.cpp b/dGame/TradingManager.cpp index a8423d9a8..a125cb453 100644 --- a/dGame/TradingManager.cpp +++ b/dGame/TradingManager.cpp @@ -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(); diff --git a/dGame/TradingManager.h b/dGame/TradingManager.h index fa427e3ea..d96a02a34 100644 --- a/dGame/TradingManager.h +++ b/dGame/TradingManager.h @@ -66,6 +66,16 @@ public: void SetItems(LWOOBJID participant, std::vector 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); diff --git a/dGame/dGameMessages/TradeMessages.cpp b/dGame/dGameMessages/TradeMessages.cpp index 046c4f2c9..bf7c5d34b 100644 --- a/dGame/dGameMessages/TradeMessages.cpp +++ b/dGame/dGameMessages/TradeMessages.cpp @@ -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()); } diff --git a/tests/dGameTests/dGameMessagesTests/TradeOfferTests.cpp b/tests/dGameTests/dGameMessagesTests/TradeOfferTests.cpp index d6b53be3f..17c9de852 100644 --- a/tests/dGameTests/dGameMessagesTests/TradeOfferTests.cpp +++ b/tests/dGameTests/dGameMessagesTests/TradeOfferTests.cpp @@ -2,6 +2,7 @@ #include +#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 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)); +}