From 61e3f1ec0da5f145a4fd3bae256bc17a9ce2b682 Mon Sep 17 00:00:00 2001
From: wraitii <wraitii@wildfiregames.com>
Date: Sun, 15 Sep 2019 12:16:28 +0000
Subject: [PATCH] Fix hotkey events synching with hotkey state.

This is a semi-revert of 78bc56f33e and a correct fix for #3194.

The core issue is that the GUI handler must come behore the hotkey
handler, otherwise typing in boxes can set off hotkeys, and the hotkey
handler is repsonsible for updating the hotkey state.
Thus the GUI handler never has an up-to-date hotkey state, since that's
done later. 78bc56f33e fixed that by calling HotkeyInputHandler
manually, but that was still broken in some (unused) cases and was hacky
(indeed, it even looked hacky as noted by elexis).

The simplest fix is to split the 'hotkey creator' handler from the
'hotkey state change' handler, and run the 'hotkey state change handler'
before any other handler. Thus the gui handler remains in front of the
'hotkey creator' handler, but it has a correct hotkey state at any time.

Differential Revision: https://code.wildfiregames.com/D1839
This was SVN commit r22909.
---
 binaries/data/mods/_test.gui/gui/gui.rng      |  36 ++++++
 binaries/data/mods/_test.gui/gui/gui_page.rng |  14 +++
 binaries/data/mods/_test.gui/gui/hotkey.js    |  14 +++
 binaries/data/mods/_test.gui/gui/hotkey.xml   |   4 +
 .../data/mods/_test.gui/gui/page_hotkey.xml   |   4 +
 source/gui/CGUI.cpp                           |   9 +-
 source/gui/GUIManager.cpp                     |   3 +
 source/gui/tests/test_GuiManager.h            | 110 ++++++++++++++++++
 source/lib/file/common/trace.cpp              |   6 +-
 source/ps/CConsole.cpp                        |   3 +
 source/ps/GameSetup/GameSetup.cpp             |   6 +-
 source/ps/GameSetup/GameSetup.h               |   4 +-
 source/ps/Hotkey.cpp                          |  20 ++--
 source/ps/Hotkey.h                            |   4 +-
 14 files changed, 216 insertions(+), 21 deletions(-)
 create mode 100755 binaries/data/mods/_test.gui/gui/gui.rng
 create mode 100755 binaries/data/mods/_test.gui/gui/gui_page.rng
 create mode 100755 binaries/data/mods/_test.gui/gui/hotkey.js
 create mode 100755 binaries/data/mods/_test.gui/gui/hotkey.xml
 create mode 100755 binaries/data/mods/_test.gui/gui/page_hotkey.xml
 create mode 100755 source/gui/tests/test_GuiManager.h

