From 4beed294047897f41580eba76a44ba08ad6f627b Mon Sep 17 00:00:00 2001 From: Petr Mikheev Date: Wed, 4 Oct 2023 22:48:17 +0200 Subject: [PATCH] Changes required during review --- apps/openmw/mwclass/esm4npc.cpp | 7 ++- apps/openmw/mwrender/esm4npcanimation.cpp | 67 +++++++++++++---------- apps/openmw/mwrender/esm4npcanimation.hpp | 7 ++- components/esm4/loadarma.cpp | 23 +++++--- components/esm4/loadarma.hpp | 14 ++--- components/misc/resourcehelpers.cpp | 6 +- components/misc/resourcehelpers.hpp | 2 +- 7 files changed, 73 insertions(+), 53 deletions(-) diff --git a/apps/openmw/mwclass/esm4npc.cpp b/apps/openmw/mwclass/esm4npc.cpp index 5399635fee..8bca91a519 100644 --- a/apps/openmw/mwclass/esm4npc.cpp +++ b/apps/openmw/mwclass/esm4npc.cpp @@ -32,6 +32,9 @@ namespace MWClass static const ESM4::Npc* chooseTemplate(const std::vector& recs, uint16_t flag) { + // If the record is neither TES4 nor TES5 (though maybe FO4 is compatible with tes5.templateFlags), then + // the function can return nullptr that will lead to "ESM4 NPC traits not found" exception and the NPC + // will not be added to the scene. But in any way it shouldn't cause a crash. for (const auto* rec : recs) if (rec->mIsTES4 || !(rec->mBaseConfig.tes5.templateFlags & flag)) return rec; @@ -68,9 +71,9 @@ namespace MWClass data->mBaseData = chooseTemplate(npcRecs, ESM4::Npc::TES5_UseBaseData); if (!data->mTraits) - throw std::runtime_error("ESM4 Npc traits not found"); + throw std::runtime_error("ESM4 NPC traits not found"); if (!data->mBaseData) - throw std::runtime_error("ESM4 Npc base data not found"); + throw std::runtime_error("ESM4 NPC base data not found"); data->mRace = store->get().find(data->mTraits->mRace); if (data->mTraits->mIsTES4) diff --git a/apps/openmw/mwrender/esm4npcanimation.cpp b/apps/openmw/mwrender/esm4npcanimation.cpp index 6e64e93d0b..005751c420 100644 --- a/apps/openmw/mwrender/esm4npcanimation.cpp +++ b/apps/openmw/mwrender/esm4npcanimation.cpp @@ -8,13 +8,13 @@ #include #include -#include "../mwworld/customdata.hpp" -#include "../mwworld/esmstore.hpp" - +#include #include #include #include "../mwclass/esm4npc.hpp" +#include "../mwworld/customdata.hpp" +#include "../mwworld/esmstore.hpp" namespace MWRender { @@ -23,18 +23,28 @@ namespace MWRender : Animation(ptr, std::move(parentNode), resourceSystem) { getOrCreateObjectRoot(); - const ESM4::Npc* traits = MWClass::ESM4Npc::getTraitsRecord(mPtr); - if (traits->mIsTES4) - insertTes4NpcBodyPartsAndEquipment(); - else - insertTes5NpcBodyPartsAndEquipment(); + updateParts(); } - void ESM4NpcAnimation::insertMesh(std::string_view model) + void ESM4NpcAnimation::updateParts() { - std::string path = "meshes\\"; - path.append(model); - mResourceSystem->getSceneManager()->getInstance(path, mObjectRoot.get()); + mObjectRoot->removeChildren(0, mObjectRoot->getNumChildren()); + const ESM4::Npc* traits = MWClass::ESM4Npc::getTraitsRecord(mPtr); + // There is no flag "mIsTES5", so we can not distinguish from other cases. + // But calling wrong `updateParts*` function shouldn't crash the game and will + // only lead to the NPC not being rendered. + if (traits->mIsTES4) + updatePartsTES4(); + else + updatePartsTES5(); + } + + void ESM4NpcAnimation::insertPart(std::string_view model) + { + if (model.empty()) + return; + mResourceSystem->getSceneManager()->getInstance( + Misc::ResourceHelpers::correctMeshPath(model, mResourceSystem->getVFS()), mObjectRoot.get()); } template @@ -48,7 +58,7 @@ namespace MWRender return rec->mModel; } - void ESM4NpcAnimation::insertTes4NpcBodyPartsAndEquipment() + void ESM4NpcAnimation::updatePartsTES4() { const ESM4::Npc* traits = MWClass::ESM4Npc::getTraitsRecord(mPtr); const ESM4::Race* race = MWClass::ESM4Npc::getRace(mPtr); @@ -57,25 +67,23 @@ namespace MWRender // TODO: Body and head parts are placed incorrectly, need to attach to bones for (const ESM4::Race::BodyPart& bodyPart : (isFemale ? race->mBodyPartsFemale : race->mBodyPartsMale)) - if (!bodyPart.mesh.empty()) - insertMesh(bodyPart.mesh); + insertPart(bodyPart.mesh); for (const ESM4::Race::BodyPart& bodyPart : (isFemale ? race->mHeadPartsFemale : race->mHeadParts)) - if (!bodyPart.mesh.empty()) - insertMesh(bodyPart.mesh); + insertPart(bodyPart.mesh); if (!traits->mHair.isZeroOrUnset()) { const MWWorld::ESMStore* store = MWBase::Environment::get().getESMStore(); if (const ESM4::Hair* hair = store->get().search(traits->mHair)) - insertMesh(hair->mModel); + insertPart(hair->mModel); } for (const ESM4::Armor* armor : MWClass::ESM4Npc::getEquippedArmor(mPtr)) - insertMesh(chooseTes4EquipmentModel(armor, isFemale)); + insertPart(chooseTes4EquipmentModel(armor, isFemale)); for (const ESM4::Clothing* clothing : MWClass::ESM4Npc::getEquippedClothing(mPtr)) - insertMesh(chooseTes4EquipmentModel(clothing, isFemale)); + insertPart(chooseTes4EquipmentModel(clothing, isFemale)); } - void ESM4NpcAnimation::insertTes5NpcBodyPartsAndEquipment() + void ESM4NpcAnimation::updatePartsTES5() { const MWWorld::ESMStore* store = MWBase::Environment::get().getESMStore(); @@ -95,10 +103,8 @@ namespace MWRender Log(Debug::Error) << "Head part not found: " << ESM::RefId(partId); continue; } - if (usedHeadPartTypes.contains(part->mType)) - continue; - usedHeadPartTypes.insert(part->mType); - insertMesh(part->mModel); + if (usedHeadPartTypes.emplace(part->mType).second) + insertPart(part->mModel); } }; @@ -126,7 +132,8 @@ namespace MWRender findArmorAddons(armor); if (!traits->mWornArmor.isZeroOrUnset()) findArmorAddons(store->get().find(traits->mWornArmor)); - findArmorAddons(store->get().find(race->mSkin)); + if (!race->mSkin.isZeroOrUnset()) + findArmorAddons(store->get().find(race->mSkin)); if (isFemale) std::sort(armorAddons.begin(), armorAddons.end(), @@ -139,12 +146,14 @@ namespace MWRender for (const ESM4::ArmorAddon* arma : armorAddons) { const uint32_t covers = arma->mBodyTemplate.bodyPart; + // if body is already covered, skip to avoid clipping if (covers & usedParts & ESM4::Armor::TES5_Body) - continue; // if body is already covered, skip to avoid clipping + continue; + // if covers at least something that wasn't covered before - add model if (covers & ~usedParts) - { // if covers at least something that wasn't covered before - add model + { usedParts |= covers; - insertMesh(isFemale ? arma->mModelFemale : arma->mModelMale); + insertPart(isFemale ? arma->mModelFemale : arma->mModelMale); } } diff --git a/apps/openmw/mwrender/esm4npcanimation.hpp b/apps/openmw/mwrender/esm4npcanimation.hpp index a8c451555f..1d93dace45 100644 --- a/apps/openmw/mwrender/esm4npcanimation.hpp +++ b/apps/openmw/mwrender/esm4npcanimation.hpp @@ -12,10 +12,11 @@ namespace MWRender const MWWorld::Ptr& ptr, osg::ref_ptr parentNode, Resource::ResourceSystem* resourceSystem); private: - void insertMesh(std::string_view model); + void insertPart(std::string_view model); - void insertTes4NpcBodyPartsAndEquipment(); - void insertTes5NpcBodyPartsAndEquipment(); + void updateParts(); + void updatePartsTES4(); + void updatePartsTES5(); }; } diff --git a/components/esm4/loadarma.cpp b/components/esm4/loadarma.cpp index 1de298c689..2bb6240ee8 100644 --- a/components/esm4/loadarma.cpp +++ b/components/esm4/loadarma.cpp @@ -95,14 +95,21 @@ void ESM4::ArmorAddon::load(ESM4::Reader& reader) break; case ESM4::SUB_DNAM: - reader.get(mMalePriority); - reader.get(mFemalePriority); - reader.get(mWeightSliderMale); - reader.get(mWeightSliderFemale); - reader.get(mUnknown1); - reader.get(mDetectionSoundValue); - reader.get(mUnknown2); - reader.get(mWeaponAdjust); + if (subHdr.dataSize == 12) + { + std::uint16_t unknownInt16; + std::uint8_t unknownInt8; + reader.get(mMalePriority); + reader.get(mFemalePriority); + reader.get(mWeightSliderMale); + reader.get(mWeightSliderFemale); + reader.get(unknownInt16); + reader.get(mDetectionSoundValue); + reader.get(unknownInt8); + reader.get(mWeaponAdjust); + } + else + reader.skipSubRecordData(); break; case ESM4::SUB_MO2T: // FIXME: should group with MOD2 case ESM4::SUB_MO2S: // FIXME: should group with MOD2 diff --git a/components/esm4/loadarma.hpp b/components/esm4/loadarma.hpp index fa36d22bbe..6733184aa8 100644 --- a/components/esm4/loadarma.hpp +++ b/components/esm4/loadarma.hpp @@ -59,17 +59,15 @@ namespace ESM4 BodyTemplate mBodyTemplate; // TES5 - std::uint8_t mMalePriority; - std::uint8_t mFemalePriority; + std::uint8_t mMalePriority = 0; + std::uint8_t mFemalePriority = 0; // Flag 0x2 in mWeightSlider means that there are 2 world models for different weights: _0.nif and _1.nif - std::uint8_t mWeightSliderMale; - std::uint8_t mWeightSliderFemale; + std::uint8_t mWeightSliderMale = 0; + std::uint8_t mWeightSliderFemale = 0; - std::uint16_t mUnknown1; - std::uint8_t mDetectionSoundValue; - std::uint8_t mUnknown2; - float mWeaponAdjust; + std::uint8_t mDetectionSoundValue = 0; + float mWeaponAdjust = 0; void load(ESM4::Reader& reader); // void save(ESM4::Writer& writer) const; diff --git a/components/misc/resourcehelpers.cpp b/components/misc/resourcehelpers.cpp index cddcbe463b..ce552df4f7 100644 --- a/components/misc/resourcehelpers.cpp +++ b/components/misc/resourcehelpers.cpp @@ -159,9 +159,11 @@ std::string Misc::ResourceHelpers::correctActorModelPath(const std::string& resP return mdlname; } -std::string Misc::ResourceHelpers::correctMeshPath(const std::string& resPath, const VFS::Manager* vfs) +std::string Misc::ResourceHelpers::correctMeshPath(std::string_view resPath, const VFS::Manager* vfs) { - return "meshes\\" + resPath; + std::string res = "meshes\\"; + res.append(resPath); + return res; } std::string Misc::ResourceHelpers::correctSoundPath(const std::string& resPath) diff --git a/components/misc/resourcehelpers.hpp b/components/misc/resourcehelpers.hpp index 37932ea155..478569ed14 100644 --- a/components/misc/resourcehelpers.hpp +++ b/components/misc/resourcehelpers.hpp @@ -33,7 +33,7 @@ namespace Misc std::string correctActorModelPath(const std::string& resPath, const VFS::Manager* vfs); // Adds "meshes\\". - std::string correctMeshPath(const std::string& resPath, const VFS::Manager* vfs); + std::string correctMeshPath(std::string_view resPath, const VFS::Manager* vfs); // Adds "sound\\". std::string correctSoundPath(const std::string& resPath);