[PATCH] SDL 2.0.4 update broke hotkeys on windows (Opening team chat with key T automatically writes a t in input dialog)

[Imarok]

On windows, opening the team chat window with key T automatically writes a t in input dialog A Patch could be to open the team chat window not until the t key has been released.

---

Testing on Windows revealed that the SDL 2.0.4 update in r17658 introduced this behavior.

[elexis]

I heard this from others too, but I can't reproduce it. According to sanderd17, it has been like that since some time, but only when the dialog is opened the first time. Maybe it is a platform-dependent bug?

Can someone check if adding chatInput.caption = ""; to the end of function toggleChatWindow() in menu.js is sufficient?

If it truly is a platform-dependent bug, it might appear in other places too though...

[Imarok]

" but only when the dialog is opened the first time": When you mean the bug only appears the first time you open team chat via "t" and after sending a message and reopen team chat with "t" the bug doesnt occur: thats wrong. The bug always appears. I'm using windows.

[Imarok]

I will try it after downloading visual c++ and the source. ;) Do you mean the menu.js in: ​ps/trunk/binaries/data/mods/public/gui/session/menu.js?

[sanderd17]

There's no need to use visual C++. The changes should be done in the scripts (*.js files), so you don't have to compile those. And we offer a precompiled version of the game for windows.

So you should download the source, then modify that file in a good text editor (like notepad++), and then you can just run it from the precompiled binary (under binaries/system).

[Imarok]

Seems logic :D

I changed the menu.js at binaries\data\mods\public\gui\session to

function toggleChatWindow(teamChat)
{
	if (g_Disconnected)
		return;

	let chatWindow = Engine.GetGUIObjectByName("chatDialogPanel");
	let chatInput = Engine.GetGUIObjectByName("chatInput");
	let hidden = chatWindow.hidden;

	closeOpenDialogs();

	if (hidden)
	{
		setTeamChat(teamChat);
		chatInput.focus();
	}
	else
	{
		if (chatInput.caption.length)
		{
			submitChatInput();
			return;
		}
		chatInput.caption = "";
	}

	chatWindow.hidden = !hidden;
	chatInput.caption = "";
}

