#5901: pointfile identification is now case-insensitive

On Windows there is no guarantee that TDM will write out a .lin file
with the same case as the map name, for example a map named "Leak.map"
may end up with a pointfile "leak_portal_xxx.lin". We now use
case-insensitive string comparisons when identifying matching
pointfiles, so that pointfiles of any case will appear in the list.

This might give wrong results if a mapper on Linux is working on both
"MAP.map" and "map.map" in the same directory, but that seems a very
unlikely situation, and the only ill effect is that the pointfile list
might include more pointfiles than necessary.

libs/string/string.h

@@ -8,16 +8,20 @@
 namespace string
 {
 
-/// \brief Returns <0 if \p string is lexicographically less than \p other after converting both to lower-case.
-/// Returns >0 if \p string is lexicographically greater than \p other after converting both to lower-case.
-/// Returns 0 if \p string is lexicographically equal to \p other after converting both to lower-case.
-/// O(n)
-inline int icmp(const char* string, const char* other)
+/**
+ * @brief Case-insensitive string comparison.
+ *
+ * Behaves like the standard strcmp() but ignores case.
+ *
+ * @return A negative integer if lhs is lexicographically less than rhs (ignoring case), 0 if both
+ * are equal, or a positive integer if lhs is greater than rhs.
+ */
+inline int icmp(const char* lhs, const char* rhs)
 {
 #ifdef WIN32
-  return _stricmp(string, other);
+  return _stricmp(lhs, rhs);
 #else
-  return strcasecmp(string, other);
+  return strcasecmp(lhs, rhs);
 #endif
 }
 

radiantcore/map/Map.cpp

@@ -312,21 +312,16 @@ bool Map::isUnnamed() const {
 
 namespace
 {
-    bool pointfileNameMatch(const std::string& candidate,
-                            const std::string& mapStem)
-    {
-        // A matching point file either has an identical stem to the map file,
-        // or the map file stem with an underscore suffix (e.g.
-        // "mapfile_portal_123_456.lin")
-        if (candidate == mapStem)
-            return true;
-        else if (candidate.rfind(mapStem + "_", 0) == 0)
-            return true;
-        else
-            return false;
-    }
+
+bool pointfileNameMatch(const std::string& candidate, const std::string& mapStem)
+{
+    // A matching point file either has an identical stem to the map file, or the map file stem
+    // with an underscore suffix (e.g. "mapfile_portal_123_456.lin")
+    return string::iequals(candidate, mapStem) || string::istarts_with(candidate, mapStem + "_");
 }
 
+} // namespace
+
 void Map::forEachPointfile(PointfileFunctor func) const
 {
     static const char* LIN_EXT = ".lin";
@@ -597,7 +592,7 @@ bool Map::save(const MapFormatPtr& mapFormat)
     }
 
     // Check if the map file has been modified in the meantime
-    if (_resource->fileOnDiskHasBeenModifiedSinceLastSave() && 
+    if (_resource->fileOnDiskHasBeenModifiedSinceLastSave() &&
         !radiant::FileOverwriteConfirmation::SendAndReceiveAnswer(
             fmt::format(_("The file {0} has been modified since it was last saved,\nperhaps by another application. "
                 "Do you really want to overwrite the file?"), _mapName), _("File modification detected")))
@@ -964,11 +959,11 @@ void Map::registerCommands()
     GlobalCommandSystem().addCommand("OpenMap", Map::openMap, { cmd::ARGTYPE_STRING | cmd::ARGTYPE_OPTIONAL });
     GlobalCommandSystem().addCommand("OpenMapFromArchive", Map::openMapFromArchive, { cmd::ARGTYPE_STRING, cmd::ARGTYPE_STRING });
     GlobalCommandSystem().addCommand("ImportMap", Map::importMap);
-    GlobalCommandSystem().addCommand("StartMergeOperation", std::bind(&Map::startMergeOperationCmd, this, std::placeholders::_1), 
+    GlobalCommandSystem().addCommand("StartMergeOperation", std::bind(&Map::startMergeOperationCmd, this, std::placeholders::_1),
         { cmd::ARGTYPE_STRING | cmd::ARGTYPE_OPTIONAL, cmd::ARGTYPE_STRING | cmd::ARGTYPE_OPTIONAL });
     GlobalCommandSystem().addCommand("AbortMergeOperation", std::bind(&Map::abortMergeOperationCmd, this, std::placeholders::_1));
     GlobalCommandSystem().addCommand("FinishMergeOperation", std::bind(&Map::finishMergeOperationCmd, this, std::placeholders::_1));
-    GlobalCommandSystem().addCommand(LOAD_PREFAB_AT_CMD, std::bind(&Map::loadPrefabAt, this, std::placeholders::_1), 
+    GlobalCommandSystem().addCommand(LOAD_PREFAB_AT_CMD, std::bind(&Map::loadPrefabAt, this, std::placeholders::_1),
         { cmd::ARGTYPE_STRING, cmd::ARGTYPE_VECTOR3, cmd::ARGTYPE_INT|cmd::ARGTYPE_OPTIONAL, cmd::ARGTYPE_INT | cmd::ARGTYPE_OPTIONAL });
     GlobalCommandSystem().addCommand("SaveSelectedAsPrefab", Map::saveSelectedAsPrefab);
     GlobalCommandSystem().addCommand("SaveMap", std::bind(&Map::saveMapCmd, this, std::placeholders::_1));

test/PointTrace.cpp

@@ -64,9 +64,10 @@ TEST_F(PointTraceTest, IdentifyMapPointfiles)
 
     // Check the pointfiles for this map
     auto pfs = pointfiles();
-    ASSERT_EQ(pfs.size(), 2);
-    EXPECT_EQ(pfs[0].filename(), "altar.lin");
-    EXPECT_EQ(pfs[1].filename(), "altar_portalL_544_64_112.lin");
+    ASSERT_EQ(pfs.size(), 3);
+    EXPECT_EQ(pfs[0].filename(), "ALTAr.lin");
+    EXPECT_EQ(pfs[1].filename(), "altar.lin");
+    EXPECT_EQ(pfs[2].filename(), "altar_portalL_544_64_112.lin");
 }
 
 TEST_F(PointTraceTest, PointFilesAssociatedWithCorrectMap)

test/resources/tdm/maps/ALTAr.lin

@@ -0,0 +1,5 @@
+544.000000 64.000000 112.000000
+544.000000 64.000000 240.000000
+512.000000 64.000000 240.000000
+512.000000 64.000000 112.000000
+544.000000 64.000000 112.000000