diff --git a/binaries/data/mods/_test.gui/gui/gui.rng b/binaries/data/mods/_test.gui/gui/gui.rng
new file mode 100755
index 0000000000..9424e0fa21
--- /dev/null
+++ b/binaries/data/mods/_test.gui/gui/gui.rng
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<grammar xmlns:a="http://relaxng.org/ns/compatibility/annotations/1.0" xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes">
+  <start>
+    <a:documentation/>
+    <!--
+      NOTE: To modify this Relax NG grammar, edit the Relax NG Compact (.rnc) file
+        and use a converter tool like trang to generate the Relax NG XML (.rng) file
+    -->
+    <a:documentation/>
+    <choice>
+      <ref name="objects"/>
+    </choice>
+  </start>
+  <define name="objects">
+    <element name="objects">
+      <zeroOrMore>
+        <choice>
+          <ref name="script"/>
+        </choice>
+      </zeroOrMore>
+    </element>
+  </define>
+  <define name="script">
+    <element name="script">
+      <interleave>
+        <text/>
+        <optional>
+          <attribute name="file"/>
+        </optional>
+        <optional>
+          <attribute name="directory"/>
+        </optional>
+      </interleave>
+    </element>
+  </define>
+</grammar>
diff --git a/binaries/data/mods/_test.gui/gui/gui_page.rng b/binaries/data/mods/_test.gui/gui/gui_page.rng
new file mode 100755
index 0000000000..c50e60f9e4
--- /dev/null
+++ b/binaries/data/mods/_test.gui/gui/gui_page.rng
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<element name="page" xmlns:a="http://relaxng.org/ns/compatibility/annotations/1.0" xmlns="http://relaxng.org/ns/structure/1.0">
+  <a:documentation/>
+  <!--
+    NOTE: To modify this Relax NG grammar, edit the Relax NG Compact (.rnc) file
+      and use a converter tool like trang to generate the Relax NG XML (.rng) file
+  -->
+  <a:documentation/>
+  <zeroOrMore>
+    <element name="include">
+      <text/>
+    </element>
+  </zeroOrMore>
+</element>
diff --git a/binaries/data/mods/_test.gui/gui/hotkey.js b/binaries/data/mods/_test.gui/gui/hotkey.js
new file mode 100755
index 0000000000..3ebf6e50e1
--- /dev/null
+++ b/binaries/data/mods/_test.gui/gui/hotkey.js
@@ -0,0 +1,14 @@
+var state_before;
+var state_after;
+
+function handleInputBeforeGui(ev) {
+	if ((ev.type == "hotkeydown" || ev.type == "hotkeyup") && ev.hotkey == "test")
+		state_before = Engine.HotkeyIsPressed("test");
+	return false;
+}
+
+function handleInputAfterGui(ev) {
+	if ((ev.type == "hotkeydown" || ev.type == "hotkeyup") && ev.hotkey == "test")
+		state_after = Engine.HotkeyIsPressed("test");
+	return false;
+}
diff --git a/binaries/data/mods/_test.gui/gui/hotkey.xml b/binaries/data/mods/_test.gui/gui/hotkey.xml
new file mode 100755
index 0000000000..04e929c76c
--- /dev/null
+++ b/binaries/data/mods/_test.gui/gui/hotkey.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<objects>
+	<script file="gui/hotkey.js"/>
+</objects>
diff --git a/binaries/data/mods/_test.gui/gui/page_hotkey.xml b/binaries/data/mods/_test.gui/gui/page_hotkey.xml
new file mode 100755
index 0000000000..e4b18be5a0
--- /dev/null
+++ b/binaries/data/mods/_test.gui/gui/page_hotkey.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<page>
+	<include>hotkey.xml</include>
+</page>
diff --git a/source/gui/CGUI.cpp b/source/gui/CGUI.cpp
index a74fa13c34..d38ae7ff80 100644
--- a/source/gui/CGUI.cpp
+++ b/source/gui/CGUI.cpp
@@ -87,15 +87,10 @@ InReaction CGUI::HandleEvent(const SDL_Event_* ev)
 		if (it != m_HotkeyObjects.end())
 			for (IGUIObject* const& obj : it->second)
 			{
-				// Update hotkey status before sending the event,
-				// else the status will be outdated when processing the GUI event.
-				HotkeyInputHandler(ev);
-				ret = IN_HANDLED;
-
 				if (ev->ev.type == SDL_HOTKEYDOWN)
-					obj->SendEvent(GUIM_PRESSED, "press");
+					ret = obj->SendEvent(GUIM_PRESSED, "press");
 				else
-					obj->SendEvent(GUIM_RELEASED, "release");
+					ret = obj->SendEvent(GUIM_RELEASED, "release");
 			}
 	}
 