started 0ad with binaries/system/pyrogenesis.exe and the bug still remains :(

[elexis]

Do you still have alpha 19? It didn't happen there?

[Imarok]

I installed the new alpha 20 3 hours ago :( I don't know if it happend in alpha 19

[Imarok]

I reinstalled alpha 19 now. There the bug only occurs the first time in the running time of the program. In alpha 20 it occurs always.

[sanderd17]

Imarok, can you add a line like

warn("it works");

to that file (next to the line you added), just to assure that the modifications were read (and the game isn't reading the scripts from some other installation. You should see a yellow "It works" line appearing then every time you open the chat window.

[Imarok]

I added this line, the warning came when pressing t but the bug stayed. I noticed pressing Ctrl+T opens the team chat window without writing t into the input box

[njm]

I think the function toggleChatWindow in menu.js should consume the key event to stop its propagation/bubbling. But I don't know how (yet).

[Imarok]

Another way would be to let the function wait until the key is unpressed.

[Dummer]

with AutoIt this with be easy

[elexis]

To me this looks like CGUI.cpp sending the press event on SDL_HOTKEYDOWN, while it should be SDL_HOTKEYUP, as the latter one also waits for the button to be released. Changing this however closes the chatbox again when typing t.

[njm]

I suggest to only open the chat window with the hotkey on SDL_HOTKEYUP (as suggested by elexis) and close the chat window when a message is submitted or the dialog is cancelled. The second part is already implemented. The first part would mean to rename toggleChatWindow to openChatWindow and only open the chat window, if it isn't open. Additionally submitChatInput in messages.js should call closeChat in menu.js instead of toggleChatWindow.

The "else"-part in toggleChatWindow seems to be obsolete anyway, because the chat dialog in session.xml uses the function submitChatInput directly.

Do you agree? If so, I can do it.

[Imarok]

I'm just a "noob" to the 0ad code, but this sounds good.

[Imarok]

change SDL_HOTKEYDOWN to SDL_HOTKEYUP and move the text input part

[elexis]

Doesn't work fully here (linux):

• Escape doesn't close the chat dialog
• Enter sends the chat and opens it again

Also when moving code, the code should be optimized:

• operators like || should appear in the end of a line
• those 2 ifs could be merged

Not sure if the comment would remain true after moving // else will return IN_PASS because we never used the button

But it seems that code path you moved is the one being triggered incorrectly.

[Imarok]

fixes #3870 but the inserted tchat text doesn't get sended anymore

[Imarok]

This patch fixes the bug but the inserted text doesn't get sent. I can't figure out why...

[Imarok]

Now the chat message gets sent

[elexis]

Closes the chat window when typing t (on a gnu/linux machine) :-L

[Imarok]

I'll give it up :(

[njm]

Replying to Imarok:

I'll give it up :(

Ok, I'll try tomorrow.

[Imarok]

Replying to njm:

Replying to Imarok:

I'll give it up :(

Ok, I'll try tomorrow.

Good luck ;)

[njm]

Improvement to Imarok's patch: Typing t in chat does not close the chat anymore. More information below.

[njm]

Information regarding above keyfixv4.patch: I used the last patch of Imarok (thanks!), that altered C++ files, and I altered JavaScript files.

• Typing a t in chat now does not close the chat anymore.
• Renamed toggleChatWindow to openChatWindow, because it is/was only used for that. Altered containing code.
• Deleted openChat, that was called when opening the chat window via menu, and use openChatWindow instead. Like this, the chat input is not deleted and now focused, when clicking Chat in the menu.
• Further improvement would be to set the cursor to the end of the chat input, when the Chat button in the menu is clicked.

[elexis]

Awesome, the patch works, great teamwork!

Can someone recapitulate what the c++ change does and which of these JS changes are crucial to fix the bug?

Now to the codestyle-nitpicking:

• Why replace the call to closeOpenDialogs with closeMenu()? The former does the same and closes also other stuff, so should be saner.
• if( should be if (
• agreing that toggleChat should be replaced with openChat and closeChat
• closeChat()` -> an early return would be better with regards to codestyle. But why add the check in the first place? If you don't want to clear chat in both cases (opening&closing), then completely remove the duplicate clears (and test that it doesnt introduce bugs).
• Make a variable for the GUI object chatInput as well, the clear-chat comment is not needed
• As mentioned by Imarok, funciton names should be consistent (either both or none of them should contain "Window")

[sanderd17]

Just tested the patch on Linux, and it indeed got rid of the annoying "t" (which I only got on the first opening of the dialog). And I didn't notice any negative side effects.

So functionality-wise, this is fine IMO, and I'll leave it to elexis to do the code review.

[elexis]

(Most) hotkeys shouldn't work if chat is focused. For example whenever the user types the idle-worker-hotkey (dot), then an idle worker will be selected. The action was not intended though, as the user only wanted to add text. Another example is selecting a CC and then typing a queue hotkey, (Y/Z, X, C, V, ...)

[elexis]

Replying to Imarok:

I reinstalled alpha 19 now. There the bug only occurs the first time in the running time of the program. In alpha 20 it occurs always.

So find the revision that introduced the platform-specific difference. It was somewhere between r17299 (alpha19) and r17965 (alpha 20).

As SDL is responsible for input, maybe it was r17658?

Boot windows, revert your stuff, do svn update -r <revision> and compile the code.

You only need a logarithmical amount of tries to find the commit. If the bug occurs on rev X but not rev Y, then check for the revision right between them. Thus you divide the number of commits to check by 2 everytime.

[Imarok]

Now hotkeys shouldn't work in chat

[Imarok]

The new patch disables escape :(

[elexis]

Nominated for most annoying bugs of the last release. Doesn't need to stay, but should really be fixed.

[Imarok]

r17658 is responsible for this bug. (I tested that)