From e35595a1eb885577be01d9323921f3e891cd4d2e Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Mon, 13 Mar 2023 01:03:33 +0000
Subject: [PATCH] StrCat improvements

1. Use `fmt::format_int` directly instead of parsing a format string.
2. Use `AppendStrView`.
3. Define the varargs versions using fold expressions when available.
4. Add tests.
---
 Source/qol/floatingnumbers.cpp |  4 +++-
 Source/utils/str_cat.cpp       | 20 ++++++--------------
 Source/utils/str_cat.hpp       | 26 +++++++++++++++++++++-----
 test/CMakeLists.txt            |  1 +
 test/str_cat_test.cpp          | 21 +++++++++++++++++++++
 5 files changed, 52 insertions(+), 20 deletions(-)
 create mode 100644 test/str_cat_test.cpp

diff --git a/Source/qol/floatingnumbers.cpp b/Source/qol/floatingnumbers.cpp
index ade0ca1fa..2c1c4cbaf 100644
--- a/Source/qol/floatingnumbers.cpp
+++ b/Source/qol/floatingnumbers.cpp
@@ -7,6 +7,7 @@
 
 #include "engine/render/text_render.hpp"
 #include "options.h"
+#include "utils/str_cat.hpp"
 
 namespace devilution {
 
@@ -61,9 +62,10 @@ UiFlags GetFontSizeByDamage(int value)
 
 void UpdateFloatingData(FloatingNumber &num)
 {
-	num.text = fmt::format("{:d}", num.value >> 6);
 	if (num.value > 0 && num.value < 64) {
 		num.text = fmt::format("{:.2f}", num.value / 64.0);
+	} else {
+		num.text = StrCat(num.value >> 6);
 	}
 
 	num.style &= ~(UiFlags::FontSize12 | UiFlags::FontSize24 | UiFlags::FontSize30);
diff --git a/Source/utils/str_cat.cpp b/Source/utils/str_cat.cpp
index f1d112a20..410d09d2c 100644
--- a/Source/utils/str_cat.cpp
+++ b/Source/utils/str_cat.cpp
@@ -1,28 +1,20 @@
 #include "utils/str_cat.hpp"
 
-#include <cstddef>
-
-#include <fmt/compile.h>
-#include <fmt/core.h>
+#include <fmt/format.h>
 
 namespace devilution {
 
-namespace {
-
-template <typename T>
-constexpr size_t MaxDecimalDigits = 241 * sizeof(T) / 100 + 1;
-
-} // namespace
-
 char *BufCopy(char *out, int value)
 {
-	return fmt::format_to(out, FMT_COMPILE("{}"), value);
+	const fmt::format_int formatted { value };
+	std::memcpy(out, formatted.data(), formatted.size());
+	return out + formatted.size();
 }
 
 void StrAppend(std::string &out, int value)
 {
-	char buf[MaxDecimalDigits<int> + 1];
-	out.append(&buf[0], BufCopy(buf, value) - &buf[0]);
+	const fmt::format_int formatted { value };
+	out.append(formatted.data(), formatted.size());
 }
 
 } // namespace devilution
diff --git a/Source/utils/str_cat.hpp b/Source/utils/str_cat.hpp
index a1e91fdb4..b37d2e035 100644
--- a/Source/utils/str_cat.hpp
+++ b/Source/utils/str_cat.hpp
@@ -33,7 +33,7 @@ inline char *BufCopy(char *out, string_view value)
  */
 inline void StrAppend(std::string &out, string_view value)
 {
-	out.append(value.data(), value.size());
+	AppendStrView(out, value);
 }
 
 /**
@@ -51,25 +51,41 @@ inline char *BufCopy(char *out, const char *str)
  */
 inline void StrAppend(std::string &out, const char *str)
 {
-	if (str == nullptr)
-		out.append("(nullptr)");
-	out.append(str, string_view(str).size());
+	AppendStrView(out, string_view(str != nullptr ? str : "(nullptr)"));
 }
 
+#if __cplusplus >= 201703L
+template <typename... Args>
+typename std::enable_if<(sizeof...(Args) > 1), char *>::type
+BufCopy(char *out, Args &&...args)
+{
+	return ((out = BufCopy(out, std::forward<Args>(args))), ...);
+}
+#else
 template <typename Arg, typename... Args>
 inline typename std::enable_if<(sizeof...(Args) > 0), char *>::type
 BufCopy(char *out, Arg &&arg, Args &&...args)
 {
 	return BufCopy(BufCopy(out, std::forward<Arg>(arg)), std::forward<Args>(args)...);
 }
+#endif
 
+#if __cplusplus >= 201703L
+template <typename... Args>
+typename std::enable_if<(sizeof...(Args) > 1), void>::type
+StrAppend(std::string &out, Args &&...args)
+{
+	(StrAppend(out, std::forward<Args>(args)), ...);
+}
+#else
 template <typename Arg, typename... Args>
-inline typename std::enable_if<(sizeof...(Args) > 0), void>::type
+typename std::enable_if<(sizeof...(Args) > 0), void>::type
 StrAppend(std::string &out, Arg &&arg, Args &&...args)
 {
 	StrAppend(out, std::forward<Arg>(arg));
 	StrAppend(out, std::forward<Args>(args)...);
 }
+#endif
 
 template <typename... Args>
 std::string StrCat(Args &&...args)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index c7a62fa40..21eaba023 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -37,6 +37,7 @@ set(tests
   rectangle_test
   scrollrt_test
   stores_test
+  str_cat_test
   timedemo_test
   utf8_test
   writehero_test
diff --git a/test/str_cat_test.cpp b/test/str_cat_test.cpp
new file mode 100644
index 000000000..ee6675c97
--- /dev/null
+++ b/test/str_cat_test.cpp
@@ -0,0 +1,21 @@
+#include <gtest/gtest.h>
+
+#include "utils/str_cat.hpp"
+
+namespace devilution {
+namespace {
+
+TEST(StrCatTest, StrCatBasicTest)
+{
+	EXPECT_EQ(StrCat("a", "b", "c", 5), "abc5");
+}
+
+TEST(StrCatTest, BufCopyBasicTest)
+{
+	char buf[5];
+	char *end = BufCopy(buf, "a", "b", "c", 5);
+	EXPECT_EQ(string_view(buf, end - buf), "abc5");
+}
+
+} // namespace
+} // namespace devilution