diff --git a/source/gui/GUIManager.cpp b/source/gui/GUIManager.cpp
index 9e3766ae44..f6fb9f479c 100644
--- a/source/gui/GUIManager.cpp
+++ b/source/gui/GUIManager.cpp
@@ -44,6 +44,9 @@ CGUIManager* g_GUI = NULL;
 // trampoline: we don't want to make the HandleEvent implementation static
 InReaction gui_handler(const SDL_Event_* ev)
 {
+	if (!g_GUI)
+		return IN_PASS;
+
 	PROFILE("GUI event handler");
 	return g_GUI->HandleEvent(ev);
 }
diff --git a/source/gui/tests/test_GuiManager.h b/source/gui/tests/test_GuiManager.h
new file mode 100755
index 0000000000..01d9e9e295
--- /dev/null
+++ b/source/gui/tests/test_GuiManager.h
@@ -0,0 +1,110 @@
+/* Copyright (C) 2019 Wildfire Games.
+ * This file is part of 0 A.D.
+ *
+ * 0 A.D. is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 0 A.D. is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with 0 A.D.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "lib/self_test.h"
+
+#include "gui/GUIManager.h"
+
+#include "gui/CGUI.h"
+#include "ps/ConfigDB.h"
+#include "ps/Filesystem.h"
+#include "ps/GameSetup/GameSetup.h"
+#include "ps/Hotkey.h"
+#include "ps/XML/Xeromyces.h"
+
+class TestGuiManager : public CxxTest::TestSuite
+{
+	CConfigDB* configDB;
+public:
+
+	void setUp()
+	{
+		g_VFS = CreateVfs();
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir()/"mods"/"_test.gui", VFS_MOUNT_MUST_EXIST));
+		TS_ASSERT_OK(g_VFS->Mount(L"cache", DataDir()/"mods"/"_testcache/"));
+
+		configDB = new CConfigDB;
+
+		CXeromyces::Startup();
+
+		g_GUI = new CGUIManager();
+	}
+
+	void tearDown()
+	{
+		delete g_GUI;
+		CXeromyces::Terminate();
+		delete configDB;
+		g_VFS.reset();
+		DeleteDirectory(DataDir()/"_testcache");
+	}
+
+	void test_hotkeysState()
+	{
+
+		// Load up a fake test hotkey when pressing 'a'.
+		const char* test_hotkey_name = "hotkey.test";
+		g_ConfigDB.SetValueString(CFG_USER, test_hotkey_name, "A");
+		LoadHotkeys();
+
+		// Load up a test page.
+		JS::RootedValue val(g_GUI->GetScriptInterface()->GetContext());
+		g_GUI->GetScriptInterface()->Eval("({})", &val);
+		std::shared_ptr<ScriptInterface::StructuredClone> data = g_GUI->GetScriptInterface()->WriteStructuredClone(JS::NullHandleValue);
+		g_GUI->PushPage(L"page_hotkey.xml", data, JS::UndefinedHandleValue);
+
+		// Press 'a'.
+		SDL_Event_ hotkeyNotification;
+		hotkeyNotification.ev.type = SDL_KEYDOWN;
+		hotkeyNotification.ev.key.keysym.sym = SDLK_a;
+
+		// Init input and poll the event.
+		InitInput();
+		in_push_priority_event(&hotkeyNotification);
+		SDL_Event_ ev;
+		while (in_poll_event(&ev))
+			in_dispatch_event(&ev);
+
+		// Ensure that our hotkey state was synchronised with the event itself.
+		bool hotkey_pressed_value = false;
+		JS::RootedValue js_hotkey_pressed_value(g_GUI->GetActiveGUI()->GetScriptInterface()->GetContext());
+		g_GUI->GetActiveGUI()->GetScriptInterface()->Eval("state_before", &js_hotkey_pressed_value);
+		g_GUI->GetActiveGUI()->GetScriptInterface()->FromJSVal(g_GUI->GetActiveGUI()->GetScriptInterface()->GetContext(), js_hotkey_pressed_value, hotkey_pressed_value);
+		TS_ASSERT_EQUALS(hotkey_pressed_value, true);
+
+		hotkey_pressed_value = false;
+		g_GUI->GetActiveGUI()->GetScriptInterface()->Eval("state_after", &js_hotkey_pressed_value);
+		g_GUI->GetActiveGUI()->GetScriptInterface()->FromJSVal(g_GUI->GetActiveGUI()->GetScriptInterface()->GetContext(), js_hotkey_pressed_value, hotkey_pressed_value);
+		TS_ASSERT_EQUALS(hotkey_pressed_value, true);
+
+		hotkeyNotification.ev.type = SDL_KEYUP;
+		in_push_priority_event(&hotkeyNotification);
+		while (in_poll_event(&ev))
+			in_dispatch_event(&ev);
+
+		hotkey_pressed_value = true;
+		g_GUI->GetActiveGUI()->GetScriptInterface()->Eval("state_before", &js_hotkey_pressed_value);
+		g_GUI->GetActiveGUI()->GetScriptInterface()->FromJSVal(g_GUI->GetActiveGUI()->GetScriptInterface()->GetContext(), js_hotkey_pressed_value, hotkey_pressed_value);
+		TS_ASSERT_EQUALS(hotkey_pressed_value, false);
+
+		hotkey_pressed_value = true;
+		g_GUI->GetActiveGUI()->GetScriptInterface()->Eval("state_after", &js_hotkey_pressed_value);
+		g_GUI->GetActiveGUI()->GetScriptInterface()->FromJSVal(g_GUI->GetActiveGUI()->GetScriptInterface()->GetContext(), js_hotkey_pressed_value, hotkey_pressed_value);
+		TS_ASSERT_EQUALS(hotkey_pressed_value, false);
+
+	}
+};
diff --git a/source/lib/file/common/trace.cpp b/source/lib/file/common/trace.cpp
index c25a2a4658..90bb7a1b71 100644
--- a/source/lib/file/common/trace.cpp
+++ b/source/lib/file/common/trace.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 2015 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  *
  * Permission is hereby granted, free of charge, to any person obtaining
  * a copy of this software and associated documentation files (the
@@ -88,9 +88,13 @@ TraceEntry::TraceEntry(const std::wstring& text)
 
 std::wstring TraceEntry::EncodeAsText() const
 {
+	// Ensure timestamp gets correctly encoded.
+	char* oldLocale = setlocale(LC_ALL, "C");
+
 	const wchar_t action = (wchar_t)m_action;
 	wchar_t buf[1000];
 	swprintf_s(buf, ARRAY_SIZE(buf), L"%#010f: %c \"%ls\" %lu\n", m_timestamp, action, m_pathname.string().c_str(), (unsigned long)m_size);
+	setlocale(LC_ALL, oldLocale);
 	return buf;
 }
 
diff --git a/source/ps/CConsole.cpp b/source/ps/CConsole.cpp
index 167ead6f69..78ec604194 100644
--- a/source/ps/CConsole.cpp
+++ b/source/ps/CConsole.cpp
@@ -635,6 +635,9 @@ static bool isUnprintableChar(SDL_Keysym key)
 
 InReaction conInputHandler(const SDL_Event_* ev)
 {
+	if (!g_Console)
+		return IN_PASS;
+
 	if ((int)ev->ev.type == SDL_HOTKEYDOWN)
 	{
 		std::string hotkey = static_cast<const char*>(ev->ev.user.data1);
diff --git a/source/ps/GameSetup/GameSetup.cpp b/source/ps/GameSetup/GameSetup.cpp
index 7de7880ed7..7e4b659163 100644
--- a/source/ps/GameSetup/GameSetup.cpp
+++ b/source/ps/GameSetup/GameSetup.cpp
@@ -541,7 +541,7 @@ void InitPsAutostart(bool networked, JS::HandleValue attrs)
 }
 
 
-static void InitInput()
+void InitInput()
 {
 	g_Joystick.Initialise();
 
@@ -569,6 +569,10 @@ static void InitInput()
 
 	// must be registered after (called before) the GUI which relies on these globals
 	in_add_handler(GlobalsInputHandler);
+
+	// Should be called first, this updates our hotkey press state
+	// so that js calls to HotkeyIsPressed are synched with events.
+	in_add_handler(HotkeyStateChange);
 }
 
 
diff --git a/source/ps/GameSetup/GameSetup.h b/source/ps/GameSetup/GameSetup.h
index 4f868aa82f..5942b4d973 100644
--- a/source/ps/GameSetup/GameSetup.h
+++ b/source/ps/GameSetup/GameSetup.h
@@ -92,8 +92,8 @@ extern void MountMods(const Paths& paths, const std::vector<CStr>& mods);
  * In the latter case the caller should call Shutdown() with SHUTDOWN_FROM_CONFIG.
  */
 extern bool Init(const CmdLineArgs& args, int flags);
