Finish crafted potion trading: AddPoison for poisons, temp IDs in trade UI

Code/client/Games/Skyrim/Forms/AlchemyItem.cpp

@@ -3,16 +3,14 @@
 #include <Systems/ModSystem.h>
 #include <TESObjectREFR.h>
 #include <World.h>
-#include <algorithm>
-#include <tuple>
 
-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<AlchemyItem>(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)
     {
@@ -21,24 +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<AlchemyItem>(apForm) : nullptr;
+    if (!potion || potion->IsFood())
         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);
-    });
+    CaptureRecipe(apForm, aEntry.Potion);
 }
 
 AlchemyItem* AlchemyItem::Find(TESObjectREFR* apOwner, const Inventory::PotionData& aData) noexcept
@@ -82,18 +82,20 @@ 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<EffectItem>*);
     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)
     {
-        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);

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.

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<AlchemyItem>(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<AlchemyItem>(TESForm::GetById(modSystem.GetGameId(arEntry.ExtraPoisonId)));
+        if (pPoison)
         {
             pExtraDataList->SetPoison(pPoison, arEntry.ExtraPoisonCount);
         }

Code/client/Services/Generic/TradeService.cpp

@@ -73,6 +73,52 @@ const char* GetItemCategory(TESForm* apForm) noexcept
     }
 }
 
+bool EffectsResolve(ModSystem& aModSystem, const Vector<Inventory::EffectItem>& acEffects) noexcept
+{
+    for (const auto& effect : acEffects)
+        if (!Cast<EffectSetting>(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<TESBoundObject>(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)
+    {
+        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<AlchemyItem>(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 +239,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 +364,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 +387,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 +438,17 @@ void TradeService::EmitStateToUI() const noexcept
 
     auto& modSystem = m_world.GetModSystem();
 
-    auto makeDict = [&](const Inventory::Entry& entry) {
+    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);
         dict->SetInt("baseId", entry.BaseId.BaseId);
@@ -400,30 +462,42 @@ 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)
+                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));
+        if (apNote)
+            details->SetString(detailIndex++, apNote);
         dict->SetList("details", details);
         return dict;
     };
 
     std::vector<int32_t> 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<uint32_t> match;
@@ -451,13 +525,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<int>(i), dict);
+        partnerList->SetDictionary(partnerIndex++, dict);
     }
     pArgs->SetList(6, partnerList);
 

Code/client/Services/TradeService.h

@@ -69,6 +69,7 @@ struct TradeService
         TiltedPhoques::Vector<Inventory::Entry> SelfItems;
         TiltedPhoques::Vector<Inventory::Entry> PartnerItems;
         TiltedPhoques::Vector<Inventory::Entry> SelfInventory;
+        TiltedPhoques::Vector<Inventory::Entry> SelfAcceptedItems;
         uint32_t CountdownMs{0};
         uint32_t CountdownTotalMs{0};
     };

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);
+    }
 }

Code/encoding/Messages/NotifyTradeState.h

@@ -24,4 +24,6 @@ struct NotifyTradeState final : ServerMessage
     TiltedPhoques::Vector<Inventory::Entry> SelfItems;
     TiltedPhoques::Vector<Inventory::Entry> PartnerItems;
     TiltedPhoques::Vector<Inventory::Entry> SelfInventory;
+    // Own offered items the ready partner can receive; empty while the partner is not ready.
+    TiltedPhoques::Vector<Inventory::Entry> SelfAcceptedItems;
 };

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));
     }
 }

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<Inventory::Entry> PreparedPotions;
+    Vector<Inventory::Entry> AcceptedItems;
 };