Fix #14190: Game crash likely related to plug-in hotkeys

Do not cache references to RegisteredShortcut as they can be invalidated when new shortcuts are registered / removed.
Use a map to improve query performance of shortcut by ID.
Store a separate list of strings for the map to use as a key.

src/openrct2-ui/input/InputManager.cpp

@@ -291,21 +291,17 @@ void InputManager::ProcessHoldEvents()
     auto& shortcutManager = GetShortcutManager();
     if (!shortcutManager.IsPendingShortcutChange())
     {
-        ProcessViewScrollEvent(ShortcutId::ViewScrollUp, _scrollUpShortcut, { 0, -1 });
-        ProcessViewScrollEvent(ShortcutId::ViewScrollDown, _scrollDownShortcut, { 0, 1 });
-        ProcessViewScrollEvent(ShortcutId::ViewScrollLeft, _scrollLeftShortcut, { -1, 0 });
-        ProcessViewScrollEvent(ShortcutId::ViewScrollRight, _scrollRightShortcut, { 1, 0 });
+        ProcessViewScrollEvent(ShortcutId::ViewScrollUp, { 0, -1 });
+        ProcessViewScrollEvent(ShortcutId::ViewScrollDown, { 0, 1 });
+        ProcessViewScrollEvent(ShortcutId::ViewScrollLeft, { -1, 0 });
+        ProcessViewScrollEvent(ShortcutId::ViewScrollRight, { 1, 0 });
     }
 }
 
-void InputManager::ProcessViewScrollEvent(
-    std::string_view shortcutId, RegisteredShortcut*& shortcut, const ScreenCoordsXY& delta)
+void InputManager::ProcessViewScrollEvent(std::string_view shortcutId, const ScreenCoordsXY& delta)
 {
-    if (shortcut == nullptr)
-    {
-        auto& shortcutManager = GetShortcutManager();
-        shortcut = shortcutManager.GetShortcut(shortcutId);
-    }
+    auto& shortcutManager = GetShortcutManager();
+    auto shortcut = shortcutManager.GetShortcut(shortcutId);
     if (shortcut != nullptr && GetState(*shortcut))
     {
         _viewScroll.x += delta.x;

src/openrct2-ui/input/InputManager.h

@@ -53,11 +53,6 @@ namespace OpenRCT2::Ui
         uint32_t _mouseState;
         std::vector<uint8_t> _keyboardState;
 
-        RegisteredShortcut* _scrollLeftShortcut{};
-        RegisteredShortcut* _scrollUpShortcut{};
-        RegisteredShortcut* _scrollRightShortcut{};
-        RegisteredShortcut* _scrollDownShortcut{};
-
         void CheckJoysticks();
 
         void HandleViewScrolling();
@@ -67,7 +62,7 @@ namespace OpenRCT2::Ui
         void ProcessInGameConsole(const InputEvent& e);
         void ProcessChat(const InputEvent& e);
         void ProcessHoldEvents();
-        void ProcessViewScrollEvent(std::string_view shortcutId, RegisteredShortcut*& shortcut, const ScreenCoordsXY& delta);
+        void ProcessViewScrollEvent(std::string_view shortcutId, const ScreenCoordsXY& delta);
 
         bool GetState(const RegisteredShortcut& shortcut) const;
         bool GetState(const ShortcutInput& shortcut) const;

src/openrct2-ui/input/ShortcutManager.cpp

@@ -119,22 +119,24 @@ void ShortcutManager::RegisterShortcut(RegisteredShortcut&& shortcut)
 {
     if (!shortcut.Id.empty() && GetShortcut(shortcut.Id) == nullptr)
     {
-        Shortcuts.push_back(shortcut);
+        auto id = std::make_unique<std::string>(shortcut.Id);
+        auto idView = std::string_view(*id);
+        _ids.push_back(std::move(id));
+        Shortcuts[idView] = shortcut;
     }
 }
 
 RegisteredShortcut* ShortcutManager::GetShortcut(std::string_view id)
 {
-    auto result = std::find_if(Shortcuts.begin(), Shortcuts.end(), [id](const RegisteredShortcut& s) { return s.Id == id; });
-    return result == Shortcuts.end() ? nullptr : &(*result);
+    auto result = Shortcuts.find(id);
+    return result == Shortcuts.end() ? nullptr : &result->second;
 }
 
 void ShortcutManager::RemoveShortcut(std::string_view id)
 {
-    Shortcuts.erase(
-        std::remove_if(
-            Shortcuts.begin(), Shortcuts.end(), [id](const RegisteredShortcut& shortcut) { return shortcut.Id == id; }),
-        Shortcuts.end());
+    Shortcuts.erase(id);
+    _ids.erase(
+        std::remove_if(_ids.begin(), _ids.end(), [id](const std::unique_ptr<std::string>& x) { return *x == id; }), _ids.end());
 }
 
 bool ShortcutManager::IsPendingShortcutChange() const
@@ -153,9 +155,9 @@ void ShortcutManager::ProcessEvent(const InputEvent& e)
     {
         for (const auto& shortcut : Shortcuts)
         {
-            if (shortcut.Matches(e))
+            if (shortcut.second.Matches(e))
             {
-                shortcut.Action();
+                shortcut.second.Action();
             }
         }
     }
@@ -334,15 +336,15 @@ void ShortcutManager::SaveUserBindings(const fs::path& path)
 
     for (const auto& shortcut : Shortcuts)
     {
-        auto& jShortcut = root[shortcut.Id];
-        if (shortcut.Current.size() == 1)
+        auto& jShortcut = root[shortcut.second.Id];
+        if (shortcut.second.Current.size() == 1)
         {
-            jShortcut = shortcut.Current[0].ToString();
+            jShortcut = shortcut.second.Current[0].ToString();
         }
         else
         {
             jShortcut = nlohmann::json::array();
-            for (const auto& binding : shortcut.Current)
+            for (const auto& binding : shortcut.second.Current)
             {
                 jShortcut.push_back(binding.ToString());
             }

src/openrct2-ui/input/ShortcutManager.h

@@ -18,6 +18,7 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -118,8 +119,11 @@ namespace OpenRCT2::Ui
         void LoadUserBindings(const fs::path& path);
         void SaveUserBindings(const fs::path& path);
 
+        // We store the IDs separately so that we can safely use them for string_view in the map
+        std::vector<std::unique_ptr<std::string>> _ids;
+
     public:
-        std::vector<RegisteredShortcut> Shortcuts;
+        std::unordered_map<std::string_view, RegisteredShortcut> Shortcuts;
 
         ShortcutManager(const std::shared_ptr<IPlatformEnvironment>& env);
         ShortcutManager(const ShortcutManager&) = delete;

src/openrct2-ui/windows/ShortcutKeys.cpp

@@ -403,9 +403,9 @@ private:
         auto& shortcutManager = GetShortcutManager();
         for (const auto& shortcut : shortcutManager.Shortcuts)
         {
-            if (IsInCurrentTab(shortcut))
+            if (IsInCurrentTab(shortcut.second))
             {
-                result.push_back(&shortcut);
+                result.push_back(&shortcut.second);
             }
         }
         return result;