-extern void InitGraphics(const CmdLineArgs& args, int flags,
-	const std::vector<CStr>& installedMods = std::vector<CStr>());
+extern void InitInput();
+extern void InitGraphics(const CmdLineArgs& args, int flags, const std::vector<CStr>& installedMods = std::vector<CStr>());
 extern void InitNonVisual(const CmdLineArgs& args);
 extern void Shutdown(int flags);
 extern void CancelLoad(const CStrW& message);
diff --git a/source/ps/Hotkey.cpp b/source/ps/Hotkey.cpp
index dd6bf4db3c..f7296fd13f 100644
--- a/source/ps/Hotkey.cpp
+++ b/source/ps/Hotkey.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -152,6 +152,15 @@ bool isNegated(const SKey& key)
 		return true;
 }
 
+InReaction HotkeyStateChange(const SDL_Event_* ev)
+{
+	if (ev->ev.type == SDL_HOTKEYDOWN)
+		g_HotkeyStatus[static_cast<const char*>(ev->ev.user.data1)] = true;
+	else if (ev->ev.type == SDL_HOTKEYUP)
+		g_HotkeyStatus[static_cast<const char*>(ev->ev.user.data1)] = false;
+	return IN_PASS;
+}
+
 InReaction HotkeyInputHandler(const SDL_Event_* ev)
 {
 	int keycode = 0;
@@ -196,13 +205,6 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
 		}
 		return IN_PASS;
 
