From 21295659bb3fab1cf5e9d325937e56dce03bac3d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 22:52:21 +0000 Subject: [PATCH 1/3] fix: recreate traded poisons with AddPoison and show crafted potion temp IDs Poisons are recreated through the engine's AddPoison call instead of AddPotion, effects keep the sender's order (recipe comparison is now order-insensitive), and the trade UI lists each crafted potion's temporary ID alongside its effects. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SSW5HGXzi9kAVm4zdsSaid --- Code/client/Games/Skyrim/Forms/AlchemyItem.cpp | 17 +++++------------ Code/client/Services/Generic/TradeService.cpp | 16 +++++++++++++--- Code/encoding/Structs/Inventory.cpp | 7 +++++++ Code/encoding/Structs/Inventory.h | 3 ++- Code/tests/trading.cpp | 15 +++++++++++++++ PLAYER_TRADING_POTIONS.md | 4 ++-- 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp b/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp index 4027950b5..e5e3c3a11 100644 --- a/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp +++ b/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp @@ -3,8 +3,6 @@ #include #include #include -#include -#include void AlchemyItem::Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept { @@ -31,14 +29,7 @@ void AlchemyItem::Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept aEntry.Potion.Effects.push_back(result); } if (!aEntry.Potion.IsValid()) - { aEntry.Potion.Effects.clear(); - return; - } - std::sort(aEntry.Potion.Effects.begin(), aEntry.Potion.Effects.end(), [](const auto& a, const auto& b) { - return std::tie(a.EffectId.ModId, a.EffectId.BaseId, a.Magnitude, a.Area, a.Duration, a.RawCost) < - std::tie(b.EffectId.ModId, b.EffectId.BaseId, b.Magnitude, b.Area, b.Duration, b.RawCost); - }); } AlchemyItem* AlchemyItem::Find(TESObjectREFR* apOwner, const Inventory::PotionData& aData) noexcept @@ -82,12 +73,14 @@ AlchemyItem* AlchemyItem::Create(const Inventory::PotionData& aData) noexcept } effects[i] = effect; } - // CommonLibSSE AddPotion: output is a one-pointer CreatedObjPtr, not a raw return value. - // AE address-library ID 36167; no AlchemyMenu, recipe, ingredients or skill calculation. + // CommonLibSSE AddPotion/AddPoison: output is a one-pointer created-object smart pointer. + // AE address-library IDs 36167 (potion) and 36168 (poison), the same calls the alchemy menu ends with. + // No AlchemyMenu, recipe, ingredient or skill calculation is involved. TP_THIS_FUNCTION(TAddPotion, void, BGSCreatedObjectManager, AlchemyItem**, GameArray*); POINTER_SKYRIMSE(TAddPotion, addPotion, 36167); + POINTER_SKYRIMSE(TAddPotion, addPoison, 36168); AlchemyItem* created = nullptr; - TiltedPhoques::ThisCall(addPotion, manager, &created, &effects); + TiltedPhoques::ThisCall(aData.IsPoison ? addPoison : addPotion, manager, &created, &effects); Memory::Free(effects.data); if (created) { diff --git a/Code/client/Services/Generic/TradeService.cpp b/Code/client/Services/Generic/TradeService.cpp index b84d7527a..9c1159fa9 100644 --- a/Code/client/Services/Generic/TradeService.cpp +++ b/Code/client/Services/Generic/TradeService.cpp @@ -400,10 +400,20 @@ void TradeService::EmitStateToUI() const noexcept auto details = CefListValue::Create(); int detailIndex = 0; - for (const auto& effect : entry.Potion.Effects) + if (!entry.Potion.Effects.empty()) { - auto* form = TESForm::GetById(modSystem.GetGameId(effect.EffectId)); - details->SetString(detailIndex++, fmt::format("{}: {} / {}s", form && form->GetName() ? form->GetName() : "Unknown effect", effect.Magnitude, effect.Duration)); + // Temporary IDs belong to the owner's game session; the recipient gets a fresh one from AddPotion. + details->SetString(detailIndex++, fmt::format("Temporary ID: {:08X}", 0xFF000000u | (entry.BaseId.BaseId & 0x00FFFFFFu))); + for (const auto& effect : entry.Potion.Effects) + { + auto* form = TESForm::GetById(modSystem.GetGameId(effect.EffectId)); + std::string text = fmt::format("{}: {:.0f}", form && form->GetName() ? form->GetName() : "Unknown effect", effect.Magnitude); + if (effect.Area > 0) + text += fmt::format(", {} ft", effect.Area); + if (effect.Duration > 0) + text += fmt::format(" for {}s", effect.Duration); + details->SetString(detailIndex++, text); + } } if (entry.ExtraHealth > 1.0f) details->SetString(detailIndex++, fmt::format("Improvement: {:.0f}%", entry.ExtraHealth * 100.0f)); diff --git a/Code/encoding/Structs/Inventory.cpp b/Code/encoding/Structs/Inventory.cpp index f373cbac6..8c1d8b425 100644 --- a/Code/encoding/Structs/Inventory.cpp +++ b/Code/encoding/Structs/Inventory.cpp @@ -1,9 +1,16 @@ #include #include +#include #include using TiltedPhoques::Serialization; +bool Inventory::PotionData::operator==(const PotionData& acRhs) const noexcept +{ + return IsPoison == acRhs.IsPoison && Effects.size() == acRhs.Effects.size() && + std::is_permutation(Effects.begin(), Effects.end(), acRhs.Effects.begin()); +} + bool Inventory::PotionData::IsValid() const noexcept { if (Effects.empty() || Effects.size() > 32) diff --git a/Code/encoding/Structs/Inventory.h b/Code/encoding/Structs/Inventory.h index e1f42ccbc..c43b49908 100644 --- a/Code/encoding/Structs/Inventory.h +++ b/Code/encoding/Structs/Inventory.h @@ -30,8 +30,9 @@ struct Inventory struct PotionData { bool IsPoison{}; + // Engine order is kept so recreation matches the original potion; comparison ignores order. Vector Effects{}; - bool operator==(const PotionData&) const noexcept = default; + bool operator==(const PotionData& acRhs) const noexcept; bool IsValid() const noexcept; }; diff --git a/Code/tests/trading.cpp b/Code/tests/trading.cpp index 73c2e5c49..eed75e570 100644 --- a/Code/tests/trading.cpp +++ b/Code/tests/trading.cpp @@ -81,6 +81,21 @@ TEST_CASE("Crafted potion trades identify recipes independently of temporary for } } +TEST_CASE("Crafted potion recipes compare independently of effect order", "[trading][alchemy]") +{ + auto first = CraftedPotion(); + first.Potion.Effects.push_back(first.Potion.Effects[0]); + first.Potion.Effects[1].EffectId.BaseId += 1; + first.Potion.Effects[1].Magnitude = 5.0f; + auto second = first; + std::swap(second.Potion.Effects[0], second.Potion.Effects[1]); + REQUIRE(first.Potion == second.Potion); + REQUIRE(SameTradeItem(first, second)); + + second.Potion.Effects[0] = second.Potion.Effects[1]; + REQUIRE_FALSE(first.Potion == second.Potion); +} + TEST_CASE("Potion readiness proofs must match the complete incoming offer", "[trading][alchemy]") { auto potion = CraftedPotion(); diff --git a/PLAYER_TRADING_POTIONS.md b/PLAYER_TRADING_POTIONS.md index 4f6dbbea2..ad1f8bc19 100644 --- a/PLAYER_TRADING_POTIONS.md +++ b/PLAYER_TRADING_POTIONS.md @@ -4,13 +4,13 @@ Build both clients and the server from the same commit on `codex/player-trading- 1. Craft a single-effect potion and a potion with multiple effects. Record each effect's magnitude and duration and the stack counts. 2. Give the recipient different Alchemy skills/perks and record their ingredients and Alchemy experience. -3. Offer part of each stack. Verify the offer lists show the actual potion name and effects, and complete the trade. +3. Offer part of each stack. Verify the offer lists show the actual potion name, the owner's temporary ID (FFxxxxxx) and each effect's magnitude and duration, and complete the trade. 4. Check that exactly the offered quantity left the sender and arrived at the recipient. Effects must match; ingredients and Alchemy experience must stay unchanged. 5. Drink one received potion, trade another back, and verify subsequent stack counts. Save/reload and verify the remaining potion still works. 6. Repeat with a crafted poison, with both players offering potions, and with an existing matching potion stack on the recipient. 7. Cancel Ready, change quantities, cancel the trade and disconnect during the countdown. Preparation must not add potions to inventory or consume anything. 8. Trade a normal cooked meal and a normal purchased potion to verify the existing static-form path. -Recreation uses Skyrim's `BGSCreatedObjectManager::AddPotion` with the captured effects directly. It does not invoke the crafting menu or calculate strength from the recipient's skills. The Ready request confirms successful reconstruction; changing an offer invalidates readiness. Unsupported temporary effects and effect conditions are excluded from trading. Custom per-instance names remain local inventory UI metadata. +Recreation uses Skyrim's `BGSCreatedObjectManager::AddPotion` (or `AddPoison` for poisons) with the captured effects directly, in the sender's effect order. It does not invoke the crafting menu or calculate strength from the recipient's skills. The Ready request confirms successful reconstruction; changing an offer invalidates readiness. Unsupported temporary effects and effect conditions are excluded from trading. Custom per-instance names remain local inventory UI metadata. Native engine behavior, including the AddPotion address and created-object reference lifetime, still requires this in-game test. Passing compilation and packet tests does not establish in-game correctness. From 8343e6f7dd833fcb5abc65e7e291bbe0ef738cb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 23:12:03 +0000 Subject: [PATCH 2/3] feat: trade only the partner items each recipient can resolve Each client hides partner items whose plugin forms or crafted effects it cannot load, and its Ready request lists the partner items it accepts. The server transfers only accepted items; the rest stay with their owner, and senders see which of their offered items the partner cannot receive. Crafted enchanted gear trades when its enchantment effects resolve. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SSW5HGXzi9kAVm4zdsSaid --- Code/client/Services/Generic/TradeService.cpp | 88 +++++++++++++++---- Code/client/Services/TradeService.h | 1 + Code/encoding/Messages/NotifyTradeState.cpp | 13 +++ Code/encoding/Messages/NotifyTradeState.h | 2 + .../Messages/TradeSetReadyRequest.cpp | 8 +- Code/encoding/Messages/TradeSetReadyRequest.h | 2 +- Code/encoding/Structs/TradeValidation.h | 30 +++++-- Code/server/Services/TradeService.cpp | 27 ++++-- Code/server/Services/TradeService.h | 3 +- Code/tests/trading.cpp | 70 ++++++++++++--- PLAYER_TRADING_POTIONS.md | 6 ++ 11 files changed, 201 insertions(+), 49 deletions(-) diff --git a/Code/client/Services/Generic/TradeService.cpp b/Code/client/Services/Generic/TradeService.cpp index 9c1159fa9..e1c6cc562 100644 --- a/Code/client/Services/Generic/TradeService.cpp +++ b/Code/client/Services/Generic/TradeService.cpp @@ -73,6 +73,47 @@ const char* GetItemCategory(TESForm* apForm) noexcept } } +bool EffectsResolve(ModSystem& aModSystem, const Vector& acEffects) noexcept +{ + for (const auto& effect : acEffects) + if (!Cast(TESForm::GetById(aModSystem.GetGameId(effect.EffectId)))) + return false; + return true; +} + +// Whether this game can rebuild a partner's item: every plugin form it references must be loaded, +// and temporary forms must carry data to recreate them here. +bool CanReceiveTradeItem(ModSystem& aModSystem, const Inventory::Entry& acItem) noexcept +{ + if (!acItem.Potion.Effects.empty()) + { + if (!acItem.Potion.IsValid() || !EffectsResolve(aModSystem, acItem.Potion.Effects)) + return false; + } + else if (acItem.BaseId.ModId == 0xFFFFFFFF || !Cast(TESForm::GetById(aModSystem.GetGameId(acItem.BaseId)))) + return false; + + if (acItem.ExtraEnchantId != 0) + { + if (acItem.ExtraEnchantId.ModId == 0xFFFFFFFF) + { + // Player-crafted enchantment: recreated from its effects on the recipient. + if (acItem.EnchantData.Effects.empty() || !EffectsResolve(aModSystem, acItem.EnchantData.Effects)) + return false; + } + else if (!TESForm::GetById(aModSystem.GetGameId(acItem.ExtraEnchantId))) + return false; + } + + if (acItem.ExtraPoisonId != 0) + { + // Applied crafted poisons carry no recipe in the inventory entry. + if (acItem.ExtraPoisonId.ModId == 0xFFFFFFFF || !Cast(TESForm::GetById(aModSystem.GetGameId(acItem.ExtraPoisonId)))) + return false; + } + return true; +} + std::string MakeDisplayName(ModSystem& aModSystem, const Inventory::Entry& aEntry) { if (!aEntry.Potion.Effects.empty()) @@ -193,23 +234,27 @@ void TradeService::SetReady(bool aReady) noexcept CancelTrade(); return; } - // Keep existing prepared forms alive through cancellation races until the session ends. + // Accept only partner items this game can rebuild; the rest stay with the partner. + // Keep prepared potion forms alive through cancellation races until the session ends. + auto& modSystem = m_world.GetModSystem(); for (const auto& item : m_session.PartnerItems) { - if (item.Potion.Effects.empty()) + if (!CanReceiveTradeItem(modSystem, item)) continue; - auto* created = AlchemyItem::Create(item.Potion); - if (!created) + if (!item.Potion.Effects.empty()) { - spdlog::error("[TradeService]: Unable to prepare incoming crafted potion; cancelling before inventory changes"); - CancelTrade(); - return; + auto* created = AlchemyItem::Create(item.Potion); + if (!created) + { + spdlog::error("[TradeService]: Unable to prepare incoming crafted potion; leaving it with the partner"); + continue; + } + if (std::find(m_preparedPotions.begin(), m_preparedPotions.end(), created) == m_preparedPotions.end()) + m_preparedPotions.push_back(created); + else + AlchemyItem::Release(created); } - if (std::find(m_preparedPotions.begin(), m_preparedPotions.end(), created) == m_preparedPotions.end()) - m_preparedPotions.push_back(created); - else - AlchemyItem::Release(created); - request.PreparedPotions.push_back(item); + request.AcceptedItems.push_back(item); } } m_transport.Send(request); @@ -314,6 +359,7 @@ void TradeService::OnTradeStarted(const NotifyTradeStarted& acMessage) noexcept m_session.SelfItems.clear(); m_session.PartnerItems.clear(); m_session.SelfInventory.clear(); + m_session.SelfAcceptedItems.clear(); m_session.CountdownMs = 0; m_session.CountdownTotalMs = 0; @@ -336,6 +382,7 @@ void TradeService::OnTradeState(const NotifyTradeState& acMessage) noexcept m_session.SelfItems = acMessage.SelfItems; m_session.PartnerItems = acMessage.PartnerItems; m_session.SelfInventory = acMessage.SelfInventory; + m_session.SelfAcceptedItems = acMessage.SelfAcceptedItems; m_session.CountdownMs = acMessage.CountdownMs; m_session.CountdownTotalMs = acMessage.CountdownTotalMs; @@ -386,7 +433,7 @@ void TradeService::EmitStateToUI() const noexcept auto& modSystem = m_world.GetModSystem(); - auto makeDict = [&](const Inventory::Entry& entry) { + auto makeDict = [&](const Inventory::Entry& entry, const char* apNote = nullptr) { auto dict = CefDictionaryValue::Create(); dict->SetInt("modId", entry.BaseId.ModId); dict->SetInt("baseId", entry.BaseId.BaseId); @@ -423,17 +470,21 @@ void TradeService::EmitStateToUI() const noexcept details->SetString(detailIndex++, "Poisoned"); if (entry.ExtraSoulLevel > 0) details->SetString(detailIndex++, fmt::format("Soul level: {}", entry.ExtraSoulLevel)); + if (apNote) + details->SetString(detailIndex++, apNote); dict->SetList("details", details); return dict; }; std::vector usedCounts(m_session.SelfInventory.size(), 0); + const auto selfAccepted = MatchAcceptedItems(m_session.SelfItems, m_session.SelfAcceptedItems); auto selfList = CefListValue::Create(); for (size_t i = 0; i < m_session.SelfItems.size(); ++i) { const auto& item = m_session.SelfItems[i]; - auto dict = makeDict(item); + const bool notReceivable = m_session.PartnerReady && !selfAccepted[i]; + auto dict = makeDict(item, notReceivable ? "Partner can't receive this item. It stays with you." : nullptr); dict->SetInt("count", std::abs(item.Count)); std::optional match; @@ -461,13 +512,16 @@ void TradeService::EmitStateToUI() const noexcept } pArgs->SetList(5, selfList); + // Partner items this game cannot rebuild are hidden; they are never accepted and stay with the partner. auto partnerList = CefListValue::Create(); - for (size_t i = 0; i < m_session.PartnerItems.size(); ++i) + int partnerIndex = 0; + for (const auto& item : m_session.PartnerItems) { - const auto& item = m_session.PartnerItems[i]; + if (!CanReceiveTradeItem(modSystem, item)) + continue; auto dict = makeDict(item); dict->SetInt("count", std::abs(item.Count)); - partnerList->SetDictionary(static_cast(i), dict); + partnerList->SetDictionary(partnerIndex++, dict); } pArgs->SetList(6, partnerList); diff --git a/Code/client/Services/TradeService.h b/Code/client/Services/TradeService.h index 816fff621..913c7bdd9 100644 --- a/Code/client/Services/TradeService.h +++ b/Code/client/Services/TradeService.h @@ -69,6 +69,7 @@ struct TradeService TiltedPhoques::Vector SelfItems; TiltedPhoques::Vector PartnerItems; TiltedPhoques::Vector SelfInventory; + TiltedPhoques::Vector SelfAcceptedItems; uint32_t CountdownMs{0}; uint32_t CountdownTotalMs{0}; }; diff --git a/Code/encoding/Messages/NotifyTradeState.cpp b/Code/encoding/Messages/NotifyTradeState.cpp index 34ed8dda2..8ce0fc35b 100644 --- a/Code/encoding/Messages/NotifyTradeState.cpp +++ b/Code/encoding/Messages/NotifyTradeState.cpp @@ -19,6 +19,10 @@ void NotifyTradeState::SerializeRaw(TiltedPhoques::Buffer::Writer& aWriter) cons Serialization::WriteVarInt(aWriter, SelfInventory.size()); for (const auto& item : SelfInventory) item.Serialize(aWriter); + + Serialization::WriteVarInt(aWriter, SelfAcceptedItems.size()); + for (const auto& item : SelfAcceptedItems) + item.Serialize(aWriter); } void NotifyTradeState::DeserializeRaw(TiltedPhoques::Buffer::Reader& aReader) noexcept @@ -57,4 +61,13 @@ void NotifyTradeState::DeserializeRaw(TiltedPhoques::Buffer::Reader& aReader) no entry.Deserialize(aReader); SelfInventory.push_back(entry); } + + auto acceptedCount = Serialization::ReadVarInt(aReader); + SelfAcceptedItems.reserve(acceptedCount); + for (size_t i = 0; i < acceptedCount; ++i) + { + Inventory::Entry entry; + entry.Deserialize(aReader); + SelfAcceptedItems.push_back(entry); + } } diff --git a/Code/encoding/Messages/NotifyTradeState.h b/Code/encoding/Messages/NotifyTradeState.h index e1fc6a989..5ade628b5 100644 --- a/Code/encoding/Messages/NotifyTradeState.h +++ b/Code/encoding/Messages/NotifyTradeState.h @@ -24,4 +24,6 @@ struct NotifyTradeState final : ServerMessage TiltedPhoques::Vector SelfItems; TiltedPhoques::Vector PartnerItems; TiltedPhoques::Vector SelfInventory; + // Own offered items the ready partner can receive; empty while the partner is not ready. + TiltedPhoques::Vector SelfAcceptedItems; }; diff --git a/Code/encoding/Messages/TradeSetReadyRequest.cpp b/Code/encoding/Messages/TradeSetReadyRequest.cpp index 93b12f948..bf12caa6c 100644 --- a/Code/encoding/Messages/TradeSetReadyRequest.cpp +++ b/Code/encoding/Messages/TradeSetReadyRequest.cpp @@ -3,8 +3,8 @@ void TradeSetReadyRequest::SerializeRaw(TiltedPhoques::Buffer::Writer& aWriter) const noexcept { Serialization::WriteBool(aWriter, Ready); - Serialization::WriteVarInt(aWriter, PreparedPotions.size()); - for (const auto& item : PreparedPotions) + Serialization::WriteVarInt(aWriter, AcceptedItems.size()); + for (const auto& item : AcceptedItems) item.Serialize(aWriter); } @@ -13,12 +13,12 @@ void TradeSetReadyRequest::DeserializeRaw(TiltedPhoques::Buffer::Reader& aReader ClientMessage::DeserializeRaw(aReader); Ready = Serialization::ReadBool(aReader); - PreparedPotions.clear(); + AcceptedItems.clear(); const auto count = Serialization::ReadVarInt(aReader); for (uint64_t i = 0; i < count; ++i) { Inventory::Entry item; item.Deserialize(aReader); - PreparedPotions.push_back(std::move(item)); + AcceptedItems.push_back(std::move(item)); } } diff --git a/Code/encoding/Messages/TradeSetReadyRequest.h b/Code/encoding/Messages/TradeSetReadyRequest.h index d757b3c4f..129bf0c17 100644 --- a/Code/encoding/Messages/TradeSetReadyRequest.h +++ b/Code/encoding/Messages/TradeSetReadyRequest.h @@ -16,5 +16,5 @@ struct TradeSetReadyRequest final : ClientMessage void DeserializeRaw(TiltedPhoques::Buffer::Reader& aReader) noexcept override; bool Ready{false}; - Vector PreparedPotions; + Vector AcceptedItems; }; diff --git a/Code/encoding/Structs/TradeValidation.h b/Code/encoding/Structs/TradeValidation.h index 0b6b68aea..54b0afe2d 100644 --- a/Code/encoding/Structs/TradeValidation.h +++ b/Code/encoding/Structs/TradeValidation.h @@ -27,19 +27,33 @@ inline bool SameTradeItem(const Inventory::Entry& left, const Inventory::Entry& return true; } -// A ready client must have recreated exactly the incoming crafted potions for this offer. -inline bool ValidatePreparedPotions(const Vector& acIncoming, const Vector& acPrepared) noexcept +// A ready client lists the partner items it can resolve and receive, in offer order. +// Items it leaves out stay with their owner. +inline bool ValidateAcceptedItems(const Vector& acIncoming, const Vector& acAccepted) noexcept { size_t index = 0; for (const auto& item : acIncoming) { - if (item.Potion.Effects.empty()) - continue; - if (index >= acPrepared.size() || acPrepared[index].Count != item.Count || !SameTradeItem(item, acPrepared[index])) - return false; - ++index; + if (index < acAccepted.size() && acAccepted[index].Count == item.Count && SameTradeItem(item, acAccepted[index])) + ++index; + } + return index == acAccepted.size(); +} + +// Marks which offered items appear in the partner's accepted list, matching in offer order. +inline Vector MatchAcceptedItems(const Vector& acOffer, const Vector& acAccepted) noexcept +{ + Vector result(acOffer.size(), false); + size_t index = 0; + for (size_t i = 0; i < acOffer.size(); ++i) + { + if (index < acAccepted.size() && acAccepted[index].Count == acOffer[i].Count && SameTradeItem(acOffer[i], acAccepted[index])) + { + result[i] = true; + ++index; + } } - return index == acPrepared.size(); + return result; } // Reserve quantities across the entire offer so repeated entries cannot reuse the same items. diff --git a/Code/server/Services/TradeService.cpp b/Code/server/Services/TradeService.cpp index f36593c20..af72c9708 100644 --- a/Code/server/Services/TradeService.cpp +++ b/Code/server/Services/TradeService.cpp @@ -266,6 +266,8 @@ void TradeService::OnTradeOfferUpdate(const PacketEvent pSession->Offers[cIndex].Items = std::move(sanitized); pSession->Offers[cIndex].Ready = false; pSession->Offers[1 - cIndex].Ready = false; + pSession->Offers[cIndex].AcceptedItems.clear(); + pSession->Offers[1 - cIndex].AcceptedItems.clear(); ResetCountdown(*pSession); spdlog::debug("[TradeService]: Updated offer for player {} in session {}", pPlayer->GetId(), pSession->Id); @@ -292,6 +294,7 @@ void TradeService::OnTradeSetReady(const PacketEvent& acPa if (!message.Ready) { pSession->Offers[cIndex].Ready = false; + pSession->Offers[cIndex].AcceptedItems.clear(); ResetCountdown(*pSession); SendStateUpdate(*pSession); return; @@ -304,12 +307,18 @@ void TradeService::OnTradeSetReady(const PacketEvent& acPa return; } - if (!ValidatePreparedPotions(pSession->Offers[1 - cIndex].Items, message.PreparedPotions)) + if (!ValidateAcceptedItems(pSession->Offers[1 - cIndex].Items, message.AcceptedItems)) { CancelSession(*pSession, TradeCancelReason::FailedValidation, pPlayer); return; } - pSession->Offers[cIndex].PreparedPotions = message.PreparedPotions; + // Keep the server's canonical copies of the accepted partner items, not the client's echo. + const auto& incoming = pSession->Offers[1 - cIndex].Items; + const auto accepted = MatchAcceptedItems(incoming, message.AcceptedItems); + pSession->Offers[cIndex].AcceptedItems.clear(); + for (size_t i = 0; i < incoming.size(); ++i) + if (accepted[i]) + pSession->Offers[cIndex].AcceptedItems.push_back(incoming[i]); pSession->Offers[cIndex].Ready = true; const bool cBothReady = pSession->Offers[0].Ready && pSession->Offers[1].Ready; @@ -454,6 +463,8 @@ void TradeService::SendStateUpdate(const TradeSession& aSession) const noexcept notify.PartnerItems = aSession.Offers[1 - i].Items; notify.CountdownTotalMs = total; notify.CountdownMs = remaining; + if (aSession.Offers[1 - i].Ready) + notify.SelfAcceptedItems = aSession.Offers[1 - i].AcceptedItems; notify.SelfInventory.clear(); if (auto* pInventory = GetInventoryFor(pPlayer)) notify.SelfInventory = pInventory->Entries; @@ -500,11 +511,13 @@ void TradeService::FinalizeTrade(TradeSession& aSession) noexcept auto* rightInventory = GetInventoryFor(sessionCopy.Players[1]); Inventory leftResult; Inventory rightResult; + // Only items the recipient could resolve change hands; the rest stay with their owner. + const auto& leftTransfer = sessionCopy.Offers[1].AcceptedItems; + const auto& rightTransfer = sessionCopy.Offers[0].AcceptedItems; if (!leftInventory || !rightInventory || - !ValidatePreparedPotions(sessionCopy.Offers[1].Items, sessionCopy.Offers[0].PreparedPotions) || - !ValidatePreparedPotions(sessionCopy.Offers[0].Items, sessionCopy.Offers[1].PreparedPotions) || - !PrepareTradeExchange(*leftInventory, *rightInventory, - sessionCopy.Offers[0].Items, sessionCopy.Offers[1].Items, leftResult, rightResult)) + !ValidateAcceptedItems(sessionCopy.Offers[0].Items, leftTransfer) || + !ValidateAcceptedItems(sessionCopy.Offers[1].Items, rightTransfer) || + !PrepareTradeExchange(*leftInventory, *rightInventory, leftTransfer, rightTransfer, leftResult, rightResult)) { CancelSession(aSession, TradeCancelReason::FailedValidation); return; @@ -519,7 +532,7 @@ void TradeService::FinalizeTrade(TradeSession& aSession) noexcept Player* recipient = sessionCopy.Players[1 - from]; const auto senderEntity = *sender->GetCharacter(); const auto recipientEntity = *recipient->GetCharacter(); - for (const auto& item : sessionCopy.Offers[from].Items) + for (const auto& item : sessionCopy.Offers[1 - from].AcceptedItems) { NotifyInventoryChanges removal; removal.ServerId = World::ToInteger(senderEntity); diff --git a/Code/server/Services/TradeService.h b/Code/server/Services/TradeService.h index f689e39f6..35ffd1e8b 100644 --- a/Code/server/Services/TradeService.h +++ b/Code/server/Services/TradeService.h @@ -46,7 +46,8 @@ class TradeService } TiltedPhoques::Vector Items; - TiltedPhoques::Vector PreparedPotions; + // Items of the partner's offer this player can resolve and receive. + TiltedPhoques::Vector AcceptedItems; bool Ready; }; diff --git a/Code/tests/trading.cpp b/Code/tests/trading.cpp index eed75e570..efd15ad26 100644 --- a/Code/tests/trading.cpp +++ b/Code/tests/trading.cpp @@ -96,20 +96,65 @@ TEST_CASE("Crafted potion recipes compare independently of effect order", "[trad REQUIRE_FALSE(first.Potion == second.Potion); } -TEST_CASE("Potion readiness proofs must match the complete incoming offer", "[trading][alchemy]") +TEST_CASE("Accepted items must be an in-order subset of the incoming offer", "[trading][alchemy]") { auto potion = CraftedPotion(); - REQUIRE_FALSE(ValidatePreparedPotions({potion}, {})); - REQUIRE(ValidatePreparedPotions({potion}, {potion})); + REQUIRE(ValidateAcceptedItems({potion}, {})); + REQUIRE(ValidateAcceptedItems({potion}, {potion})); auto stale = potion; stale.Count += 1; - REQUIRE_FALSE(ValidatePreparedPotions({potion}, {stale})); + REQUIRE_FALSE(ValidateAcceptedItems({potion}, {stale})); stale = potion; stale.Potion.Effects[0].Magnitude += 1; - REQUIRE_FALSE(ValidatePreparedPotions({potion}, {stale})); - REQUIRE_FALSE(ValidatePreparedPotions({}, {potion})); - REQUIRE_FALSE(ValidatePreparedPotions({potion}, {potion, potion})); - REQUIRE(ValidatePreparedPotions({}, {})); + REQUIRE_FALSE(ValidateAcceptedItems({potion}, {stale})); + REQUIRE_FALSE(ValidateAcceptedItems({}, {potion})); + REQUIRE_FALSE(ValidateAcceptedItems({potion}, {potion, potion})); + REQUIRE(ValidateAcceptedItems({}, {})); +} + +TEST_CASE("Only items each recipient accepts change hands", "[trading][alchemy]") +{ + // A offers a modded-effect potion and a vanilla-effect poison; B offers a vanilla weapon and modded armor. + auto moddedPotion = CraftedPotion(1); + moddedPotion.Count = 1; + moddedPotion.Potion.Effects[0].EffectId.ModId = 7; + auto poison = CraftedPotion(2); + poison.Count = 1; + poison.Potion.IsPoison = true; + Inventory::Entry weapon; + weapon.BaseId.BaseId = 0x12EB7; + weapon.Count = 1; + weapon.ExtraEnchantId.ModId = 0xFFFFFFFF; + weapon.ExtraEnchantId.BaseId = 0x800; + weapon.EnchantData.IsWeapon = true; + weapon.EnchantData.Effects.push_back(poison.Potion.Effects[0]); + Inventory::Entry armor; + armor.BaseId.ModId = 9; + armor.BaseId.BaseId = 0x801; + armor.Count = 1; + + Inventory left; + left.Entries = {moddedPotion, poison}; + Inventory right; + right.Entries = {weapon, armor}; + const Vector leftOffer{moddedPotion, poison}; + const Vector rightOffer{weapon, armor}; + const Vector acceptedByB{poison}; + const Vector acceptedByA{weapon}; + REQUIRE(ValidateAcceptedItems(leftOffer, acceptedByB)); + REQUIRE(ValidateAcceptedItems(rightOffer, acceptedByA)); + REQUIRE(MatchAcceptedItems(leftOffer, acceptedByB) == Vector{false, true}); + REQUIRE(MatchAcceptedItems(rightOffer, acceptedByA) == Vector{true, false}); + + Inventory leftResult; + Inventory rightResult; + REQUIRE(PrepareTradeExchange(left, right, acceptedByB, acceptedByA, leftResult, rightResult)); + REQUIRE(leftResult.Entries.size() == 2); + REQUIRE(SameTradeItem(leftResult.Entries[0], moddedPotion)); + REQUIRE(SameTradeItem(leftResult.Entries[1], weapon)); + REQUIRE(rightResult.Entries.size() == 2); + REQUIRE(SameTradeItem(rightResult.Entries[0], armor)); + REQUIRE(SameTradeItem(rightResult.Entries[1], poison)); } TEST_CASE("Potion recipes and readiness proofs round trip through packet factories", "[trading][encoding][alchemy]") @@ -124,7 +169,7 @@ TEST_CASE("Potion recipes and readiness proofs round trip through packet factori { TradeSetReadyRequest request; request.Ready = true; - request.PreparedPotions.push_back(potion); + request.AcceptedItems.push_back(potion); TiltedPhoques::Buffer::Writer writer(&buffer); request.Serialize(writer); TiltedPhoques::Buffer::Reader reader(&buffer); @@ -132,14 +177,15 @@ TEST_CASE("Potion recipes and readiness proofs round trip through packet factori REQUIRE(message); const auto* decoded = static_cast(message.get()); REQUIRE(decoded->Ready); - REQUIRE(decoded->PreparedPotions == request.PreparedPotions); - REQUIRE(decoded->PreparedPotions[0].Potion == potion.Potion); + REQUIRE(decoded->AcceptedItems == request.AcceptedItems); + REQUIRE(decoded->AcceptedItems[0].Potion == potion.Potion); } SECTION("Full inventory state") { NotifyTradeState state; state.PartnerItems.push_back(potion); state.SelfInventory.push_back(potion); + state.SelfAcceptedItems.push_back(potion); TiltedPhoques::Buffer::Writer writer(&buffer); state.Serialize(writer); TiltedPhoques::Buffer::Reader reader(&buffer); @@ -148,6 +194,8 @@ TEST_CASE("Potion recipes and readiness proofs round trip through packet factori const auto* decoded = static_cast(message.get()); REQUIRE(decoded->PartnerItems[0].Potion == potion.Potion); REQUIRE(decoded->SelfInventory[0].Potion == potion.Potion); + REQUIRE(decoded->SelfAcceptedItems.size() == 1); + REQUIRE(decoded->SelfAcceptedItems[0].Potion == potion.Potion); } } diff --git a/PLAYER_TRADING_POTIONS.md b/PLAYER_TRADING_POTIONS.md index ad1f8bc19..c461427be 100644 --- a/PLAYER_TRADING_POTIONS.md +++ b/PLAYER_TRADING_POTIONS.md @@ -14,3 +14,9 @@ Build both clients and the server from the same commit on `codex/player-trading- Recreation uses Skyrim's `BGSCreatedObjectManager::AddPotion` (or `AddPoison` for poisons) with the captured effects directly, in the sender's effect order. It does not invoke the crafting menu or calculate strength from the recipient's skills. The Ready request confirms successful reconstruction; changing an offer invalidates readiness. Unsupported temporary effects and effect conditions are excluded from trading. Custom per-instance names remain local inventory UI metadata. Native engine behavior, including the AddPotion address and created-object reference lifetime, still requires this in-game test. Passing compilation and packet tests does not establish in-game correctness. + +## Items the recipient cannot load + +Each client shows only the partner items it can rebuild: every plugin form the item references must be loaded locally, and crafted (temporary) potions and enchantments must have effects that resolve locally. Items using an applied crafted poison are never receivable. When a player readies, the Ready request lists exactly the partner items that client accepts; the server transfers only those, and everything else stays with its owner. Once the partner is ready, the sender's own offer marks any item the partner cannot receive. + +9. A offers a crafted potion with an effect from a plugin B lacks, plus a crafted poison with vanilla effects. B offers a weapon with a crafted vanilla enchantment, plus armor from a plugin A lacks. Verify A sees only the weapon, B sees only the poison, each sender's offer marks the hidden item after the partner readies, and after the countdown only the weapon and poison change hands. From 9900e11bd7835e87813c02ff30b0b89a9a3a2d63 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 23:22:53 +0000 Subject: [PATCH 3/3] feat: trade weapons coated with crafted poisons Applied crafted poisons now carry their recipe in the inventory entry and compare by recipe instead of temporary ID. The recipient recreates the coating with AddPoison when every effect resolves locally; otherwise the weapon stays hidden and remains with its owner. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SSW5HGXzi9kAVm4zdsSaid --- .../client/Games/Skyrim/Forms/AlchemyItem.cpp | 33 +++++++++++------ Code/client/Games/Skyrim/Forms/AlchemyItem.h | 2 + Code/client/Games/Skyrim/TESObjectREFR.cpp | 19 +++++++--- Code/client/Services/Generic/TradeService.cpp | 37 +++++++++++++------ Code/encoding/Structs/Inventory.cpp | 13 +++++++ Code/encoding/Structs/Inventory.h | 11 +++++- Code/tests/trading.cpp | 28 ++++++++++++++ PLAYER_TRADING_POTIONS.md | 3 +- 8 files changed, 114 insertions(+), 32 deletions(-) diff --git a/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp b/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp index e5e3c3a11..ed0607c7e 100644 --- a/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp +++ b/Code/client/Games/Skyrim/Forms/AlchemyItem.cpp @@ -4,13 +4,13 @@ #include #include -void AlchemyItem::Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept +bool AlchemyItem::CaptureRecipe(TESForm* apForm, Inventory::PotionData& aData) noexcept { + aData = {}; auto* potion = apForm && apForm->IsTemporary() ? Cast(apForm) : nullptr; if (!potion || potion->IsFood()) - return; - aEntry.Potion = {}; - aEntry.Potion.IsPoison = potion->IsPoison(); + return false; + aData.IsPoison = potion->IsPoison(); auto& mods = World::Get().GetModSystem(); for (auto* effect : potion->listOfEffects) { @@ -19,17 +19,26 @@ void AlchemyItem::Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept if (!effect || !effect->pEffectSetting || effect->Condition.pHead || effect->pEffectSetting->IsTemporary() || !mods.GetServerModId(effect->pEffectSetting->formID, result.EffectId)) { - aEntry.Potion.Effects.clear(); - return; + aData.Effects.clear(); + return false; } result.Magnitude = effect->data.fMagnitude; result.Area = effect->data.iArea; result.Duration = effect->data.iDuration; result.RawCost = effect->fRawCost; - aEntry.Potion.Effects.push_back(result); + aData.Effects.push_back(result); } - if (!aEntry.Potion.IsValid()) - aEntry.Potion.Effects.clear(); + if (!aData.IsValid()) + aData.Effects.clear(); + return !aData.Effects.empty(); +} + +void AlchemyItem::Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept +{ + auto* potion = apForm && apForm->IsTemporary() ? Cast(apForm) : nullptr; + if (!potion || potion->IsFood()) + return; + CaptureRecipe(apForm, aEntry.Potion); } AlchemyItem* AlchemyItem::Find(TESObjectREFR* apOwner, const Inventory::PotionData& aData) noexcept @@ -84,9 +93,9 @@ AlchemyItem* AlchemyItem::Create(const Inventory::PotionData& aData) noexcept Memory::Free(effects.data); if (created) { - Inventory::Entry check; - Capture(created, check); - if (check.Potion == aData) + Inventory::PotionData check; + CaptureRecipe(created, check); + if (check == aData) return created; spdlog::error("[TradeService]: AddPotion returned effects/type differing from the offered potion"); Release(created); diff --git a/Code/client/Games/Skyrim/Forms/AlchemyItem.h b/Code/client/Games/Skyrim/Forms/AlchemyItem.h index 69c6597cd..4877f302c 100644 --- a/Code/client/Games/Skyrim/Forms/AlchemyItem.h +++ b/Code/client/Games/Skyrim/Forms/AlchemyItem.h @@ -30,6 +30,8 @@ struct AlchemyItem : MagicItem bool IsPoison() const noexcept { return (data.flags & Poison) != 0; } bool IsFood() const noexcept { return (data.flags & Food) != 0 && !IsPoison(); } + // Records a player-crafted potion or poison's effects; false for other forms or unsupported effects. + static bool CaptureRecipe(TESForm* apForm, Inventory::PotionData& aData) noexcept; static void Capture(TESForm* apForm, Inventory::Entry& aEntry) noexcept; static AlchemyItem* Find(TESObjectREFR* apOwner, const Inventory::PotionData& aData) noexcept; // The returned pointer owns one created-object-manager reference. Release it after use. diff --git a/Code/client/Games/Skyrim/TESObjectREFR.cpp b/Code/client/Games/Skyrim/TESObjectREFR.cpp index 5ed88268f..7369586e1 100644 --- a/Code/client/Games/Skyrim/TESObjectREFR.cpp +++ b/Code/client/Games/Skyrim/TESObjectREFR.cpp @@ -558,6 +558,7 @@ void TESObjectREFR::GetItemFromExtraData(Inventory::Entry& arEntry, ExtraDataLis { modSystem.GetServerModId(pExtraPoison->pPoison->formID, arEntry.ExtraPoisonId); arEntry.ExtraPoisonCount = pExtraPoison->uiCount; + AlchemyItem::CaptureRecipe(pExtraPoison->pPoison, arEntry.PoisonData); } } @@ -618,12 +619,18 @@ ExtraDataList* TESObjectREFR::GetExtraDataFromItem(const Inventory::Entry& arEnt if (arEntry.ExtraPoisonId != 0) { - // TODO: does poison have the same temp problem as enchants? - // doesn't seem to be the case, there are only like 3 poisons, and no custom ones - TP_ASSERT(arEntry.ExtraPoisonId.ModId != 0xFFFFFFFF, "Poison is sent as temp!"); - - uint32_t poisonId = modSystem.GetGameId(arEntry.ExtraPoisonId); - if (AlchemyItem* pPoison = Cast(TESForm::GetById(poisonId))) + AlchemyItem* pPoison = nullptr; + if (arEntry.ExtraPoisonId.ModId == 0xFFFFFFFF) + { + // Player-crafted poison: recreate it from the owner's recipe. The created-object reference + // is kept so the form outlives this extra data list being attached to the item. + pPoison = AlchemyItem::Create(arEntry.PoisonData); + if (!pPoison) + spdlog::warn("{}: Unable to recreate applied crafted poison", __FUNCTION__); + } + else + pPoison = Cast(TESForm::GetById(modSystem.GetGameId(arEntry.ExtraPoisonId))); + if (pPoison) { pExtraDataList->SetPoison(pPoison, arEntry.ExtraPoisonCount); } diff --git a/Code/client/Services/Generic/TradeService.cpp b/Code/client/Services/Generic/TradeService.cpp index e1c6cc562..f71172fea 100644 --- a/Code/client/Services/Generic/TradeService.cpp +++ b/Code/client/Services/Generic/TradeService.cpp @@ -107,8 +107,13 @@ bool CanReceiveTradeItem(ModSystem& aModSystem, const Inventory::Entry& acItem) if (acItem.ExtraPoisonId != 0) { - // Applied crafted poisons carry no recipe in the inventory entry. - if (acItem.ExtraPoisonId.ModId == 0xFFFFFFFF || !Cast(TESForm::GetById(aModSystem.GetGameId(acItem.ExtraPoisonId)))) + if (acItem.ExtraPoisonId.ModId == 0xFFFFFFFF) + { + // Applied player-crafted poison: recreated from its effects on the recipient. + if (!acItem.PoisonData.IsValid() || !EffectsResolve(aModSystem, acItem.PoisonData.Effects)) + return false; + } + else if (!Cast(TESForm::GetById(aModSystem.GetGameId(acItem.ExtraPoisonId)))) return false; } return true; @@ -433,6 +438,16 @@ void TradeService::EmitStateToUI() const noexcept auto& modSystem = m_world.GetModSystem(); + auto formatEffect = [&](const Inventory::EffectItem& effect) { + auto* form = TESForm::GetById(modSystem.GetGameId(effect.EffectId)); + std::string text = fmt::format("{}: {:.0f}", form && form->GetName() ? form->GetName() : "Unknown effect", effect.Magnitude); + if (effect.Area > 0) + text += fmt::format(", {} ft", effect.Area); + if (effect.Duration > 0) + text += fmt::format(" for {}s", effect.Duration); + return text; + }; + auto makeDict = [&](const Inventory::Entry& entry, const char* apNote = nullptr) { auto dict = CefDictionaryValue::Create(); dict->SetInt("modId", entry.BaseId.ModId); @@ -452,21 +467,19 @@ void TradeService::EmitStateToUI() const noexcept // Temporary IDs belong to the owner's game session; the recipient gets a fresh one from AddPotion. details->SetString(detailIndex++, fmt::format("Temporary ID: {:08X}", 0xFF000000u | (entry.BaseId.BaseId & 0x00FFFFFFu))); for (const auto& effect : entry.Potion.Effects) - { - auto* form = TESForm::GetById(modSystem.GetGameId(effect.EffectId)); - std::string text = fmt::format("{}: {:.0f}", form && form->GetName() ? form->GetName() : "Unknown effect", effect.Magnitude); - if (effect.Area > 0) - text += fmt::format(", {} ft", effect.Area); - if (effect.Duration > 0) - text += fmt::format(" for {}s", effect.Duration); - details->SetString(detailIndex++, text); - } + details->SetString(detailIndex++, formatEffect(effect)); } if (entry.ExtraHealth > 1.0f) details->SetString(detailIndex++, fmt::format("Improvement: {:.0f}%", entry.ExtraHealth * 100.0f)); if (entry.ExtraEnchantId.BaseId || !entry.EnchantData.Effects.empty()) details->SetString(detailIndex++, "Enchanted"); - if (entry.ExtraPoisonId.BaseId) + if (!entry.PoisonData.Effects.empty()) + { + details->SetString(detailIndex++, fmt::format("Poisoned ({} uses left):", entry.ExtraPoisonCount)); + for (const auto& effect : entry.PoisonData.Effects) + details->SetString(detailIndex++, " " + formatEffect(effect)); + } + else if (entry.ExtraPoisonId.BaseId) details->SetString(detailIndex++, "Poisoned"); if (entry.ExtraSoulLevel > 0) details->SetString(detailIndex++, fmt::format("Soul level: {}", entry.ExtraSoulLevel)); diff --git a/Code/encoding/Structs/Inventory.cpp b/Code/encoding/Structs/Inventory.cpp index 8c1d8b425..5f853e1ec 100644 --- a/Code/encoding/Structs/Inventory.cpp +++ b/Code/encoding/Structs/Inventory.cpp @@ -71,6 +71,10 @@ void Inventory::Entry::Serialize(TiltedPhoques::Buffer::Writer& aWriter) const n Serialization::WriteVarInt(aWriter, Potion.Effects.size()); for (const auto& effect : Potion.Effects) effect.Serialize(aWriter); + Serialization::WriteBool(aWriter, PoisonData.IsPoison); + Serialization::WriteVarInt(aWriter, PoisonData.Effects.size()); + for (const auto& effect : PoisonData.Effects) + effect.Serialize(aWriter); } void Inventory::Entry::Deserialize(TiltedPhoques::Buffer::Reader& aReader) noexcept @@ -111,6 +115,15 @@ void Inventory::Entry::Deserialize(TiltedPhoques::Buffer::Reader& aReader) noexc effect.Deserialize(aReader); Potion.Effects.push_back(effect); } + PoisonData.IsPoison = Serialization::ReadBool(aReader); + PoisonData.Effects.clear(); + const auto poisonEffectCount = Serialization::ReadVarInt(aReader); + for (uint64_t i = 0; i < poisonEffectCount; ++i) + { + EffectItem effect; + effect.Deserialize(aReader); + PoisonData.Effects.push_back(effect); + } } bool Inventory::operator==(const Inventory& acRhs) const noexcept diff --git a/Code/encoding/Structs/Inventory.h b/Code/encoding/Structs/Inventory.h index c43b49908..a0e7cc2e5 100644 --- a/Code/encoding/Structs/Inventory.h +++ b/Code/encoding/Structs/Inventory.h @@ -52,6 +52,8 @@ struct Inventory GameId ExtraPoisonId{}; uint32_t ExtraPoisonCount{}; + // Recipe of an applied player-crafted poison, whose temporary ID is only valid for its owner. + PotionData PoisonData{}; int32_t ExtraSoulLevel{}; @@ -82,10 +84,17 @@ struct Inventory // TODO: the whole server side state thing is very flawed // since many of these things can and will change, like poison id or charge // or the fact that the enchant id can be temp - return ExtraCharge == acRhs.ExtraCharge && ExtraEnchantId == acRhs.ExtraEnchantId && ExtraEnchantCharge == acRhs.ExtraEnchantCharge && ExtraEnchantRemoveUnequip == acRhs.ExtraEnchantRemoveUnequip && ExtraHealth == acRhs.ExtraHealth && ExtraPoisonId == acRhs.ExtraPoisonId && + return ExtraCharge == acRhs.ExtraCharge && ExtraEnchantId == acRhs.ExtraEnchantId && ExtraEnchantCharge == acRhs.ExtraEnchantCharge && ExtraEnchantRemoveUnequip == acRhs.ExtraEnchantRemoveUnequip && ExtraHealth == acRhs.ExtraHealth && IsSamePoison(acRhs) && ExtraPoisonCount == acRhs.ExtraPoisonCount && ExtraSoulLevel == acRhs.ExtraSoulLevel && ExtraWorn == acRhs.ExtraWorn && ExtraWornLeft == acRhs.ExtraWornLeft && IsQuestItem == acRhs.IsQuestItem; } + bool IsSamePoison(const Entry& acRhs) const noexcept + { + if (!PoisonData.Effects.empty() || !acRhs.PoisonData.Effects.empty()) + return ExtraPoisonId.ModId == 0xFFFFFFFF && acRhs.ExtraPoisonId.ModId == 0xFFFFFFFF && PoisonData == acRhs.PoisonData; + return ExtraPoisonId == acRhs.ExtraPoisonId; + } + bool IsWorn() const noexcept { return ExtraWorn || ExtraWornLeft; } }; diff --git a/Code/tests/trading.cpp b/Code/tests/trading.cpp index efd15ad26..1cef9b47a 100644 --- a/Code/tests/trading.cpp +++ b/Code/tests/trading.cpp @@ -96,6 +96,34 @@ TEST_CASE("Crafted potion recipes compare independently of effect order", "[trad REQUIRE_FALSE(first.Potion == second.Potion); } +TEST_CASE("Applied crafted poisons identify by recipe, not temporary ID", "[trading][alchemy]") +{ + Inventory::Entry sword; + sword.BaseId.BaseId = 0x12EB7; + sword.Count = 1; + sword.ExtraPoisonId.ModId = 0xFFFFFFFF; + sword.ExtraPoisonId.BaseId = 0x900; + sword.ExtraPoisonCount = 3; + sword.PoisonData = CraftedPotion().Potion; + sword.PoisonData.IsPoison = true; + REQUIRE(sword.PoisonData.IsValid()); + + auto recreated = sword; + recreated.ExtraPoisonId.BaseId = 0xABC; + REQUIRE(SameTradeItem(sword, recreated)); + recreated.PoisonData.Effects[0].Magnitude += 1; + REQUIRE_FALSE(SameTradeItem(sword, recreated)); + + TiltedPhoques::Buffer buffer(1024); + TiltedPhoques::Buffer::Writer writer(&buffer); + sword.Serialize(writer); + TiltedPhoques::Buffer::Reader reader(&buffer); + Inventory::Entry decoded; + decoded.Deserialize(reader); + REQUIRE(decoded == sword); + REQUIRE(decoded.PoisonData == sword.PoisonData); +} + TEST_CASE("Accepted items must be an in-order subset of the incoming offer", "[trading][alchemy]") { auto potion = CraftedPotion(); diff --git a/PLAYER_TRADING_POTIONS.md b/PLAYER_TRADING_POTIONS.md index c461427be..ef083e9fc 100644 --- a/PLAYER_TRADING_POTIONS.md +++ b/PLAYER_TRADING_POTIONS.md @@ -17,6 +17,7 @@ Native engine behavior, including the AddPotion address and created-object refer ## Items the recipient cannot load -Each client shows only the partner items it can rebuild: every plugin form the item references must be loaded locally, and crafted (temporary) potions and enchantments must have effects that resolve locally. Items using an applied crafted poison are never receivable. When a player readies, the Ready request lists exactly the partner items that client accepts; the server transfers only those, and everything else stays with its owner. Once the partner is ready, the sender's own offer marks any item the partner cannot receive. +Each client shows only the partner items it can rebuild: every plugin form the item references must be loaded locally, and crafted (temporary) potions and enchantments must have effects that resolve locally. Weapons coated with a crafted poison carry the poison's recipe and are receivable when its effects resolve; the recipient's coating is recreated with `AddPoison`, keeping the remaining uses. When a player readies, the Ready request lists exactly the partner items that client accepts; the server transfers only those, and everything else stays with its owner. Once the partner is ready, the sender's own offer marks any item the partner cannot receive. 9. A offers a crafted potion with an effect from a plugin B lacks, plus a crafted poison with vanilla effects. B offers a weapon with a crafted vanilla enchantment, plus armor from a plugin A lacks. Verify A sees only the weapon, B sees only the poison, each sender's offer marks the hidden item after the partner readies, and after the countdown only the weapon and poison change hands. +10. Coat a weapon with a crafted vanilla-effect poison, trade it, and verify the recipient's weapon shows the same poison effects and remaining uses, then hits apply the poison. Repeat with a poison using an effect from a plugin the recipient lacks: the weapon must stay hidden and remain with the sender.