From 31244ad82a6d83c9a91f5ffaa6fd6adaa3534cff Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 00:17:45 -0500 Subject: [PATCH 1/8] prefer lambdas over std::bind --- library/Core.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index b927d49cc4..17754e6884 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -330,7 +330,7 @@ static std::string dfhack_version_desc() return s.str(); } -static bool init_run_script(color_ostream &out, lua_State *state, const std::string& pcmd, std::vector& pargs) +static bool init_run_script(color_ostream &out, lua_State *state, const std::string& pcmd, std::span pargs) { if (!lua_checkstack(state, pargs.size()+10)) return false; @@ -338,21 +338,23 @@ static bool init_run_script(color_ostream &out, lua_State *state, const std::str lua_getfield(state, -1, "run_script"); lua_remove(state, -2); lua_pushstring(state, pcmd.c_str()); - for (auto& arg : pargs) + for (const auto& arg : pargs) lua_pushstring(state, arg.c_str()); return true; } -static command_result runLuaScript(color_ostream &out, std::string name, std::vector &args) +static command_result runLuaScript(color_ostream &out, std::string name, std::span args) { - using namespace std::placeholders; - auto init_fn = std::bind(init_run_script, _1, _2, name, args); + auto init_fn = [n = std::move(name), args](color_ostream& out, lua_State* state) -> bool { + return init_run_script(out, state, n, args); + }; + bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(true), init_fn); return ok ? CR_OK : CR_FAILURE; } -static bool init_enable_script(color_ostream &out, lua_State *state, std::string& name, bool enable) +static bool init_enable_script(color_ostream &out, lua_State *state, const std::string& name, bool enable) { if (!lua_checkstack(state, 4)) return false; @@ -364,10 +366,12 @@ static bool init_enable_script(color_ostream &out, lua_State *state, std::string return true; } -static command_result enableLuaScript(color_ostream &out, std::string name, bool state) +static command_result enableLuaScript(color_ostream &out, std::string name, bool enabled) { - using namespace std::placeholders; - auto init_fn = std::bind(init_enable_script, _1, _2, name, state); + auto init_fn = [n = std::move(name), enabled](color_ostream& out, lua_State* state) -> bool { + return init_enable_script(out, state, n, enabled); + }; + bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(), init_fn); return ok ? CR_OK : CR_FAILURE; From 31e3caecabd83c1f9b081aef461a7f5207935bff Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 02:02:55 -0500 Subject: [PATCH 2/8] attempt to use more modern c++20 --- library/Core.cpp | 184 +++++++++++++++++++++-------------------- library/include/Core.h | 2 +- 2 files changed, 95 insertions(+), 91 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 17754e6884..0fa91ccee4 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -74,6 +74,7 @@ distribution. #include #include #include +#include #include #include #include @@ -806,11 +807,11 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s bool reload = (first == "reload"); if (parts.size()) { - for (auto p = parts.begin(); p != parts.end(); p++) + for (const auto& p : parts) { - if (p->size() && (*p)[0] == '-') + if (p.empty() && p[0] == '-') { - if (p->find('a') != std::string::npos) + if (p.find('a') != std::string::npos) all = true; } } @@ -826,15 +827,15 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s } else { - for (auto p = parts.begin(); p != parts.end(); p++) + for (auto& p : parts) { - if (!p->size() || (*p)[0] == '-') + if (p.empty() || p[0] == '-') continue; - if (load && !plug_mgr->load(*p)) + if (load && !plug_mgr->load(p)) ret = CR_FAILURE; - else if (unload && !plug_mgr->unload(*p)) + else if (unload && !plug_mgr->unload(p)) ret = CR_FAILURE; - else if (reload && !plug_mgr->reload(*p)) + else if (reload && !plug_mgr->reload(p)) ret = CR_FAILURE; } } @@ -854,16 +855,15 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s if(parts.size()) { - for (size_t i = 0; i < parts.size(); i++) + for (auto& part : parts) { - std::string part = parts[i]; if (part.find('\\') != std::string::npos) { con.printerr("Replacing backslashes with forward slashes in \"%s\"\n", part.c_str()); - for (size_t j = 0; j < part.size(); j++) + for (char& c : part) { - if (part[j] == '\\') - part[j] = '/'; + if (c == '\\') + c = '/'; } } @@ -902,9 +902,10 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s } else { - for (auto it = plug_mgr->begin(); it != plug_mgr->end(); ++it) + for (auto& [key, plug] : *plug_mgr) { - Plugin * plug = it->second; + if (!plug) + continue; if (!plug->can_be_enabled()) continue; con.print( @@ -929,13 +930,14 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s con.print(header_format, "Name", "State", "Cmds", "Enabled"); plug_mgr->refresh(); - for (auto it = plug_mgr->begin(); it != plug_mgr->end(); ++it) + for (auto& [key, plug] : *plug_mgr) { - Plugin * plug = it->second; if (!plug) continue; - if (parts.size() && std::find(parts.begin(), parts.end(), plug->getName()) == parts.end()) + + if (std::ranges::find(parts, plug->getName()) == parts.end()) continue; + color_value color; switch (plug->getState()) { @@ -1012,9 +1014,10 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s std::string keystr = parts[1]; if (parts[0] == "set") ClearKeyBindings(keystr); - for (int i = parts.size()-1; i >= 2; i--) + // for (int i = parts.size()-1; i >= 2; i--) + for (const auto& part : parts | std::views::drop(2) | std::views::reverse) { - if (!AddKeyBinding(keystr, parts[i])) { + if (!AddKeyBinding(keystr, part)) { con.printerr("Invalid key spec: %s\n", keystr.c_str()); break; } @@ -1022,10 +1025,11 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s } else if (parts.size() >= 2 && parts[0] == "clear") { - for (size_t i = 1; i < parts.size(); i++) + // for (size_t i = 1; i < parts.size(); i++) + for (const auto& part : parts | std::views::drop(1)) { - if (!ClearKeyBindings(parts[i])) { - con.printerr("Invalid key spec: %s\n", parts[i].c_str()); + if (!ClearKeyBindings(part)) { + con.printerr("Invalid key spec: %s\n", part.c_str()); break; } } @@ -1035,8 +1039,8 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s std::vector list = ListKeyBindings(parts[1]); if (list.empty()) con << "No bindings." << std::endl; - for (size_t i = 0; i < list.size(); i++) - con << " " << list[i] << std::endl; + for (const auto& kb : list) + con << " " << kb << std::endl; } else { @@ -1121,9 +1125,9 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s else if (first == "kill-lua") { bool force = false; - for (auto it = parts.begin(); it != parts.end(); ++it) + for (const auto& part : parts) { - if (*it == "force") + if (part == "force") force = true; } if (!Lua::Interrupt(force)) @@ -1189,14 +1193,14 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s con << "Unrecognized event name: " << parts[1] << std::endl; return CR_WRONG_USAGE; } - for (auto it = state_change_scripts.begin(); it != state_change_scripts.end(); ++it) + for (const auto& state_script : state_change_scripts) { - if (!parts[1].size() || (it->event == sc_event_id(parts[1]))) + if (!parts[1].size() || (state_script.event == sc_event_id(parts[1]))) { - con.print("%s (%s): %s%s\n", sc_event_name(it->event).c_str(), - it->save_specific ? "save-specific" : "global", - it->save_specific ? "/raw/" : "/", - it->path.c_str()); + con.print("%s (%s): %s%s\n", sc_event_name(state_script.event).c_str(), + state_script.save_specific ? "save-specific" : "global", + state_script.save_specific ? "/raw/" : "/", + state_script.path.c_str()); } } return CR_OK; @@ -1216,9 +1220,9 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s } bool save_specific = (parts.size() >= 4 && parts[3] == "-save"); StateChangeScript script(evt, parts[2], save_specific); - for (auto it = state_change_scripts.begin(); it != state_change_scripts.end(); ++it) + for (const auto& state_script : state_change_scripts) { - if (script == *it) + if (script == state_script) { con << "Script already registered" << std::endl; return CR_FAILURE; @@ -1448,7 +1452,7 @@ static void fIOthread(IODATA * iod) while (true) { - std::string command = ""; + std::string command; int ret; while ((ret = con.lineedit("[DFHack]# ",command, main_history)) == Console::RETRY); @@ -1812,7 +1816,7 @@ bool Core::InitSimulationThread() if (entry.second) continue; std::filesystem::path filename = entry.first; - if (!config_files.count(filename)) { + if (!config_files.contains(filename)) { std::filesystem::path src_file = getConfigDefaultsPath() / filename; if (!Filesystem::isfile(src_file)) continue; @@ -1855,7 +1859,7 @@ bool Core::InitSimulationThread() plug_mgr->init(); std::cerr << "Starting the TCP listener.\n"; auto listen = ServerMain::listen(RemoteClient::GetDefaultPort()); - IODATA *temp = new IODATA; + auto *temp = new IODATA; temp->core = this; temp->plug_mgr = plug_mgr; @@ -1890,13 +1894,13 @@ bool Core::InitSimulationThread() if (raw[offset] == '"') { offset++; - size_t next = raw.find("\"", offset); + size_t next = raw.find('"', offset); args.push_back(raw.substr(offset, next - offset)); offset = next + 2; } else { - size_t next = raw.find(" ", offset); + size_t next = raw.find(' ', offset); if (next == std::string::npos) { args.push_back(raw.substr(offset)); @@ -1915,9 +1919,9 @@ bool Core::InitSimulationThread() if (first.length() > 0 && first[0] == '+') { std::vector cmd; - for (it++; it != args.end(); it++) { - const std::string & arg = *it; - if (arg.length() > 0 && arg[0] == '+') + for (const auto& arg : args) + { + if (!arg.empty() && arg[0] == '+') { break; } @@ -1927,9 +1931,9 @@ bool Core::InitSimulationThread() if (runCommand(con, first.substr(1), cmd) != CR_OK) { std::cerr << "Error running command: " << first.substr(1); - for (auto it2 = cmd.begin(); it2 != cmd.end(); it2++) + for (const auto& arg : cmd) { - std::cerr << " \"" << *it2 << "\""; + std::cerr << " \"" << arg << "\""; } std::cerr << "\n"; } @@ -1953,7 +1957,7 @@ bool Core::setHotkeyCmd( std::string cmd ) // access command std::lock_guard lock(HotkeyMutex); hotkey_set = SET; - hotkey_cmd = cmd; + hotkey_cmd = std::move(cmd); HotkeyCond.notify_all(); return true; } @@ -2170,43 +2174,44 @@ void Core::onUpdate(color_ostream &out) perf_counters.incCounter(perf_counters.update_lua_ms, step_start_ms); } -void getFilesWithPrefixAndSuffix(const std::filesystem::path& folder, const std::string& prefix, const std::string& suffix, std::vector& result) { +static void getFilesWithPrefixAndSuffix(const std::filesystem::path& folder, const std::string& prefix, const std::string& suffix, std::vector& result) { std::vector files; DFHack::Filesystem::listdir(folder, files); - for ( auto f : files) { + for ( const auto& f : files) { if (f.stem().string().starts_with(prefix) && f.extension() == suffix) result.push_back(f); } - return; } size_t loadScriptFiles(Core* core, color_ostream& out, const std::vector& prefix, const std::filesystem::path& folder) { static const std::string suffix = ".init"; std::vector scriptFiles; - for ( size_t a = 0; a < prefix.size(); a++ ) { - getFilesWithPrefixAndSuffix(folder, prefix[a], ".init", scriptFiles); + for ( const auto& p : prefix ) { + getFilesWithPrefixAndSuffix(folder, p, ".init", scriptFiles); } - std::sort(scriptFiles.begin(), scriptFiles.end(), - [](const std::filesystem::path& a, const std::filesystem::path& b) { - return a.stem() < b.stem(); - }); + + std::ranges::sort(scriptFiles, + [](const std::filesystem::path& a, const std::filesystem::path& b) { + return a.stem() < b.stem(); + }); + size_t result = 0; - for ( size_t a = 0; a < scriptFiles.size(); a++ ) { + for ( const auto& file : scriptFiles) { result++; - core->loadScriptFile(out, folder / scriptFiles[a], false); + core->loadScriptFile(out, folder / file, false); } return result; } namespace DFHack { namespace X { - typedef state_change_event Key; - typedef std::vector Val; - typedef std::pair Entry; - typedef std::vector EntryVector; - typedef std::map InitVariationTable; + using Key = state_change_event; + using Val = std::vector; + using Entry = std::pair; + using EntryVector = std::vector; + using InitVariationTable = std::map; - EntryVector computeInitVariationTable(void* none, ...) { + static EntryVector computeInitVariationTable(void* none, ...) { va_list list; va_start(list,none); EntryVector result; @@ -2227,7 +2232,7 @@ namespace DFHack { return result; } - InitVariationTable getTable(const EntryVector& vec) { + static InitVariationTable getTable(const EntryVector& vec) { return InitVariationTable(vec.begin(),vec.end()); } } @@ -2258,17 +2263,17 @@ void Core::handleLoadAndUnloadScripts(color_ostream& out, state_change_event eve loadScriptFiles(this, out, set, rawFolder); } - for (auto it = state_change_scripts.begin(); it != state_change_scripts.end(); ++it) + for (const auto& script : state_change_scripts) { - if (it->event == event) + if (script.event == event) { - if (!it->save_specific) + if (!script.save_specific) { - loadScriptFile(out, it->path, false); + loadScriptFile(out, script.path, false); } - else if (it->save_specific && isWorldLoaded()) + else if (script.save_specific && isWorldLoaded()) { - loadScriptFile(out, rawFolder / it->path, false); + loadScriptFile(out, rawFolder / script.path, false); } } } @@ -2278,7 +2283,7 @@ void Core::onStateChange(color_ostream &out, state_change_event event) { using df::global::gametype; static md5wrapper md5w; - static std::string ostype = ""; + static std::string ostype; if (!ostype.size()) { @@ -2480,7 +2485,7 @@ bool Core::DFH_ncurses_key(int key) return ncurses_wgetch(key, dummy); } -bool Core::getSuppressDuplicateKeyboardEvents() { +bool Core::getSuppressDuplicateKeyboardEvents() const { return suppress_duplicate_keyboard_events; } @@ -2605,8 +2610,8 @@ bool Core::SelectHotkey(int sym, int modifiers) // Check the internal keybindings std::vector &bindings = key_bindings[sym]; - for (int i = bindings.size()-1; i >= 0; --i) { - auto &binding = bindings[i]; + //for (int i = bindings.size()-1; i >= 0; --i) { + for (const auto& binding : bindings | std::views::reverse) { DEBUG(keybinding).print("examining hotkey with commandline: '%s'\n", binding.cmdline.c_str()); if (binding.modifiers != modifiers) { @@ -2662,8 +2667,8 @@ bool Core::SelectHotkey(int sym, int modifiers) setHotkeyCmd(cmd); return true; } - else - return false; + + return false; } static bool parseKeySpec(std::string keyspec, int *psym, int *pmod, std::string *pfocus) @@ -2684,13 +2689,13 @@ static bool parseKeySpec(std::string keyspec, int *psym, int *pmod, std::string // ugh, ugly for (;;) { - if (keyspec.size() > 6 && keyspec.substr(0, 6) == "Shift-") { + if (keyspec.size() > 6 && keyspec.starts_with("Shift-")) { *pmod |= 1; keyspec = keyspec.substr(6); - } else if (keyspec.size() > 5 && keyspec.substr(0, 5) == "Ctrl-") { + } else if (keyspec.size() > 5 && keyspec.starts_with("Ctrl-")) { *pmod |= 2; keyspec = keyspec.substr(5); - } else if (keyspec.size() > 4 && keyspec.substr(0, 4) == "Alt-") { + } else if (keyspec.size() > 4 && keyspec.starts_with("Alt-")) { *pmod |= 4; keyspec = keyspec.substr(4); } else @@ -2709,13 +2714,13 @@ static bool parseKeySpec(std::string keyspec, int *psym, int *pmod, std::string } else if (keyspec.size() == 2 && keyspec[0] == 'F' && keyspec[1] >= '1' && keyspec[1] <= '9') { *psym = SDLK_F1 + (keyspec[1]-'1'); return true; - } else if (keyspec.size() == 3 && keyspec.substr(0, 2) == "F1" && keyspec[2] >= '0' && keyspec[2] <= '2') { + } else if (keyspec.size() == 3 && keyspec.starts_with("F1") && keyspec[2] >= '0' && keyspec[2] <= '2') { *psym = SDLK_F10 + (keyspec[2]-'0'); return true; - } else if (keyspec.size() == 6 && keyspec.substr(0, 5) == "MOUSE" && keyspec[5] >= '4' && keyspec[5] <= '9') { + } else if (keyspec.size() == 6 && keyspec.starts_with("MOUSE") && keyspec[5] >= '4' && keyspec[5] <= '9') { *psym = SDLK_F13 + (keyspec[5]-'4'); return true; - } else if (keyspec.size() == 7 && keyspec.substr(0, 6) == "MOUSE1" && keyspec[5] >= '0' && keyspec[5] <= '5') { + } else if (keyspec.size() == 7 && keyspec.starts_with("MOUSE1") && keyspec[5] >= '0' && keyspec[5] <= '5') { *psym = SDLK_F19 + (keyspec[5]-'0'); return true; } else if (keyspec == "Enter") { @@ -2754,9 +2759,9 @@ bool Core::AddKeyBinding(std::string keyspec, std::string cmdline) { std::vector focus_strings; split_string(&focus_strings, raw_focus, "|"); - for (size_t i = 0; i < focus_strings.size(); i++) + for (const auto& fs : focus_strings) { - if (!AddKeyBinding(raw_spec + "@" + focus_strings[i], cmdline)) + if (!AddKeyBinding(raw_spec + "@" + fs, cmdline)) return false; } return true; @@ -2838,7 +2843,7 @@ bool Core::RemoveAlias(const std::string &name) bool Core::IsAlias(const std::string &name) { std::lock_guard lock(alias_mutex); - return aliases.find(name) != aliases.end(); + return aliases.contains(name); } bool Core::RunAlias(color_ostream &out, const std::string &name, @@ -2903,10 +2908,9 @@ bool ClassNameCheck::operator() (Process *p, void * ptr) const { void ClassNameCheck::getKnownClassNames(std::vector &names) { - std::set::iterator it = known_class_names.begin(); - - for (; it != known_class_names.end(); it++) - names.push_back(*it); + for(const auto& kcn : known_class_names) { + names.push_back(kcn); + } } MemoryPatcher::MemoryPatcher(Process *p_) : p(p_) @@ -2922,7 +2926,7 @@ MemoryPatcher::~MemoryPatcher() bool MemoryPatcher::verifyAccess(void *target, size_t count, bool write) { - uint8_t *sptr = (uint8_t*)target; + auto *sptr = (uint8_t*)target; uint8_t *eptr = sptr + count; // Find the valid memory ranges diff --git a/library/include/Core.h b/library/include/Core.h index 383a0a9c95..50cfd8909b 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -180,7 +180,7 @@ namespace DFHack std::filesystem::path findScript(std::string name); void getScriptPaths(std::vector *dest); - bool getSuppressDuplicateKeyboardEvents(); + bool getSuppressDuplicateKeyboardEvents() const; void setSuppressDuplicateKeyboardEvents(bool suppress); void setMortalMode(bool value); void setArmokTools(const std::vector &tool_names); From a5717a66d6d0a5bf18079ce129511f1a37e6c363 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 02:21:26 -0500 Subject: [PATCH 3/8] add missing include --- library/Core.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/library/Core.cpp b/library/Core.cpp index 0fa91ccee4..1fee8000e6 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -75,6 +75,7 @@ distribution. #include #include #include +#include #include #include #include From f9fae1391af320e690cdce0ab1edcbac577ddde5 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 02:43:41 -0500 Subject: [PATCH 4/8] fix typo --- library/Core.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 1fee8000e6..ef271b980d 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -80,7 +80,6 @@ distribution. #include #include #include -#include #include #include #include @@ -810,7 +809,10 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s { for (const auto& p : parts) { - if (p.empty() && p[0] == '-') + if (p.empty()) + continue; + + if (p[0] == '-') { if (p.find('a') != std::string::npos) all = true; From c454de10d85e36d7e377e010d3bc274ab17225b1 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 19:12:25 -0500 Subject: [PATCH 5/8] use size() as before --- library/Core.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 3371746995..7bd11562e3 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -794,10 +794,7 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s { for (const auto& p : parts) { - if (p.empty()) - continue; - - if (p[0] == '-') + if (p.size() && p[0] == '-') { if (p.find('a') != std::string::npos) all = true; From 16d7e985a3d60c61b7f1bc8d90a242e4786b8686 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 20:59:57 -0500 Subject: [PATCH 6/8] typo: missing parts.size() test for 'plug' without arguments. use std::ranges::end() iterator. --- library/Core.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 7bd11562e3..14d82a30aa 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -895,7 +895,7 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s con.print( "%21s %-3s%s\n", - (plug->getName()+":").c_str(), + (key+":").c_str(), plug->is_enabled() ? "on" : "off", plug->can_set_enabled() ? "" : " (controlled internally)" ); @@ -920,7 +920,7 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s if (!plug) continue; - if (std::ranges::find(parts, plug->getName()) == parts.end()) + if (parts.size() && std::ranges::find(parts, key) == std::ranges::end(parts)) continue; color_value color; From a52f36b895c89a3d2221ecf1ad54b4147ca4cc38 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Tue, 11 Nov 2025 22:49:30 -0500 Subject: [PATCH 7/8] mostly add utility functions for backslash handling to MiscUtils.h --- library/Core.cpp | 32 ++++++++++++-------------------- library/include/MiscUtils.h | 13 +++++++++++++ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index 14d82a30aa..b49784db4b 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -105,7 +105,7 @@ using std::string; // FIXME: A lot of code in one file, all doing different things... there's something fishy about it. static bool parseKeySpec(std::string keyspec, int *psym, int *pmod, std::string *pfocus = nullptr); -size_t loadScriptFiles(Core* core, color_ostream& out, const std::vector& prefix, const std::filesystem::path& folder); +static size_t loadScriptFiles(Core* core, color_ostream& out, std::span prefix, const std::filesystem::path& folder); namespace DFHack { @@ -316,7 +316,7 @@ static std::string dfhack_version_desc() return s.str(); } -static bool init_run_script(color_ostream &out, lua_State *state, const std::string& pcmd, std::span pargs) +static bool init_run_script(color_ostream &out, lua_State *state, const std::string& pcmd, const std::span pargs) { if (!lua_checkstack(state, pargs.size()+10)) return false; @@ -329,7 +329,7 @@ static bool init_run_script(color_ostream &out, lua_State *state, const std::str return true; } -static command_result runLuaScript(color_ostream &out, std::string name, std::span args) +static command_result runLuaScript(color_ostream &out, std::string name, const std::span args) { auto init_fn = [n = std::move(name), args](color_ostream& out, lua_State* state) -> bool { return init_run_script(out, state, n, args); @@ -673,14 +673,14 @@ void tags_helper(color_ostream &con, const std::string &tag) { } } -void ls_helper(color_ostream &con, const std::vector ¶ms) { +static void ls_helper(color_ostream &con, const std::span params) { std::vector filter; bool skip_tags = false; bool show_dev_commands = false; - std::string exclude_strs = ""; + std::string exclude_strs; bool in_exclude = false; - for (auto str : params) { + for (const auto& str : params) { if (in_exclude) exclude_strs = str; else if (str == "--notags") @@ -733,14 +733,10 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s if (first.empty()) return CR_NOT_IMPLEMENTED; - if (first.find('\\') != std::string::npos) + if (has_backslashes(first)) { con.printerr("Replacing backslashes with forward slashes in \"%s\"\n", first.c_str()); - for (size_t i = 0; i < first.size(); i++) - { - if (first[i] == '\\') - first[i] = '/'; - } + replace_backslashes_with_forwardslashes(first); } // let's see what we actually got @@ -842,14 +838,10 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s { for (auto& part : parts) { - if (part.find('\\') != std::string::npos) + if (has_backslashes(part)) { con.printerr("Replacing backslashes with forward slashes in \"%s\"\n", part.c_str()); - for (char& c : part) - { - if (c == '\\') - c = '/'; - } + replace_backslashes_with_forwardslashes(part); } part = GetAliasCommand(part, true); @@ -920,7 +912,7 @@ command_result Core::runCommand(color_ostream &con, const std::string &first_, s if (!plug) continue; - if (parts.size() && std::ranges::find(parts, key) == std::ranges::end(parts)) + if (parts.size() && std::ranges::find(parts, key) == parts.end()) continue; color_value color; @@ -2168,7 +2160,7 @@ static void getFilesWithPrefixAndSuffix(const std::filesystem::path& folder, con } } -size_t loadScriptFiles(Core* core, color_ostream& out, const std::vector& prefix, const std::filesystem::path& folder) { +size_t loadScriptFiles(Core* core, color_ostream& out, const std::span prefix, const std::filesystem::path& folder) { static const std::string suffix = ".init"; std::vector scriptFiles; for ( const auto& p : prefix ) { diff --git a/library/include/MiscUtils.h b/library/include/MiscUtils.h index 2ff04282e4..1d703b2f83 100644 --- a/library/include/MiscUtils.h +++ b/library/include/MiscUtils.h @@ -475,6 +475,19 @@ static inline std::string &trim(std::string &s) { return ltrim(rtrim(s)); } +static inline bool has_backslashes(const std::string_view str) +{ + return (str.find('\\') != std::string::npos); +} + +static inline void replace_backslashes_with_forwardslashes(std::string& str) +{ + for (auto& c : str) { + if (c == '\\') + c = '/'; + } +} + enum struct NumberFormatType : int32_t { DEFAULT = 0, ENGLISH, From 1edf08d4ec6fa2b275f14b4ea24d104d4f99b136 Mon Sep 17 00:00:00 2001 From: dhthwy <302825+dhthwy@users.noreply.github.com> Date: Sun, 16 Nov 2025 16:00:22 -0500 Subject: [PATCH 8/8] use std::string_view. eliminate C-varargs when building state_change_event table. --- library/Core.cpp | 71 ++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index b49784db4b..685703eb33 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -316,23 +316,23 @@ static std::string dfhack_version_desc() return s.str(); } -static bool init_run_script(color_ostream &out, lua_State *state, const std::string& pcmd, const std::span pargs) +static bool init_run_script(color_ostream &out, lua_State *state, const std::string_view pcmd, const std::span pargs) { if (!lua_checkstack(state, pargs.size()+10)) return false; Lua::PushDFHack(state); lua_getfield(state, -1, "run_script"); lua_remove(state, -2); - lua_pushstring(state, pcmd.c_str()); + lua_pushlstring(state, pcmd.data(), pcmd.size()); for (const auto& arg : pargs) lua_pushstring(state, arg.c_str()); return true; } -static command_result runLuaScript(color_ostream &out, std::string name, const std::span args) +static command_result runLuaScript(color_ostream &out, const std::string_view name, const std::span args) { - auto init_fn = [n = std::move(name), args](color_ostream& out, lua_State* state) -> bool { - return init_run_script(out, state, n, args); + auto init_fn = [name, args](color_ostream& out, lua_State* state) -> bool { + return init_run_script(out, state, name, args); }; bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(true), init_fn); @@ -340,22 +340,22 @@ static command_result runLuaScript(color_ostream &out, std::string name, const s return ok ? CR_OK : CR_FAILURE; } -static bool init_enable_script(color_ostream &out, lua_State *state, const std::string& name, bool enable) +static bool init_enable_script(color_ostream &out, lua_State *state, const std::string_view name, bool enable) { if (!lua_checkstack(state, 4)) return false; Lua::PushDFHack(state); lua_getfield(state, -1, "enable_script"); lua_remove(state, -2); - lua_pushstring(state, name.c_str()); + lua_pushlstring(state, name.data(), name.size()); lua_pushboolean(state, enable); return true; } -static command_result enableLuaScript(color_ostream &out, std::string name, bool enabled) +static command_result enableLuaScript(color_ostream &out, const std::string_view name, bool enabled) { - auto init_fn = [n = std::move(name), enabled](color_ostream& out, lua_State* state) -> bool { - return init_enable_script(out, state, n, enabled); + auto init_fn = [name, enabled](color_ostream& out, lua_State* state) -> bool { + return init_enable_script(out, state, name, enabled); }; bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(), init_fn); @@ -677,7 +677,7 @@ static void ls_helper(color_ostream &con, const std::span par std::vector filter; bool skip_tags = false; bool show_dev_commands = false; - std::string exclude_strs; + std::string_view exclude_strs; bool in_exclude = false; for (const auto& str : params) { @@ -2188,24 +2188,31 @@ namespace DFHack { using EntryVector = std::vector; using InitVariationTable = std::map; - static EntryVector computeInitVariationTable(void* none, ...) { - va_list list; - va_start(list,none); + template + concept VariationTableTypes = std::same_as || std::convertible_to; + + template + static EntryVector computeInitVariationTable(Ts&&... ts) { + using Arg = std::variant; + std::vector args{ Arg{std::forward(ts)}... }; + + Key current_key = SC_UNKNOWN; EntryVector result; - while(true) { - Key key = (Key)va_arg(list,int); - if ( key == SC_UNKNOWN ) - break; - Val val; - while (true) { - const char *v = va_arg(list, const char *); - if (!v || !v[0]) - break; - val.emplace_back(v); + Val val; + for (auto& arg : args) { + if (std::holds_alternative(arg)) { + if (current_key != SC_UNKNOWN && !val.empty()) { + result.emplace_back(current_key, val); + val.clear(); + } + current_key = std::get(arg); + } else if (std::holds_alternative(arg)) { + auto str = std::get(arg); + if (!str.empty()) + val.emplace_back(str); } - result.push_back(Entry(key,val)); } - va_end(list); + return result; } @@ -2216,12 +2223,12 @@ namespace DFHack { } void Core::handleLoadAndUnloadScripts(color_ostream& out, state_change_event event) { - static const X::InitVariationTable table = X::getTable(X::computeInitVariationTable(nullptr, - (int)SC_WORLD_LOADED, "onLoad", "onLoadWorld", "onWorldLoaded", "", - (int)SC_WORLD_UNLOADED, "onUnload", "onUnloadWorld", "onWorldUnloaded", "", - (int)SC_MAP_LOADED, "onMapLoad", "onLoadMap", "", - (int)SC_MAP_UNLOADED, "onMapUnload", "onUnloadMap", "", - (int)SC_UNKNOWN + static const X::InitVariationTable table = X::getTable(X::computeInitVariationTable( + SC_WORLD_LOADED, "onLoad", "onLoadWorld", "onWorldLoaded", + SC_WORLD_UNLOADED, "onUnload", "onUnloadWorld", "onWorldUnloaded", + SC_MAP_LOADED, "onMapLoad", "onLoadMap", + SC_MAP_UNLOADED, "onMapUnload", "onUnloadMap", + SC_UNKNOWN )); if (!df::global::world)