-	case SDL_HOTKEYDOWN:
-		g_HotkeyStatus[static_cast<const char*>(ev->ev.user.data1)] = true;
-		return IN_PASS;
-
-	case SDL_HOTKEYUP:
-		g_HotkeyStatus[static_cast<const char*>(ev->ev.user.data1)] = false;
-		return IN_PASS;
 
 	default:
 		return IN_PASS;
@@ -248,7 +250,7 @@ InReaction HotkeyInputHandler(const SDL_Event_* ev)
 
 	bool consoleCapture = false;
 
-	if (g_Console->IsActive() && keycode < SDL_SCANCODE_TO_KEYCODE(SDL_NUM_SCANCODES))
+	if (g_Console && g_Console->IsActive() && keycode < SDL_SCANCODE_TO_KEYCODE(SDL_NUM_SCANCODES))
 		consoleCapture = true;
 
 	// Here's an interesting bit:
diff --git a/source/ps/Hotkey.h b/source/ps/Hotkey.h
index cfad324af8..e5356dc127 100644
--- a/source/ps/Hotkey.h
+++ b/source/ps/Hotkey.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2014 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -43,6 +43,8 @@ const int SDL_HOTKEYUP = SDL_USEREVENT + 1;
 
 extern void LoadHotkeys();
 extern void UnloadHotkeys();
+
+extern InReaction HotkeyStateChange(const SDL_Event_* ev);
 extern InReaction HotkeyInputHandler(const SDL_Event_* ev);
 
 extern bool HotkeyIsPressed(const CStr& keyname);