Harden trade item application after sender crash

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

@@ -27,7 +27,11 @@ EnchantmentItem* EnchantmentItem::Create(const Inventory::EnchantmentData& aData
         effectItem.fRawCost = effect.RawCost;
         effectItem.pEffectSetting = Cast<EffectSetting>(TESForm::GetById(modSystem.GetGameId(effect.EffectId)));
         if (!effectItem.pEffectSetting)
+        {
+            // The engine dereferences every effect setting; a missing plugin effect must not reach it.
             spdlog::error("Effect setting not found: {:X}:{:X}", effect.EffectId.ModId, effect.EffectId.BaseId);
+            return nullptr;
+        }
 
         // TODO: TESCondition?
 

Code/client/Games/Skyrim/TESObjectREFR.cpp

@@ -612,9 +612,10 @@ ExtraDataList* TESObjectREFR::GetExtraDataFromItem(const Inventory::Entry& arEnt
             pEnchantment = Cast<EnchantmentItem>(TESForm::GetById(enchantId));
         }
 
-        TP_ASSERT(pEnchantment, "No Enchantment created or found.");
-
-        pExtraDataList->SetEnchantmentData(pEnchantment, arEntry.ExtraEnchantCharge, arEntry.ExtraEnchantRemoveUnequip);
+        if (pEnchantment)
+            pExtraDataList->SetEnchantmentData(pEnchantment, arEntry.ExtraEnchantCharge, arEntry.ExtraEnchantRemoveUnequip);
+        else
+            spdlog::warn("{}: Enchantment {:X}:{:X} could not be created or found", __FUNCTION__, arEntry.ExtraEnchantId.ModId, arEntry.ExtraEnchantId.BaseId);
     }
 
     if (arEntry.ExtraPoisonId != 0)
@@ -931,8 +932,9 @@ void TESObjectREFR::AddOrRemoveItem(const Inventory::Entry& arEntry, bool aIsSet
                 return nullptr;
             if (ExtraDataList* pExisting = FindExtraDataForItem(apItem, arEntry))
                 return pExisting;
-            spdlog::warn("{}: No stored extra data matches item {:X}; removing with rebuilt data", __FUNCTION__, apItem->formID);
-            return GetExtraDataFromItem(arEntry);
+            // Never hand RemoveItem a list the inventory does not own; remote copies often lack the exact data.
+            spdlog::warn("{}: No stored extra data matches item {:X}; removing without extra data", __FUNCTION__, apItem->formID);
+            return nullptr;
         };
         if (arEntry.Potion.Effects.empty())
             RemoveItem(pObject, -arEntry.Count, ITEM_REMOVE_REASON::kRemove, removalList(pObject), nullptr);
@@ -974,7 +976,9 @@ void TESObjectREFR::AddOrRemoveItem(const Inventory::Entry& arEntry, bool aIsSet
         }
     }
 
-    AlchemyItem::Release(createdPotion);
+    // The created potion's reference is deliberately kept: dropping it could free a form the inventory
+    // still points at if the inventory does not hold its own created-object reference.
+    (void)createdPotion;
     UpdateItemList(nullptr);
 }
 

Code/client/Services/Generic/TradeService.cpp

@@ -124,12 +124,11 @@ std::string MakeDisplayName(ModSystem& aModSystem, const Inventory::Entry& aEntr
 {
     if (!aEntry.Potion.Effects.empty())
     {
-        auto* local = AlchemyItem::Find(PlayerCharacter::Get(), aEntry.Potion);
-        auto* created = local ? nullptr : AlchemyItem::Create(aEntry.Potion);
-        auto* potion = local ? local : created;
-        std::string name = potion && potion->GetName() ? potion->GetName() : "Crafted potion (unavailable)";
-        AlchemyItem::Release(created);
-        return name;
+        // Only name from a potion this game already has; creating forms just to render the UI is avoided.
+        if (auto* local = AlchemyItem::Find(PlayerCharacter::Get(), aEntry.Potion); local && local->GetName())
+            return local->GetName();
+        auto* first = TESForm::GetById(aModSystem.GetGameId(aEntry.Potion.Effects[0].EffectId));
+        return fmt::format("{} of {}", aEntry.Potion.IsPoison ? "Poison" : "Potion", first && first->GetName() ? first->GetName() : "unknown effect");
     }
     const uint32_t formId = aModSystem.GetGameId(aEntry.BaseId);
     if (formId)
@@ -223,8 +222,7 @@ TradeService::~TradeService() noexcept
 
 void TradeService::ReleasePreparedPotions() noexcept
 {
-    for (auto* potion : m_preparedPotions)
-        AlchemyItem::Release(potion);
+    // References are kept: the same forms may already be in this player's inventory after the trade.
     m_preparedPotions.clear();
 }
 
@@ -253,8 +251,6 @@ void TradeService::SetReady(bool aReady) noexcept
                 }
                 if (std::find(m_preparedPotions.begin(), m_preparedPotions.end(), created) == m_preparedPotions.end())
                     m_preparedPotions.push_back(created);
-                else
-                    AlchemyItem::Release(created);
             }
             request.AcceptedItems.push_back(item);
         }