From 35fe26b7a68496b47943af6caf23d4589277551b Mon Sep 17 00:00:00 2001
From: Eric Wasylishen <ewasylishen@gmail.com>
Date: Mon, 19 Dec 2022 00:55:20 -0700
Subject: [PATCH] qbsp: fix FindTargetEntity (was breaking hiprotate)

---
 common/entdata.cc           |   2 +-
 include/common/entdata.h    |   2 +-
 qbsp/brush.cc               |   5 +-
 testmaps/qbsp_hiprotate.map | 104 ++++++++++++++++++++++++++++++++++++
 tests/test_qbsp.cc          |  39 ++++++++------
 5 files changed, 134 insertions(+), 18 deletions(-)
 create mode 100644 testmaps/qbsp_hiprotate.map

diff --git a/common/entdata.cc b/common/entdata.cc
index f05b000f..a7ab815c 100644
--- a/common/entdata.cc
+++ b/common/entdata.cc
@@ -32,7 +32,7 @@ entdict_t::entdict_t() = default;
 
 entdict_t::entdict_t(parser_base_t &parser) { parse(parser); }
 
-std::string entdict_t::get(const std::string_view &key) const
+const std::string &entdict_t::get(const std::string_view &key) const
 {
     if (auto it = find(key); it != keyvalues.end()) {
         return it->second;
diff --git a/include/common/entdata.h b/include/common/entdata.h
index 45dd44ef..5729281a 100644
--- a/include/common/entdata.h
+++ b/include/common/entdata.h
@@ -42,7 +42,7 @@ public:
     entdict_t();
     entdict_t(parser_base_t &parser);
 
-    std::string get(const std::string_view &key) const;
+    const std::string &get(const std::string_view &key) const;
     vec_t get_float(const std::string_view &key) const;
     int32_t get_int(const std::string_view &key) const;
     // returns number of vector components read
diff --git a/qbsp/brush.cc b/qbsp/brush.cc
index 5b4ebe02..92f6e49c 100644
--- a/qbsp/brush.cc
+++ b/qbsp/brush.cc
@@ -206,14 +206,17 @@ static void CheckFace(side_t *face, const mapface_t &sourceface, std::optional<s
 /*
 =================
 FindTargetEntity
+
+Finds the entity whose `targetname` value is case-insensitve-equal to `target`.
 =================
 */
 static const mapentity_t *FindTargetEntity(const std::string &target)
 {
     for (const auto &entity : map.entities) {
         const std::string &name = entity.epairs.get("targetname");
-        if (!string_iequals(target, name))
+        if (string_iequals(target, name)) {
             return &entity;
+        }
     }
 
     return nullptr;
diff --git a/testmaps/qbsp_hiprotate.map b/testmaps/qbsp_hiprotate.map
new file mode 100644
index 00000000..324fec89
--- /dev/null
+++ b/testmaps/qbsp_hiprotate.map
@@ -0,0 +1,104 @@
+// Game: Quake
+// Format: Valve
+// entity 0
+{
+"mapversion" "220"
+"classname" "worldspawn"
+"wad" "deprecated/free_wad.wad;deprecated/fence.wad;deprecated/origin.wad;deprecated/hintskip.wad"
+"_wateralpha" "0.5"
+"_tb_def" "builtin:Quoth2.fgd"
+// brush 0
+{
+( -176 -256 64 ) ( -176 -255 64 ) ( -176 -256 65 ) tsl_wall1 [ 0 -1 0 0 ] [ 0 0 -1 -32 ] 0 1 1
+( -176 -432 64 ) ( -176 -432 65 ) ( -175 -432 64 ) tsl_wall1 [ 1 0 0 16 ] [ 0 0 -1 -32 ] 0 1 1
+( -176 -256 96 ) ( -175 -256 96 ) ( -176 -255 96 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 -1 0 0 ] 0 1 1
+( -160 192 352 ) ( -160 193 352 ) ( -159 192 352 ) tsl_wall1 [ 1 0 0 16 ] [ 0 -1 0 0 ] 0 1 1
+( -160 176 80 ) ( -159 176 80 ) ( -160 176 81 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 0 -1 -32 ] 0 1 1
+( -160 192 80 ) ( -160 192 81 ) ( -160 193 80 ) tsl_wall1 [ 0 1 0 0 ] [ 0 0 -1 -32 ] 0 1 1
+}
+// brush 1
+{
+( -160 176 88 ) ( -160 177 88 ) ( -160 176 89 ) tsl_wall1 [ 0 1.0000000000000002 0 -32 ] [ 0 0 -1.0000000000000002 -32 ] 0 1 1
+( -160 176 88 ) ( -160 176 89 ) ( -159 176 88 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 0 -1 -32 ] 0 1 1
+( -160 176 96 ) ( -159 176 96 ) ( -160 177 96 ) tsl_wall1 [ -1.0000000000000002 0 0 -16 ] [ 0 1.0000000000000002 0 -40 ] 0 1 1
+( 288 192 352 ) ( 288 193 352 ) ( 289 192 352 ) tsl_wall1 [ -1.0000000000000002 0 0 -16 ] [ 0 -1.0000000000000002 0 48 ] 0 1 1
+( 288 192 96 ) ( 289 192 96 ) ( 288 192 97 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 0 -1 -32 ] 0 1 1
+( 288 192 96 ) ( 288 192 97 ) ( 288 193 96 ) tsl_wall1 [ 0 -1.0000000000000002 0 0 ] [ 0 0 -1.0000000000000002 -32 ] 0 1 1
+}
+// brush 2
+{
+( -160 -112 96 ) ( -160 -111 96 ) ( -160 -112 97 ) orangestuff8 [ 0 1 0 -16 ] [ 0 0 -1 0 ] 0 1 1
+( -80 -432 80 ) ( -81 -432 80 ) ( -80 -432 81 ) orangestuff8 [ -1 0 0 16 ] [ 0 0 -1 0 ] 180 1 1
+( -80 -432 80 ) ( -80 -431 80 ) ( -81 -432 80 ) orangestuff8 [ 1 0 0 -16 ] [ 0 -1 0 16 ] 180 1 1
+( -160 -112 96 ) ( -161 -112 96 ) ( -160 -111 96 ) orangestuff8 [ -1 0 0 16 ] [ 0 -1 0 16 ] 180 1 1
+( -160 176 96 ) ( -160 176 97 ) ( -161 176 96 ) orangestuff8 [ 1 0 0 -16 ] [ 0 0 -1 0 ] 180 1 1
+( 288 -432 80 ) ( 288 -432 81 ) ( 288 -431 80 ) orangestuff8 [ 0 -1 0 16 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 3
+{
+( -160 -448 88 ) ( -160 -447 88 ) ( -160 -448 89 ) tsl_wall1 [ 0 1.0000000000000002 0 80 ] [ 0 0 -1.0000000000000002 -32 ] 0 1 1
+( -160 -448 88 ) ( -160 -448 89 ) ( -159 -448 88 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 0 -1 -32 ] 0 1 1
+( -160 -448 96 ) ( -159 -448 96 ) ( -160 -447 96 ) tsl_wall1 [ -1.0000000000000002 0 0 -16 ] [ 0 1.0000000000000002 0 72 ] 0 1 1
+( 288 -432 352 ) ( 288 -431 352 ) ( 289 -432 352 ) tsl_wall1 [ -1.0000000000000002 0 0 -16 ] [ 0 -1.0000000000000002 0 -64 ] 0 1 1
+( 288 -432 96 ) ( 289 -432 96 ) ( 288 -432 97 ) tsl_wall1 [ -1 0 0 -16 ] [ 0 0 -1 -32 ] 0 1 1
+( 288 -432 96 ) ( 288 -432 97 ) ( 288 -431 96 ) tsl_wall1 [ 0 -1.0000000000000002 0 -112 ] [ 0 0 -1.0000000000000002 -32 ] 0 1 1
+}
+// brush 4
+{
+( -160 -256 352 ) ( -160 -255 352 ) ( -160 -256 353 ) orangestuff8 [ 0 0 -1.0000000000000002 0 ] [ 0 -1.0000000000000002 0 0 ] 180 1 1
+( 288 -432 360 ) ( 288 -432 361 ) ( 289 -432 360 ) orangestuff8 [ -1.0000000000000002 0 0 0 ] [ 0 0 1.0000000000000002 -16 ] 180 1 1
+( -160 -256 352 ) ( -159 -256 352 ) ( -160 -255 352 ) orangestuff8 [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1
+( 288 176 368 ) ( 288 177 368 ) ( 289 176 368 ) orangestuff8 [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1
+( 288 176 360 ) ( 289 176 360 ) ( 288 176 361 ) orangestuff8 [ -1.0000000000000002 0 0 0 ] [ 0 0 1.0000000000000002 -16 ] 180 1 1
+( 288 176 360 ) ( 288 176 361 ) ( 288 177 360 ) orangestuff8 [ 0 0 1.0000000000000002 0 ] [ 0 -1.0000000000000002 0 0 ] 180 1 1
+}
+// brush 5
+{
+( 288 192 80 ) ( 288 193 80 ) ( 288 192 81 ) tsl_wall1 [ 0 1 0 0 ] [ 0 0 -1 -32 ] 0 1 1
+( 304 -432 64 ) ( 303 -432 64 ) ( 304 -432 65 ) tsl_wall1 [ -1 0 0 16 ] [ 0 0 -1 -32 ] 180 1 1
+( 304 -256 96 ) ( 304 -255 96 ) ( 303 -256 96 ) tsl_wall1 [ 1 0 0 -16 ] [ 0 -1 0 0 ] 180 1 1
+( 304 -256 352 ) ( 303 -256 352 ) ( 304 -255 352 ) tsl_wall1 [ 1 0 0 -16 ] [ 0 -1 0 0 ] 180 1 1
+( 288 176 80 ) ( 288 176 81 ) ( 287 176 80 ) tsl_wall1 [ 1 0 0 -16 ] [ 0 0 -1 -32 ] 180 1 1
+( 304 -256 64 ) ( 304 -256 65 ) ( 304 -255 64 ) tsl_wall1 [ 0 -1 0 0 ] [ 0 0 -1 -32 ] 0 1 1
+}
+}
+// entity 1
+{
+"classname" "light"
+"origin" "72 -136 168"
+"light" "3000"
+}
+// entity 2
+{
+"classname" "info_player_start"
+"origin" "-88 -64 120"
+}
+// entity 3
+{
+"classname" "rotate_object"
+"targetname" "fan"
+"target" "fan_inforotate"
+// brush 0
+{
+( 168 -192 344 ) ( 168 -240 344 ) ( 168 -240 336 ) bolt14 [ -0.7071067811865477 0.7071067811865477 0 0 ] [ 0 0 -1.0000000000000002 0 ] 0 1 1
+( 168 -240 344 ) ( 264 -240 344 ) ( 264 -240 336 ) bolt14 [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 264 -240 336 ) ( 264 -192 336 ) ( 168 -192 336 ) bolt14 [ -1 0 0 24 ] [ 0 -1 0 20 ] 0 1 1
+( 168 -192 344 ) ( 264 -192 344 ) ( 264 -240 344 ) bolt14 [ 1 0 0 -24 ] [ 0 -1 0 20 ] 0 1 1
+( 264 -192 336 ) ( 264 -192 344 ) ( 168 -192 344 ) bolt14 [ -1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 264 -240 344 ) ( 264 -192 344 ) ( 264 -192 336 ) bolt14 [ 0.7071067811865477 -0.7071067811865477 0 0 ] [ 0 0 -1.0000000000000002 0 ] 0 1 1
+}
+}
+// entity 4
+{
+"classname" "func_rotate_entity"
+"origin" "216 -216 340"
+"target" "fan"
+"rotate" "0 -90 0"
+"spawnflags" "2"
+}
+// entity 5
+{
+"classname" "info_rotate"
+"origin" "216 -216 340"
+"targetname" "fan_inforotate"
+}
diff --git a/tests/test_qbsp.cc b/tests/test_qbsp.cc
index 7df01aa1..0ebc0893 100644
--- a/tests/test_qbsp.cc
+++ b/tests/test_qbsp.cc
@@ -992,27 +992,36 @@ TEST_CASE("brush_clipping_order" * doctest::test_suite("testmaps_q1"))
  */
 TEST_CASE("origin" * doctest::test_suite("testmaps_q1"))
 {
-    const auto [bsp, bspx, prt] = LoadTestmapQ1("qbsp_origin.map");
+    const std::vector<std::string> maps{
+        "qbsp_origin.map",
+        "qbsp_hiprotate.map" // same, but uses info_rotate instead of an origin brush
+    };
 
-    REQUIRE(prt.has_value());
+    for (const auto& map : maps) {
+        SUBCASE(map.c_str()) {
+            const auto [bsp, bspx, prt] = LoadTestmapQ1(map);
 
-    // 0 = world, 1 = rotate_object
-    REQUIRE(2 == bsp.dmodels.size());
+            REQUIRE(prt.has_value());
 
-    // check that the origin brush didn't clip away any solid faces, or generate faces
-    REQUIRE(6 == bsp.dmodels[1].numfaces);
+            // 0 = world, 1 = rotate_object
+            REQUIRE(2 == bsp.dmodels.size());
 
-    // FIXME: should the origin brush update the dmodel's origin too?
-    REQUIRE(qvec3f(0, 0, 0) == bsp.dmodels[1].origin);
+            // check that the origin brush didn't clip away any solid faces, or generate faces
+            REQUIRE(6 == bsp.dmodels[1].numfaces);
 
-    // check that the origin brush updated the entity lump
-    parser_t parser(bsp.dentdata, { "qbsp_origin.bsp" });
-    auto ents = EntData_Parse(parser);
-    auto it = std::find_if(ents.begin(), ents.end(), 
-        [](const entdict_t &dict) -> bool { return dict.get("classname") == "rotate_object"; });
+            // FIXME: should the origin brush update the dmodel's origin too?
+            REQUIRE(qvec3f(0, 0, 0) == bsp.dmodels[1].origin);
 
-    REQUIRE(it != ents.end());
-    CHECK(it->get("origin") == "216 -216 340");
+            // check that the origin brush updated the entity lump
+            parser_t parser(bsp.dentdata, {"qbsp_origin.bsp"});
+            auto ents = EntData_Parse(parser);
+            auto it = std::find_if(ents.begin(), ents.end(),
+                [](const entdict_t &dict) -> bool { return dict.get("classname") == "rotate_object"; });
+
+            REQUIRE(it != ents.end());
+            CHECK(it->get("origin") == "216 -216 340");
+        }
+    }
 }
 
 TEST_CASE("simple" * doctest::test_suite("testmaps_q1"))