From ed5a81e1f0bf84ea338695c9496775842caaf8f7 Mon Sep 17 00:00:00 2001 From: Arkadii Hlushchevskyi Date: Sat, 28 Dec 2024 22:11:08 +0200 Subject: [PATCH] Ensured that dead NPCs don't equip outfits. - Added locking in OutfitManager for extra safety. --- SPID/include/OutfitManager.h | 5 +++ SPID/include/PCH.h | 4 ++ SPID/include/PCLevelMultManager.h | 3 -- SPID/src/Distribute.cpp | 4 ++ SPID/src/OutfitManager.cpp | 72 ++++++++++++++++++++++--------- 5 files changed, 64 insertions(+), 24 deletions(-) diff --git a/SPID/include/OutfitManager.h b/SPID/include/OutfitManager.h index 986388f..06cd9b5 100644 --- a/SPID/include/OutfitManager.h +++ b/SPID/include/OutfitManager.h @@ -112,6 +112,11 @@ namespace Outfits friend fmt::formatter; + /// + /// Lock for replacements. + /// + mutable Lock _lock; + /// /// Map of Actor's FormID and corresponding Outfit Replacements that are being tracked by the manager. /// diff --git a/SPID/include/PCH.h b/SPID/include/PCH.h index ba76683..0e6bc89 100644 --- a/SPID/include/PCH.h +++ b/SPID/include/PCH.h @@ -48,6 +48,10 @@ using Map = ankerl::unordered_dense::map; template using Set = ankerl::unordered_dense::set; +using Lock = std::shared_mutex; +using ReadLocker = std::shared_lock; +using WriteLocker = std::unique_lock; + struct string_hash { using is_transparent = void; // enable heterogeneous overloads diff --git a/SPID/include/PCLevelMultManager.h b/SPID/include/PCLevelMultManager.h index 330bfaa..41ec098 100644 --- a/SPID/include/PCLevelMultManager.h +++ b/SPID/include/PCLevelMultManager.h @@ -39,9 +39,6 @@ namespace PCLevelMult void SetNewGameStarted(); private: - using Lock = std::shared_mutex; - using ReadLocker = std::shared_lock; - using WriteLocker = std::unique_lock; enum class LEVEL_CAP_STATE { diff --git a/SPID/src/Distribute.cpp b/SPID/src/Distribute.cpp index 7f39d78..649066d 100644 --- a/SPID/src/Distribute.cpp +++ b/SPID/src/Distribute.cpp @@ -175,8 +175,12 @@ namespace Distribute void Distribute(NPCData& npcData, bool onlyLeveledEntries) { const auto input = PCLevelMult::Input{ npcData.GetActor(), npcData.GetNPC(), onlyLeveledEntries }; + + // We always do the normal distribution even for Dead NPCs, + // if Distributable Form is only meant to be distributed while NPC is alive, the entry must contain -D filter. Distribute(npcData, input); + // TODO: This will be moved to DeathDistribution's own hook. if (npcData.IsDead()) { // If NPC is already dead, perform the On Death Distribution. DeathDistribution::Manager::GetSingleton()->Distribute(npcData); } diff --git a/SPID/src/OutfitManager.cpp b/SPID/src/OutfitManager.cpp index a8e87e8..f5e1644 100644 --- a/SPID/src/OutfitManager.cpp +++ b/SPID/src/OutfitManager.cpp @@ -124,6 +124,8 @@ namespace Outfits #endif auto* manager = Manager::GetSingleton(); + ReadLocker lock(manager->_lock); + std::unordered_map loadedReplacements; auto& newReplacements = manager->replacements; @@ -201,7 +203,9 @@ namespace Outfits #ifndef NDEBUG logger::info("{:*^30}", "SAVING"); #endif - auto replacements = Manager::GetSingleton()->replacements; + auto manager = Manager::GetSingleton(); + ReadLocker lock(manager->_lock); + const auto& replacements = manager->replacements; #ifndef NDEBUG logger::info("Saving {} distributed outfits...", replacements.size()); #endif @@ -232,17 +236,19 @@ namespace Outfits /// This hook appliues pending outfit replacements before loading 3D model. Outfit Replacements are created by SetDefaultOutfit. struct Load3D { - static RE::NiAVObject* thunk(RE::Character* a_this, bool a_backgroundLoading) + static RE::NiAVObject* thunk(RE::Character* actor, bool a_backgroundLoading) { #ifndef NDEBUG - //logger::info("Load3D({})", *a_this); + logger::info("Load3D({}); Background: {}", *actor, a_backgroundLoading); #endif - const auto manager = Manager::GetSingleton(); - if (!manager->isLoadingGame) { - manager->ApplyOutfit(a_this); + if (!Manager::GetSingleton()->isLoadingGame) { + // Wrapping in a task maybe possibly perhaps would fix the crash in issue #67 + SKSE::GetTaskInterface()->AddTask([actor]() { + Manager::GetSingleton()->ApplyOutfit(actor); + }); } - return func(a_this, a_backgroundLoading); + return func(actor, a_backgroundLoading); } static inline REL::Relocation func; @@ -311,6 +317,7 @@ namespace Outfits RE::BSEventNotifyControl Manager::ProcessEvent(const RE::TESFormDeleteEvent* a_event, RE::BSTEventSource*) { + WriteLocker lock(_lock); if (a_event && a_event->formID != 0) { replacements.erase(a_event->formID); initialOutfits.erase(a_event->formID); @@ -358,35 +365,58 @@ namespace Outfits } logger::info("\tNew Outfit: {}", *outfit); #endif + WriteLocker lock(_lock); if (auto existing = replacements.find(actor->formID); existing != replacements.end()) { // we already have tracked replacement #ifndef NDEBUG logger::info("\tFound existing replacement {}", existing->second); #endif - if (outfit == defaultOutfit && outfit == existing->second.distributed) { // if the outfit we are trying to set is already the default one and we have a replacement for it, then we confirm that it was set. + // If we have an existing replacement and actor is already dead, + // then we don't want to set new outfit to avoid sudden changes that player might not expect. + // But only if the outfit was already given to them. + // This will allow to apply initial outfit to the dead actor in case new mod was added or something. + // + // TODO: Consider tracking looting state of the outfit? + // e.g. if an actor still wears all parts of the outfit, then allow to change it. + // This might be unexpected, since dead NPCs are supposed to have their outfit locked. + if (actor->IsDead()) { +#ifndef NDEBUG + logger::info("\tDead NPCs can't change the outfit"); +#endif + return false; + } + + // if the outfit we are trying to set is already the default one and we have a replacement for it, then we confirm that it was set. + if (outfit == defaultOutfit && outfit == existing->second.distributed) { #ifndef NDEBUG logger::info("\tExisting replacement is already set"); #endif return true; } - } - if (!CanEquipOutfit(actor, outfit)) { + if (!CanEquipOutfit(actor, outfit)) { #ifndef NDEBUG - logger::warn("\tAttempted to equip Outfit {} that can't be worn by given actor.", *outfit); + logger::warn("\tAttempted to equip Outfit {} that can't be worn by given actor.", *outfit); #endif - return false; - } - - if (auto previous = replacements.find(actor->formID); previous != replacements.end()) { - previous->second.distributed = outfit; + return false; + } + existing->second.distributed = outfit; +#ifndef NDEBUG + logger::warn("\tUpdated replacement {}", existing->second); +#endif + } else { + if (!CanEquipOutfit(actor, outfit)) { #ifndef NDEBUG - logger::warn("\tUpdated replacement {}", previous->second); + logger::warn("\tAttempted to equip Outfit {} that can't be worn by given actor.", *outfit); #endif - } else if (defaultOutfit) { - replacements.try_emplace(actor->formID, defaultOutfit, outfit); + return false; + } + + if (defaultOutfit) { + replacements.try_emplace(actor->formID, defaultOutfit, outfit); #ifndef NDEBUG - logger::warn("\tAdded replacement {}", OutfitReplacement(defaultOutfit, outfit)); + logger::warn("\tAdded replacement {}", OutfitReplacement(defaultOutfit, outfit)); #endif + } } return true; @@ -396,7 +426,7 @@ namespace Outfits { if (!actor) return false; - + ReadLocker lock(_lock); if (const auto replacement = replacements.find(actor->formID); replacement != replacements.end()) { if (replacement->second.distributed) { return ApplyOutfit(actor, replacement->second.distributed);