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

@@ -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>

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>

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;
+}

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>

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>

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");
 			}
 	}
 

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);
 }

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);
+
+	}
+};

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;
 }
 

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);

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);
 }
 
 

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);

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:

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);