From c7d5ea9fbf221c425400903a2ffd56160f0e843e Mon Sep 17 00:00:00 2001 From: Alexei Kotov Date: Tue, 14 Nov 2023 00:44:23 +0300 Subject: [PATCH] Improve BulletNifLoader handling of extra data Only handle extra data for the root node(s) Properly handle MRK flag editor marker filtering Fix BSXFlags test --- .../nifloader/testbulletnifloader.cpp | 139 +++++++++--------- components/nifbullet/bulletnifloader.cpp | 62 ++++---- components/nifbullet/bulletnifloader.hpp | 1 + 3 files changed, 99 insertions(+), 103 deletions(-) diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index 58e1073307..7dce21bac6 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp @@ -957,12 +957,12 @@ namespace } TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision) + for_root_node_with_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision) { mNiStringExtraData.mData = "NCC__"; mNiStringExtraData.recType = Nif::RC_NiStringExtraData; - mNiTriShape.mExtra = Nif::ExtraPtr(&mNiStringExtraData); mNiTriShape.mParents.push_back(&mNiNode); + mNiNode.mExtra = Nif::ExtraPtr(&mNiStringExtraData); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; Nif::NIFFile file("test.nif"); @@ -985,13 +985,13 @@ namespace } TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_not_first_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision) + for_root_node_with_not_first_extra_data_string_equal_ncc_should_return_shape_with_cameraonly_collision) { mNiStringExtraData.mNext = Nif::ExtraPtr(&mNiStringExtraData2); mNiStringExtraData2.mData = "NCC__"; mNiStringExtraData2.recType = Nif::RC_NiStringExtraData; - mNiTriShape.mExtra = Nif::ExtraPtr(&mNiStringExtraData); mNiTriShape.mParents.push_back(&mNiNode); + mNiNode.mExtra = Nif::ExtraPtr(&mNiStringExtraData); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; Nif::NIFFile file("test.nif"); @@ -1012,8 +1012,62 @@ namespace EXPECT_EQ(*result, expected); } + TEST_F( + TestBulletNifLoader, for_root_node_with_extra_data_string_starting_with_nc_should_return_shape_with_nocollision) + { + mNiStringExtraData.mData = "NC___"; + mNiStringExtraData.recType = Nif::RC_NiStringExtraData; + mNiTriShape.mParents.push_back(&mNiNode); + mNiNode.mExtra = Nif::ExtraPtr(&mNiStringExtraData); + mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; + + Nif::NIFFile file("test.nif"); + file.mRoots.push_back(&mNiNode); + file.mHash = mHash; + + const auto result = mLoader.load(file); + + std::unique_ptr triangles(new btTriangleMesh(false)); + triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); + std::unique_ptr compound(new btCompoundShape); + compound->addChildShape(btTransform::getIdentity(), new Resource::TriangleMeshShape(triangles.release(), true)); + + Resource::BulletShape expected; + expected.mCollisionShape.reset(compound.release()); + expected.mVisualCollisionType = Resource::VisualCollisionType::Default; + + EXPECT_EQ(*result, expected); + } + TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_extra_data_string_starting_with_nc_should_return_shape_with_nocollision) + for_root_node_with_not_first_extra_data_string_starting_with_nc_should_return_shape_with_nocollision) + { + mNiStringExtraData.mNext = Nif::ExtraPtr(&mNiStringExtraData2); + mNiStringExtraData2.mData = "NC___"; + mNiStringExtraData2.recType = Nif::RC_NiStringExtraData; + mNiTriShape.mParents.push_back(&mNiNode); + mNiNode.mExtra = Nif::ExtraPtr(&mNiStringExtraData); + mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; + + Nif::NIFFile file("test.nif"); + file.mRoots.push_back(&mNiNode); + file.mHash = mHash; + + const auto result = mLoader.load(file); + + std::unique_ptr triangles(new btTriangleMesh(false)); + triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); + std::unique_ptr compound(new btCompoundShape); + compound->addChildShape(btTransform::getIdentity(), new Resource::TriangleMeshShape(triangles.release(), true)); + + Resource::BulletShape expected; + expected.mCollisionShape.reset(compound.release()); + expected.mVisualCollisionType = Resource::VisualCollisionType::Default; + + EXPECT_EQ(*result, expected); + } + + TEST_F(TestBulletNifLoader, for_tri_shape_child_node_with_extra_data_string_should_ignore_extra_data) { mNiStringExtraData.mData = "NC___"; mNiStringExtraData.recType = Nif::RC_NiStringExtraData; @@ -1034,35 +1088,6 @@ namespace Resource::BulletShape expected; expected.mCollisionShape.reset(compound.release()); - expected.mVisualCollisionType = Resource::VisualCollisionType::Default; - - EXPECT_EQ(*result, expected); - } - - TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_not_first_extra_data_string_starting_with_nc_should_return_shape_with_nocollision) - { - mNiStringExtraData.mNext = Nif::ExtraPtr(&mNiStringExtraData2); - mNiStringExtraData2.mData = "NC___"; - mNiStringExtraData2.recType = Nif::RC_NiStringExtraData; - mNiTriShape.mExtra = Nif::ExtraPtr(&mNiStringExtraData); - mNiTriShape.mParents.push_back(&mNiNode); - mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; - - Nif::NIFFile file("test.nif"); - file.mRoots.push_back(&mNiNode); - file.mHash = mHash; - - const auto result = mLoader.load(file); - - std::unique_ptr triangles(new btTriangleMesh(false)); - triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); - std::unique_ptr compound(new btCompoundShape); - compound->addChildShape(btTransform::getIdentity(), new Resource::TriangleMeshShape(triangles.release(), true)); - - Resource::BulletShape expected; - expected.mCollisionShape.reset(compound.release()); - expected.mVisualCollisionType = Resource::VisualCollisionType::Default; EXPECT_EQ(*result, expected); } @@ -1101,33 +1126,13 @@ namespace EXPECT_EQ(*result, expected); } - TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_extra_data_string_mrk_should_return_shape_with_null_collision_shape) - { - mNiStringExtraData.mData = "MRK"; - mNiStringExtraData.recType = Nif::RC_NiStringExtraData; - mNiTriShape.mExtra = Nif::ExtraPtr(&mNiStringExtraData); - mNiTriShape.mParents.push_back(&mNiNode); - mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; - - Nif::NIFFile file("test.nif"); - file.mRoots.push_back(&mNiNode); - file.mHash = mHash; - - const auto result = mLoader.load(file); - - Resource::BulletShape expected; - - EXPECT_EQ(*result, expected); - } - TEST_F(TestBulletNifLoader, bsx_editor_marker_flag_disables_collision_for_markers) { - mNiIntegerExtraData.mData = 34; // BSXFlags "has collision" | "editor marker" - mNiIntegerExtraData.recType = Nif::RC_BSXFlags; - mNiTriShape.mExtraList.push_back(Nif::ExtraPtr(&mNiIntegerExtraData)); mNiTriShape.mParents.push_back(&mNiNode); mNiTriShape.mName = "EditorMarker"; + mNiIntegerExtraData.mData = 34; // BSXFlags "has collision" | "editor marker" + mNiIntegerExtraData.recType = Nif::RC_BSXFlags; + mNiNode.mExtraList.push_back(Nif::ExtraPtr(&mNiIntegerExtraData)); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; Nif::NIFFile file("test.nif"); @@ -1142,18 +1147,14 @@ namespace EXPECT_EQ(*result, expected); } - TEST_F(TestBulletNifLoader, - for_tri_shape_child_node_with_extra_data_string_mrk_and_other_collision_node_should_return_shape_with_triangle_mesh_shape_with_all_meshes) + TEST_F(TestBulletNifLoader, mrk_editor_marker_flag_disables_collision_for_markers) { + mNiTriShape.mParents.push_back(&mNiNode); + mNiTriShape.mName = "Tri EditorMarker"; mNiStringExtraData.mData = "MRK"; mNiStringExtraData.recType = Nif::RC_NiStringExtraData; - mNiTriShape.mExtra = Nif::ExtraPtr(&mNiStringExtraData); - mNiTriShape.mParents.push_back(&mNiNode2); - mNiNode2.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; - mNiNode2.recType = Nif::RC_RootCollisionNode; - mNiNode2.mParents.push_back(&mNiNode); - mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiNode2) }; - mNiNode.recType = Nif::RC_NiNode; + mNiNode.mExtra = Nif::ExtraPtr(&mNiStringExtraData); + mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; Nif::NIFFile file("test.nif"); file.mRoots.push_back(&mNiNode); @@ -1161,13 +1162,7 @@ namespace const auto result = mLoader.load(file); - std::unique_ptr triangles(new btTriangleMesh(false)); - triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0)); - std::unique_ptr compound(new btCompoundShape); - compound->addChildShape(btTransform::getIdentity(), new Resource::TriangleMeshShape(triangles.release(), true)); - Resource::BulletShape expected; - expected.mCollisionShape.reset(compound.release()); EXPECT_EQ(*result, expected); } diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 66c7eea12d..ec46afec41 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -263,6 +263,32 @@ namespace NifBullet args.mAutogenerated = colNode == nullptr; + // Check for extra data + for (const auto& e : node.getExtraList()) + { + if (e->recType == Nif::RC_NiStringExtraData) + { + // String markers may contain important information + // affecting the entire subtree of this node + auto sd = static_cast(e.getPtr()); + + // Editor marker flag + if (sd->mData == "MRK") + args.mHasTriMarkers = true; + else if (Misc::StringUtils::ciStartsWith(sd->mData, "NC")) + { + // NC prefix is case-insensitive but the second C in NCC flag needs be uppercase. + + // Collide only with camera. + if (sd->mData.length() > 2 && sd->mData[2] == 'C') + mShape->mVisualCollisionType = Resource::VisualCollisionType::Camera; + // No collision. + else + mShape->mVisualCollisionType = Resource::VisualCollisionType::Default; + } + } + } + // FIXME: BulletNifLoader should never have to provide rendered geometry for camera collision if (colNode && colNode->mChildren.empty()) { @@ -323,35 +349,6 @@ namespace NifBullet if (node.recType == Nif::RC_AvoidNode) args.mAvoid = true; - // Check for extra data - for (const auto& e : node.getExtraList()) - { - if (e->recType == Nif::RC_NiStringExtraData) - { - // String markers may contain important information - // affecting the entire subtree of this node - auto sd = static_cast(e.getPtr()); - - if (Misc::StringUtils::ciStartsWith(sd->mData, "NC")) - { - // NCC flag in vanilla is partly case sensitive: prefix NC is case insensitive but second C needs be - // uppercase - if (sd->mData.length() > 2 && sd->mData[2] == 'C') - // Collide only with camera. - mShape->mVisualCollisionType = Resource::VisualCollisionType::Camera; - else - // No collision. - mShape->mVisualCollisionType = Resource::VisualCollisionType::Default; - } - // Don't autogenerate collision if MRK is set. - // FIXME: verify if this covers the entire subtree - else if (sd->mData == "MRK" && args.mAutogenerated) - { - return; - } - } - } - if ((args.mAutogenerated || args.mIsCollisionNode) && isTypeNiGeometry(node.recType)) handleNiTriShape(static_cast(node), parent, args); @@ -373,11 +370,14 @@ namespace NifBullet void BulletNifLoader::handleNiTriShape( const Nif::NiGeometry& niGeometry, const Nif::Parent* nodeParent, HandleNodeArgs args) { - // mHasMarkers is specifically BSXFlags editor marker flag. - // If this changes, the check must be corrected. + // This flag comes from BSXFlags if (args.mHasMarkers && Misc::StringUtils::ciStartsWith(niGeometry.mName, "EditorMarker")) return; + // This flag comes from Morrowind + if (args.mHasTriMarkers && Misc::StringUtils::ciStartsWith(niGeometry.mName, "Tri EditorMarker")) + return; + if (niGeometry.mData.empty() || niGeometry.mData->mVertices.empty()) return; diff --git a/components/nifbullet/bulletnifloader.hpp b/components/nifbullet/bulletnifloader.hpp index 521bbe91dd..c87c1242de 100644 --- a/components/nifbullet/bulletnifloader.hpp +++ b/components/nifbullet/bulletnifloader.hpp @@ -53,6 +53,7 @@ namespace NifBullet struct HandleNodeArgs { bool mHasMarkers{ false }; + bool mHasTriMarkers{ false }; bool mAnimated{ false }; bool mIsCollisionNode{ false }; bool mAutogenerated{ false };