From a23be68557b23df3772388dafc934a3bac6fe8b8 Mon Sep 17 00:00:00 2001 From: Robert Burnham Date: Tue, 21 Jul 2026 08:26:14 -0500 Subject: [PATCH] Fix stale `spGetActionHotKeys` returns (#3082) Rebuild the hotkey map on actual binding changes. --- rts/Game/UI/KeyBindings.cpp | 69 +++++++++++++++++++++++++------------ rts/Game/UI/KeyBindings.h | 10 +++++- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/rts/Game/UI/KeyBindings.cpp b/rts/Game/UI/KeyBindings.cpp index bde8e3efb1..cc756cd26d 100644 --- a/rts/Game/UI/KeyBindings.cpp +++ b/rts/Game/UI/KeyBindings.cpp @@ -611,6 +611,7 @@ void CKeyBindings::AddActionToKeyMap(KeyMap& bindings, Action& action) ActionList& al = bindings[ks]; action.bindingIndex = ++bindingsCount; al.push_back(action); + buildHotkeyMap = true; } else { ActionList& al = it->second; assert(it->first == ks); @@ -624,6 +625,7 @@ void CKeyBindings::AddActionToKeyMap(KeyMap& bindings, Action& action) // not yet bound, push it action.bindingIndex = ++bindingsCount; al.push_back(action); + buildHotkeyMap = true; } } } @@ -684,6 +686,9 @@ bool CKeyBindings::UnBind(const std::string& keystr, const std::string& command) if (al.empty()) bindings.erase(it); + if (success) + buildHotkeyMap = true; + return success; } @@ -708,6 +713,7 @@ bool CKeyBindings::UnBindKeyset(const std::string& keystr) return false; bindings.erase(it); + buildHotkeyMap = true; return true; } @@ -741,13 +747,16 @@ bool CKeyBindings::UnBindAction(const std::string& command) RECOIL_DETAILED_TRACY_ZONE; if (debugEnabled) LOG("[CKeyBindings::%s] command=%s", __func__, command.c_str()); - // clear both maps; || would short-circuit and leave the scancode binding when // the action is also bound to a keycode const bool removedFromCode = RemoveActionFromKeyMap(command, codeBindings); const bool removedFromScan = RemoveActionFromKeyMap(command, scanBindings); + const bool changed = removedFromCode || removedFromScan; + + if (changed) + buildHotkeyMap = true; - return removedFromCode || removedFromScan; + return changed; } @@ -818,9 +827,6 @@ void CKeyBindings::ConfigNotify(const std::string& key, const std::string& value void CKeyBindings::LoadDefaults() { RECOIL_DETAILED_TRACY_ZONE; - const bool tmpBuildHotkeyMap = buildHotkeyMap; - buildHotkeyMap = false; - if (debugEnabled) LOG("[CKeyBindings::%s]", __func__); @@ -829,8 +835,7 @@ void CKeyBindings::LoadDefaults() for (const auto& b: defaultBindings) { Bind(b.key, b.action); } - - buildHotkeyMap = tmpBuildHotkeyMap; + // no rebuild here: only ever used as a building block, the caller rebuilds } @@ -865,7 +870,7 @@ void CKeyBindings::PushAction(const Action& action) } } -bool CKeyBindings::ExecuteCommand(const std::string& line) +bool CKeyBindings::ExecuteCommandInternal(const std::string& line) { RECOIL_DETAILED_TRACY_ZONE; const std::vector words = CSimpleParser::Tokenize(line, 2); @@ -894,7 +899,7 @@ bool CKeyBindings::ExecuteCommand(const std::string& line) if (loadStack.empty() && words.size() == 1) LoadDefaults(); - Load(filename); + LoadInternal(filename); } else if (command == "keyreload") { const std::string& filename = words.size() > 1 ? words[1] : DEFAULT_FILENAME; @@ -902,13 +907,13 @@ bool CKeyBindings::ExecuteCommand(const std::string& line) if (debugEnabled) LOG("[CKeyBindings::%s] line=%s", __func__, line.c_str()); - ExecuteCommand("unbindall"); - ExecuteCommand("unbind enter chat"); + ExecuteCommandInternal("unbindall"); + ExecuteCommandInternal("unbind enter chat"); if (loadStack.empty() && words.size() == 1) LoadDefaults(); - Load(filename); + LoadInternal(filename); } else if (command == "keydefaults") { LoadDefaults(); @@ -937,6 +942,7 @@ bool CKeyBindings::ExecuteCommand(const std::string& line) keyCodes.Reset(); scanCodes.Reset(); bindingsCount = 0; + buildHotkeyMap = true; Bind("enter", "chat"); // bare minimum if (debugEnabled) @@ -946,14 +952,19 @@ bool CKeyBindings::ExecuteCommand(const std::string& line) return false; } - if (buildHotkeyMap) - BuildHotkeyMap(); - return false; } -bool CKeyBindings::Load(const std::string& filename) +bool CKeyBindings::ExecuteCommand(const std::string& line) +{ + const bool ret = ExecuteCommandInternal(line); + MaybeBuildHotkeyMap(); + return ret; +} + + +bool CKeyBindings::LoadInternal(const std::string& filename) { RECOIL_DETAILED_TRACY_ZONE; if (std::find(loadStack.begin(), loadStack.end(), filename) != loadStack.end()) { @@ -965,9 +976,6 @@ bool CKeyBindings::Load(const std::string& filename) return false; } - const bool tmpBuildHotkeyMap = buildHotkeyMap; - buildHotkeyMap = false; - if (debugEnabled) { LOG("[CKeyBindings::%s] filename=%s%s", __func__, filename.c_str(), loadStack.empty() ? "" : ", load stack:"); for (auto it = loadStack.rbegin(); it != loadStack.rend(); ++it) @@ -980,17 +988,34 @@ bool CKeyBindings::Load(const std::string& filename) CSimpleParser parser(ifs); while (!parser.Eof()) { - ExecuteCommand(parser.GetCleanLine()); + ExecuteCommandInternal(parser.GetCleanLine()); } loadStack.pop_back(); - buildHotkeyMap = tmpBuildHotkeyMap; - return true; } +bool CKeyBindings::Load(const std::string& filename) +{ + const bool ret = LoadInternal(filename); + MaybeBuildHotkeyMap(); + return ret; +} + + +void CKeyBindings::MaybeBuildHotkeyMap() +{ + RECOIL_DETAILED_TRACY_ZONE; + if (!buildHotkeyMap) + return; + + BuildHotkeyMap(); + buildHotkeyMap = false; +} + + void CKeyBindings::BuildHotkeyMap() { RECOIL_DETAILED_TRACY_ZONE; diff --git a/rts/Game/UI/KeyBindings.h b/rts/Game/UI/KeyBindings.h index 04260a7993..9caf37e610 100644 --- a/rts/Game/UI/KeyBindings.h +++ b/rts/Game/UI/KeyBindings.h @@ -58,8 +58,16 @@ class CKeyBindings : public CommandReceiver protected: void BuildHotkeyMap(); + // rebuilds the reverse map once, and only if a binding actually changed + void MaybeBuildHotkeyMap(); void DebugActionList(const ActionList& actionList) const; + // the *Internal variants do the work but never rebuild the reverse map; + // the public Load/ExecuteCommand wrappers rebuild once at the end, so every + // outside call path rebuilds exactly once regardless of how it recurses + bool LoadInternal(const std::string& filename = DEFAULT_FILENAME); + bool ExecuteCommandInternal(const std::string& line); + void AddActionToKeyMap(KeyMap& bindings, Action& action); static bool RemoveActionFromKeyMap(const std::string& command, KeyMap& bindings); @@ -90,7 +98,7 @@ class CKeyBindings : public CommandReceiver int fakeMetaKey = -1; int keyChainTimeout = 750; - bool buildHotkeyMap = true; + bool buildHotkeyMap = true; // reverse hotkey map is stale and needs rebuilding bool debugEnabled = false; };