Aleph-One-Marathon / alephone

Aleph One is the open source continuation of Bungie’s Marathon 2 game engine.
https://alephone.lhowon.org/
GNU General Public License v3.0
654 stars 99 forks source link

replace libboost_filesystem dependency with std::filesystem #447

Closed darealshinji closed 1 year ago

darealshinji commented 1 year ago

Is it possible to replace the build dependency libboost_filesystem with C++'s std::filesystem?

LidMop commented 1 year ago

libc++ on MacOS before 10.15 doesn't support std::filesystem. Aside from that, FileHandler.cpp would need changes to handle the nonexistence of std::unique_path(), differences in the return type of last_write_time(), and possibly other little things.

darealshinji commented 1 year ago

I see. Another thing I would like to mention is that at least on Linux libboost_system.so is not needed and will always be removed by the linker using -Wl,--as-needed. I can actually remove the whole AX_BOOST_SYSTEM part from configure.ac. Even being pedantic and linking with -Wl,-z,defs works without issues. Is libboost_system still needed on other platforms?

darealshinji commented 1 year ago

boost::replace_all could be swapped with a custom inline function:

--- a/Source_Files/CSeries/csstrings.cpp
+++ b/Source_Files/CSeries/csstrings.cpp
@@ -46,7 +46,6 @@
 #include "Scenario.h"

 #include <map>
-#include <boost/algorithm/string/replace.hpp>

 #ifdef __WIN32__
 #define WIN32_LEAN_AND_MEAN
@@ -378,15 +377,20 @@ std::string wide_to_utf8(const std::wstring& utf16) { return wide_to_utf8(utf16.
  */
 void expand_app_variables_inplace(std::string& str)
 {
-   boost::replace_all(str, "$appName$", get_application_name());
-   boost::replace_all(str, "$appVersion$", A1_DISPLAY_VERSION);
-   boost::replace_all(str, "$appLongVersion$", A1_VERSION_STRING);
-   boost::replace_all(str, "$appDate$", A1_DISPLAY_DATE_VERSION);
-   boost::replace_all(str, "$appPlatform$", A1_DISPLAY_PLATFORM);
-   boost::replace_all(str, "$appURL$", A1_HOMEPAGE_URL);
-   boost::replace_all(str, "$appLogFile$", loggingFileName());
-   boost::replace_all(str, "$scenarioName$", Scenario::instance()->GetName());
-   boost::replace_all(str, "$scenarioVersion$", Scenario::instance()->GetVersion());
+   auto replace_all = [] (std::string& s, const std::string& from, const std::string& to) {
+       for (size_t pos = 0; (pos = s.find(from, pos)) != std::string::npos; pos += to.size())
+           s.replace(pos, from.size(), to);
+   };
+
+   replace_all(str, "$appName$", get_application_name());
+   replace_all(str, "$appVersion$", A1_DISPLAY_VERSION);
+   replace_all(str, "$appLongVersion$", A1_VERSION_STRING);
+   replace_all(str, "$appDate$", A1_DISPLAY_DATE_VERSION);
+   replace_all(str, "$appPlatform$", A1_DISPLAY_PLATFORM);
+   replace_all(str, "$appURL$", A1_HOMEPAGE_URL);
+   replace_all(str, "$appLogFile$", loggingFileName());
+   replace_all(str, "$scenarioName$", Scenario::instance()->GetName());
+   replace_all(str, "$scenarioVersion$", Scenario::instance()->GetVersion());
 }

 std::string expand_app_variables(const std::string& input)

There's other stuff too like boost::algorithm::ends_with() could be swapped with std::string::ends_with() but that requires C++20 and there's no case-insensitive version (or you write your own function).

LidMop commented 1 year ago

I see. Another thing I would like to mention is that at least on Linux libboost_system.so is not needed and will always be removed by the linker using -Wl,--as-needed. I can actually remove the whole AX_BOOST_SYSTEM part from configure.ac. Even being pedantic and linking with -Wl,-z,defs works without issues. Is libboost_system still needed on other platforms?

Boost.System became header-only (with a stub library you can still link to) in Boost 1.69 (Dec 2018), but we still support versions down to 1.65.

treellama commented 1 year ago

We can certainly replace boost things with std once they move over, although we are only at C++17. I don't see the reason to write inlines to replace stuff in boost, though, since we're already using it.