diff --git a/common/bspfile.cc b/common/bspfile.cc index f79d0ec0..93dbe0cf 100644 --- a/common/bspfile.cc +++ b/common/bspfile.cc @@ -1211,15 +1211,10 @@ struct gamedef_q2_t : public gamedef_t contentflags_t contents_remap_for_export(const contentflags_t &contents, remap_type_t type) const override { if (contents.flags & EWT_VISCONTENTS_DETAIL_WALL) { - return create_solid_contents(); - } - // Solid wipes out any other contents - // Previously, this was done in LeafNode but we've changed to detail-solid being - // non-sealing. - if (type == remap_type_t::leaf) { - if (contents.flags & EWT_VISCONTENTS_SOLID) { - return create_solid_contents(); - } + contents_int_t result = contents.flags; + result &= (~EWT_VISCONTENTS_DETAIL_WALL); + result |= EWT_VISCONTENTS_SOLID; + return contentflags_t::make(result); } return contents; @@ -1227,14 +1222,15 @@ struct gamedef_q2_t : public gamedef_t contentflags_t combine_contents(const contentflags_t &a, const contentflags_t &b) const override { - // structural solid (but not detail solid) eats any other contents + contents_int_t bits_a = a.flags; + contents_int_t bits_b = b.flags; + + // structural solid eats detail flags if (contents_are_solid(a) || contents_are_solid(b)) { - return create_solid_contents(); + bits_a &= ~EWT_CFLAG_DETAIL; + bits_b &= ~EWT_CFLAG_DETAIL; } - auto bits_a = a.flags; - auto bits_b = b.flags; - return contentflags_t::make(bits_a | bits_b); } diff --git a/qbsp/tree.cc b/qbsp/tree.cc index e1e2ff13..facd1780 100644 --- a/qbsp/tree.cc +++ b/qbsp/tree.cc @@ -135,8 +135,11 @@ static void PruneNodes_R(node_t *node, prune_stats_t &stats) // fixme-brushbsp: is it correct to strip off detail flags here? if (IsAnySolidLeaf(nodedata->children[0]) && IsAnySolidLeaf(nodedata->children[1])) { + contentflags_t merged_contents = qbsp_options.target_game->combine_contents(nodedata->children[0]->get_leafdata()->contents, + nodedata->children[1]->get_leafdata()->contents); + // This discards any faces on-node. Should be safe (?) - ConvertNodeToLeaf(node, qbsp_options.target_game->create_solid_contents()); + ConvertNodeToLeaf(node, merged_contents); stats.nodes_pruned++; } diff --git a/tests/test_common.cc b/tests/test_common.cc index e00f4493..678e7835 100644 --- a/tests/test_common.cc +++ b/tests/test_common.cc @@ -222,37 +222,44 @@ TEST_SUITE("common") { auto *game_q2 = bspver_q2.game; - const std::array test_contents{ - game_q2->create_contents_from_native(Q2_CONTENTS_EMPTY), - game_q2->create_contents_from_native(Q2_CONTENTS_SOLID), - game_q2->create_contents_from_native(Q2_CONTENTS_WINDOW), - game_q2->create_contents_from_native(Q2_CONTENTS_AUX), - game_q2->create_contents_from_native(Q2_CONTENTS_LAVA), - game_q2->create_contents_from_native(Q2_CONTENTS_SLIME), - game_q2->create_contents_from_native(Q2_CONTENTS_WATER), - game_q2->create_contents_from_native(Q2_CONTENTS_MIST), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_SOLID), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_WINDOW), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_AUX), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_LAVA), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_SLIME), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_WATER), - game_q2->create_contents_from_native(Q2_CONTENTS_DETAIL | Q2_CONTENTS_MIST) + struct before_after_t { + int32_t before; + int32_t after; }; SUBCASE("solid combined with others") { + const std::vector before_after_adding_solid { + {Q2_CONTENTS_EMPTY, Q2_CONTENTS_SOLID}, + {Q2_CONTENTS_SOLID, Q2_CONTENTS_SOLID}, + {Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER, Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER}, + {Q2_CONTENTS_WINDOW, Q2_CONTENTS_SOLID | Q2_CONTENTS_WINDOW}, + {Q2_CONTENTS_AUX, Q2_CONTENTS_SOLID | Q2_CONTENTS_AUX}, + {Q2_CONTENTS_LAVA, Q2_CONTENTS_SOLID | Q2_CONTENTS_LAVA}, + {Q2_CONTENTS_SLIME, Q2_CONTENTS_SOLID | Q2_CONTENTS_SLIME}, + {Q2_CONTENTS_WATER, Q2_CONTENTS_SOLID | Q2_CONTENTS_WATER}, + {Q2_CONTENTS_MIST, Q2_CONTENTS_SOLID | Q2_CONTENTS_MIST}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_SOLID, Q2_CONTENTS_SOLID}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_WINDOW, Q2_CONTENTS_SOLID | Q2_CONTENTS_WINDOW}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_AUX, Q2_CONTENTS_SOLID |Q2_CONTENTS_AUX}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_LAVA, Q2_CONTENTS_SOLID |Q2_CONTENTS_LAVA}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_SLIME, Q2_CONTENTS_SOLID |Q2_CONTENTS_SLIME}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_WATER, Q2_CONTENTS_SOLID | Q2_CONTENTS_WATER}, // detail flag gets erased + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_MIST, Q2_CONTENTS_SOLID | Q2_CONTENTS_MIST} // detail flag gets erased + }; + auto solid = game_q2->create_solid_contents(); CHECK(game_q2->contents_to_native(solid) == Q2_CONTENTS_SOLID); - for (const auto &c : test_contents) { - // solid is treated specially in Q2 and wipes out any other content - // flags when combined + for (const auto &[before, after] : before_after_adding_solid) { + auto combined = game_q2->contents_remap_for_export( - game_q2->combine_contents(solid, c), gamedef_t::remap_type_t::leaf); + game_q2->combine_contents(game_q2->create_contents_from_native(before), + solid), gamedef_t::remap_type_t::leaf); - CHECK(game_q2->contents_to_native(combined) == Q2_CONTENTS_SOLID); + CHECK(game_q2->contents_to_native(combined) == after); CHECK(combined.is_solid(game_q2)); + CHECK(!combined.is_any_detail(game_q2)); } } @@ -260,14 +267,30 @@ TEST_SUITE("common") { contentflags_t water = game_q2->create_contents_from_native(Q2_CONTENTS_WATER); - for (const auto &c : test_contents) { - auto combined = game_q2->combine_contents(water, c); - - SUBCASE(fmt::format("water combined with {}", c.to_string(game_q2)).c_str()) + const std::vector before_after_adding_water { + {Q2_CONTENTS_EMPTY, Q2_CONTENTS_WATER}, + {Q2_CONTENTS_SOLID, Q2_CONTENTS_WATER | Q2_CONTENTS_SOLID}, + {Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER, Q2_CONTENTS_WATER | Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER}, + {Q2_CONTENTS_WINDOW, Q2_CONTENTS_WATER | Q2_CONTENTS_WINDOW}, + {Q2_CONTENTS_AUX, Q2_CONTENTS_WATER | Q2_CONTENTS_AUX}, + {Q2_CONTENTS_LAVA, Q2_CONTENTS_WATER | Q2_CONTENTS_LAVA}, + {Q2_CONTENTS_SLIME, Q2_CONTENTS_WATER | Q2_CONTENTS_SLIME}, + {Q2_CONTENTS_WATER, Q2_CONTENTS_WATER}, + {Q2_CONTENTS_MIST, Q2_CONTENTS_WATER | Q2_CONTENTS_MIST}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_SOLID, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_SOLID}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_WINDOW, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_WINDOW}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_AUX, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_AUX}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_LAVA, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_LAVA}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_SLIME, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_SLIME}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_WATER, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL}, + {Q2_CONTENTS_DETAIL | Q2_CONTENTS_MIST, Q2_CONTENTS_WATER | Q2_CONTENTS_DETAIL | Q2_CONTENTS_MIST} + }; + for (const auto &[before, after] : before_after_adding_water) { + auto combined = game_q2->combine_contents(game_q2->create_contents_from_native(before), water); + + SUBCASE(fmt::format("water combined with {}", game_q2->create_contents_from_native(before).to_string(game_q2)).c_str()) { - if (!(game_q2->contents_to_native(c) & Q2_CONTENTS_SOLID)) { - CHECK(game_q2->contents_to_native(combined) == (Q2_CONTENTS_WATER | game_q2->contents_to_native(c))); - } + CHECK(game_q2->contents_to_native(combined) == after); } } } diff --git a/tests/test_qbsp_q2.cc b/tests/test_qbsp_q2.cc index 569ab9bb..80c2dea9 100644 --- a/tests/test_qbsp_q2.cc +++ b/tests/test_qbsp_q2.cc @@ -48,12 +48,11 @@ TEST_CASE("detail" * doctest::test_suite("testmaps_q2")) for (size_t i = 1; i < bsp.dleafs.size(); ++i) { ++counts_by_contents[bsp.dleafs[i].contents]; } - CHECK(2 == counts_by_contents.size()); // number of types + CHECK(3 == counts_by_contents.size()); // number of types - CHECK(counts_by_contents.find(Q2_CONTENTS_SOLID | Q2_CONTENTS_DETAIL) == - counts_by_contents.end()); // the detail bit gets cleared + CHECK(1 == counts_by_contents.at(Q2_CONTENTS_SOLID | Q2_CONTENTS_DETAIL)); // detail leafs CHECK(8 == counts_by_contents.at(0)); // empty leafs - CHECK(counts_by_contents.at(Q2_CONTENTS_SOLID) >= 8); + CHECK(counts_by_contents.at(Q2_CONTENTS_SOLID) >= 6); CHECK(counts_by_contents.at(Q2_CONTENTS_SOLID) <= 12); // clusters: @@ -84,7 +83,7 @@ TEST_CASE("detail" * doctest::test_suite("testmaps_q2")) // check for correct contents auto *detail_leaf = BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], inside_button); - CHECK(Q2_CONTENTS_SOLID == detail_leaf->contents); + CHECK((Q2_CONTENTS_SOLID | Q2_CONTENTS_DETAIL) == detail_leaf->contents); CHECK(-1 == detail_leaf->cluster); CHECK(0 == detail_leaf->area); // solid leafs get the invalid area 0 @@ -541,7 +540,7 @@ TEST_CASE("q2_detail_overlapping_solid_sealing" * doctest::test_suite("testmaps_ // check leaf contents CHECK(Q2_CONTENTS_EMPTY == BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_start_room)->contents); - CHECK(Q2_CONTENTS_SOLID == BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_void)->contents); + CHECK((Q2_CONTENTS_SOLID & BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_void)->contents) == Q2_CONTENTS_SOLID); } /** @@ -719,8 +718,10 @@ TEST_CASE("q2_ladder" * doctest::test_suite("testmaps_q2")) auto *leaf = BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], point_in_ladder); - // the brush lacked a visible contents, so it became solid. solid leafs wipe out any other content bits - CHECK(leaf->contents == (Q2_CONTENTS_SOLID)); + // the brush lacked a visible contents, so it became solid. + // ladder and detail flags are preseved now. + // (previously we were wiping them out and just writing out leafs as Q2_CONTENTS_SOLID). + CHECK(leaf->contents == (Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER | Q2_CONTENTS_DETAIL)); CHECK(1 == Leaf_Brushes(&bsp, leaf).size()); CHECK((Q2_CONTENTS_SOLID | Q2_CONTENTS_LADDER | Q2_CONTENTS_DETAIL) == Leaf_Brushes(&bsp, leaf).at(0)->contents); @@ -774,13 +775,13 @@ TEST_CASE("q2_detail_wall" * doctest::test_suite("testmaps_q2")) INFO("check leaf / brush contents"); CAPTURE(game->create_contents_from_native(detail_wall_leaf->contents).to_string(game)); - CHECK(Q2_CONTENTS_SOLID == detail_wall_leaf->contents); + CHECK((Q2_CONTENTS_SOLID | Q2_CONTENTS_DETAIL) == detail_wall_leaf->contents); REQUIRE(1 == Leaf_Brushes(&bsp, detail_wall_leaf).size()); auto *brush = Leaf_Brushes(&bsp, detail_wall_leaf).at(0); CAPTURE(game->create_contents_from_native(brush->contents).to_string(game)); - CHECK(Q2_CONTENTS_SOLID == brush->contents); + CHECK((Q2_CONTENTS_SOLID | Q2_CONTENTS_DETAIL) == brush->contents); } { @@ -1059,7 +1060,7 @@ TEST_CASE("q2_unknown_contents" * doctest::test_suite("testmaps_q2")) auto *leaf = BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], {0, 0, 0}); // FIXME: should the unknown contents get converted to SOLID in the leaf? - CHECK(leaf->contents == (Q2_CONTENTS_SOLID)); + CHECK(leaf->contents == (Q2_CONTENTS_SOLID | 1024)); CHECK(1 == Leaf_Brushes(&bsp, leaf).size()); // FIXME: should the unknown contents have SOLID added in the brush? @@ -1073,7 +1074,7 @@ TEST_CASE("q2_unknown_contents" * doctest::test_suite("testmaps_q2")) auto *leaf = BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], {64, 0, 0}); // FIXME: should the unknown contents get converted to SOLID in the leaf? - CHECK(leaf->contents == (Q2_CONTENTS_SOLID)); + CHECK(leaf->contents == (Q2_CONTENTS_SOLID | nth_bit(30))); CHECK(1 == Leaf_Brushes(&bsp, leaf).size()); // FIXME: should the unknown contents have SOLID added in the brush?