qbsp: q1: fix detail to be non-sealing again

common/bspfile.cc

@@ -618,8 +618,7 @@ public:
         auto bits_a = contentflags_to_bits(a);
         auto bits_b = contentflags_to_bits(b);
 
-        if (bits_a.solid || bits_b.solid) {
+        if (contents_are_solid(a) || contents_are_solid(b)) {
-            // qbsp3 behaviour: clear any other set content flags
             return create_solid_contents();
         }
 

qbsp/prtfile.cc

@@ -58,7 +58,8 @@ static void WritePortals_r(node_t *node, std::ofstream &portalFile, bool cluster
         WritePortals_r(node->children[1], portalFile, clusters);
         return;
     }
-    if (node->contents.is_solid(qbsp_options.target_game))
+    // at this point, `node` may be a leaf or a cluster
+    if (node->is_leaf && node->contents.is_any_solid(qbsp_options.target_game))
         return;
 
     for (p = node->portals; p; p = next) {
@@ -98,14 +99,14 @@ static void WritePortals_r(node_t *node, std::ofstream &portalFile, bool cluster
     }
 }
 
-static int WriteClusters_r(node_t *node, std::ofstream &portalFile, int viscluster)
+static int WritePTR2ClusterMapping_r(node_t *node, std::ofstream &portalFile, int viscluster)
 {
     if (!node->is_leaf) {
-        viscluster = WriteClusters_r(node->children[0], portalFile, viscluster);
+        viscluster = WritePTR2ClusterMapping_r(node->children[0], portalFile, viscluster);
-        viscluster = WriteClusters_r(node->children[1], portalFile, viscluster);
+        viscluster = WritePTR2ClusterMapping_r(node->children[1], portalFile, viscluster);
         return viscluster;
     }
-    if (node->contents.is_solid(qbsp_options.target_game))
+    if (node->is_leaf && node->contents.is_any_solid(qbsp_options.target_game))
         return viscluster;
 
     /* If we're in the next cluster, start a new line */
@@ -173,7 +174,7 @@ static void NumberLeafs_r(node_t *node, portal_state_t &state, int cluster)
         return;
     }
 
-    if (node->contents.is_any_solid(qbsp_options.target_game)) {
+    if (node->is_leaf && node->contents.is_any_solid(qbsp_options.target_game)) {
         /* solid block, viewpoint never inside */
         node->visleafnum = -1;
         node->viscluster = -1;
@@ -237,7 +238,7 @@ static void WritePortalfile(node_t *headnode, portal_state_t &state)
         fmt::print(portalFile, "{}\n", state.num_visclusters.count.load());
         fmt::print(portalFile, "{}\n", state.num_visportals.count.load());
         WritePortals_r(headnode, portalFile, true);
-        check = WriteClusters_r(headnode, portalFile, 0);
+        check = WritePTR2ClusterMapping_r(headnode, portalFile, 0);
         if (check != state.num_visclusters.count.load() - 1) {
             FError("Internal error: Detail cluster mismatch");
         }

qbsp/writebsp.cc

@@ -233,7 +233,7 @@ static void ExportDrawNodes(node_t *node)
         if (node->children[i]->is_leaf) {
             // In Q2, all leaves must have their own ID even if they share solidity.
             if (qbsp_options.target_game->id != GAME_QUAKE_II &&
-                node->children[i]->contents.is_solid(qbsp_options.target_game)) {
+                node->children[i]->contents.is_any_solid(qbsp_options.target_game)) {
                 dnode->children[i] = PLANENUM_LEAF;
             } else {
                 int32_t nextLeafIndex = static_cast<int32_t>(map.bsp.dleafs.size());

tests/test_common.cc

@@ -52,6 +52,15 @@ TEST_SUITE("common")
             CHECK(combined.native == CONTENTS_WATER);
             CHECK(combined.is_detail_illusionary(game_q1));
         }
+
+        SUBCASE("detail_solid plus water")
+        {
+            auto combined = game_q1->combine_contents(detail_solid, contentflags_t{CONTENTS_WATER});
+
+            CHECK(combined.is_detail_solid(game_q1));
+            CHECK(!combined.is_liquid(game_q1));
+            CHECK(!combined.is_solid(game_q1));
+        }
     }
 
     TEST_CASE("shared content flag tests")

tests/test_qbsp.cc

@@ -854,14 +854,11 @@ TEST_CASE("detail_illusionary_noclipfaces_intersecting" * doctest::test_suite("t
     CHECK(prt->portalleafs == 1);
 }
 
-/**
+TEST_CASE("q1_detail_non_sealing" * doctest::test_suite("testmaps_q1"))
- * Since moving to a qbsp3 codebase, detail seals by default.
- */
-TEST_CASE("detail_seals" * doctest::test_suite("testmaps_q1"))
 {
-    const auto [bsp, bspx, prt] = LoadTestmapQ1("qbsp_detail_seals.map");
+    const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_detail_non_sealing.map");
 
-    CHECK(prt.has_value());
+    CHECK(!prt.has_value());
 }
 
 TEST_CASE("detail_doesnt_remove_world_nodes" * doctest::test_suite("testmaps_q1"))
@@ -887,16 +884,12 @@ TEST_CASE("detail_doesnt_remove_world_nodes" * doctest::test_suite("testmaps_q1"
     // make sure the detail face exists
     CHECK(nullptr != BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], {32, -72, 136}, {-1, 0, 0}));
 
-#if 0
-// fixme-brushbsp: with qbsp3 code, the strucutral node is actually clippped away.
-// we could repurpose this test case to test func_detail_wall (q2 window) in which case it would not be clipped away.
     {
         // but the sturctural nodes/leafs should not be clipped away by detail
         const qvec3d covered_by_detail{48, -88, 128};
         auto *covered_by_detail_node = BSP_FindNodeAtPoint(&bsp, &bsp.dmodels[0], covered_by_detail, {-1, 0, 0});
         CHECK(nullptr != covered_by_detail_node);
     }
-#endif
 }
 
 TEST_CASE("merge" * doctest::test_suite("testmaps_q1"))