Skip to content

Commit

Permalink
Merge branch 'ncroot' into 'master'
Browse files Browse the repository at this point in the history 
Improve BulletNifLoader handling of extra data

See merge request OpenMW/openmw!3590
  • Loading branch information
@jvoisin
jvoisin committed Nov 15, 2023
2 parents 710b560 + c7d5ea9 commit 9b1cb99
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 78 deletions.
89 changes: 42 additions & 47 deletions apps/openmw_test_suite/nifloader/testbulletnifloader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -1012,13 +1012,13 @@ namespace
EXPECT_EQ(*result, expected);
}

TEST_F(TestBulletNifLoader,
for_tri_shape_child_node_with_extra_data_string_starting_with_nc_should_return_shape_with_nocollision)
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.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");
Expand All @@ -1040,13 +1040,13 @@ namespace
}

TEST_F(TestBulletNifLoader,
for_tri_shape_child_node_with_not_first_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.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");
Expand All @@ -1067,6 +1067,31 @@ namespace
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;
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<btTriangleMesh> triangles(new btTriangleMesh(false));
triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
std::unique_ptr<btCompoundShape> 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);
}

TEST_F(TestBulletNifLoader, for_empty_root_collision_node_without_nc_should_return_shape_with_cameraonly_collision)
{
Nif::NiTriShape niTriShape;
Expand Down Expand Up @@ -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");
Expand All @@ -1142,32 +1147,22 @@ 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);
file.mHash = mHash;

const auto result = mLoader.load(file);

std::unique_ptr<btTriangleMesh> triangles(new btTriangleMesh(false));
triangles->addTriangle(btVector3(0, 0, 0), btVector3(1, 0, 0), btVector3(1, 1, 0));
std::unique_ptr<btCompoundShape> 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);
}
Expand Down
62 changes: 31 additions & 31 deletions components/nifbullet/bulletnifloader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<const Nif::NiStringExtraData*>(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())
{
Expand Down Expand Up @@ -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<const Nif::NiStringExtraData*>(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<const Nif::NiGeometry&>(node), parent, args);

Expand All @@ -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;

Expand Down
1 change: 1 addition & 0 deletions components/nifbullet/bulletnifloader.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace NifBullet
struct HandleNodeArgs
{
bool mHasMarkers{ false };
bool mHasTriMarkers{ false };
bool mAnimated{ false };
bool mIsCollisionNode{ false };
bool mAutogenerated{ false };
Expand Down

0 comments on commit 9b1cb99

Please sign in to comment.