diff --git a/Code/client/Games/Skyrim/Forms/EnchantmentItem.cpp b/Code/client/Games/Skyrim/Forms/EnchantmentItem.cpp index cc586d708..cf976bc91 100644 --- a/Code/client/Games/Skyrim/Forms/EnchantmentItem.cpp +++ b/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(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? diff --git a/Code/client/Games/Skyrim/TESObjectREFR.cpp b/Code/client/Games/Skyrim/TESObjectREFR.cpp index d309c4306..836852308 100644 --- a/Code/client/Games/Skyrim/TESObjectREFR.cpp +++ b/Code/client/Games/Skyrim/TESObjectREFR.cpp @@ -612,9 +612,10 @@ ExtraDataList* TESObjectREFR::GetExtraDataFromItem(const Inventory::Entry& arEnt pEnchantment = Cast(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); } diff --git a/Code/client/Services/Generic/TradeService.cpp b/Code/client/Services/Generic/TradeService.cpp index ce4774b92..62d88328c 100644 --- a/Code/client/Services/Generic/TradeService.cpp +++ b/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); }