Merge lp:~thenifker13-deactivatedaccount/widelands/trunk into lp:widelands

[Antonino Siena]

Window closing, opening OptionsMenu and exiting using the keyboard.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5130. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/540341469.
Appveyor build 4912. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_thenifker13_widelands_trunk-4912.

[lp:~thenifker13-deactivatedaccount/widelands/trunk]

9124. By Antonino Siena on 2019-06-02

Added space to comment

[Toni Förster (stonerl)]

When a window looses focus, it cannot be closed by ESC any more. E.g. open the stats menu and then the mini map. The mini map can be closed with ESC but not the stats menu. Instead, the main menu opens.

review: Needs Fixing

[Antonino Siena (thenifker13-deactivatedaccount)]

Is it possible to access all windows(like in a vector/array)?

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5136. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/540464281.
Appveyor build 4918. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_thenifker13_widelands_trunk-4918.

[GunChleoc (gunchleoc)]

No, but you can access a Panel's children and parent internally. Have a look at Panel::do_think() to see how to iterate through a panel's children.

[Antonino Siena (thenifker13-deactivatedaccount)]

Fixed it - will push it now!

[lp:~thenifker13-deactivatedaccount/widelands/trunk]

9125. By Antonino Siena on 2019-06-03

When closing a window using ESCAPE will cause the next open window to gain focus

[Toni Förster (stonerl)]

LGTM :)

There are some menus in the game, e.g. the tutorial screen, that do not handle the ESC key. That's not part of the feature but if you'd like to tip your toes into, it would be much appreciated.

review: Approve

[Antonino Siena (thenifker13-deactivatedaccount)]

I'm gonna look into these too, contemporary.

[Antonino Siena (thenifker13-deactivatedaccount)]

So you mean it should be implemented for 'story_message_box' too?

[lp:~thenifker13-deactivatedaccount/widelands/trunk]

9126. By Antonino Siena on 2019-06-03

Allow ESCAPE to close story_message_box

9127. By Antonino Siena on 2019-06-03

Open and Close Editor menu using ESCAPE and exit from from Editor using ENTER

9128. By Antonino Siena on 2019-06-04

Added ESCAPE -> back functionality for main menu menues

[GunChleoc (gunchleoc)]

I have added some notes to the diff for further improvements that you can make.

[Antonino Siena (thenifker13-deactivatedaccount)]

Forgot to add the binding there, thanks.

Yes I want to add a binding for GameMessageMenu too, but I didnt figure
yet out how to properly bind in this case.
I tried it previously like this:
/SDL_Keysym esc = SDL_Keysym{SDL_SCANCODE_ESCAPE, SDLK_ESCAPE,
KMOD_NONE, 0}; list->cancel.connect(boost::bind(&UI::Window::handle_key,
boost::ref(*this), _1, _2)(true, esc) );/

Ok I just removed the else case as it is not necessary because of return
in the if case.

Am 04.06.19 um 17:44 schrieb GunChleoc:
> I have added some notes to the diff for further improvements that you can make.
>
> Diff comments:
>
>> === modified file 'src/ui_fsmenu/loadgame.cc'
>> --- src/ui_fsmenu/loadgame.cc 2019-05-26 17:21:15 +0000
>> +++ src/ui_fsmenu/loadgame.cc 2019-06-04 12:56:27 +0000
>> @@ -170,6 +170,10 @@
>> }
>>
>> bool FullscreenMenuLoadGame::handle_key(bool down, SDL_Keysym code) {
>> + if (code.sym == SDLK_ESCAPE)
> You should be able to define load_or_save_.table_.cancel.connect instead.
>
>> + FullscreenMenuLoadGame::clicked_back();
>> + return true;
>> +
>> if (!down)
>> return false;
>>
>>
>> === modified file 'src/wui/game_message_menu.cc'
>> --- src/wui/game_message_menu.cc 2019-05-26 17:21:15 +0000
>> +++ src/wui/game_message_menu.cc 2019-06-04 12:56:27 +0000
>> @@ -362,6 +362,10 @@
>> * Handle message menu hotkeys.
>> */
>> bool GameMessageMenu::handle_key(bool down, SDL_Keysym code) {
>> + // Special ESCAPE handling
>> + // When ESCAPE is pressed down is false
>> + if (code.sym == SDLK_ESCAPE)
>> + return UI::Window::handle_key(true, code);
> The difference for ths function is that it has
>
>
> return list->handle_key(down, code);
>
> on the bottom. Do an if-else around that and you might not need the special handling.
>
>> if (down) {
>> switch (code.sym) {
>> // Don't forget to change the tooltips if any of these get reassigned
>>
>> === modified file 'src/wui/game_options_menu.cc'
>> --- src/wui/game_options_menu.cc 2019-03-18 07:11:02 +0000
>> +++ src/wui/game_options_menu.cc 2019-06-04 12:56:27 +0000
>> @@ -101,6 +101,28 @@
>> center_to_parent();
>> }
>>
>> +bool GameOptionsMenu::handle_key(bool down, SDL_Keysym code) {
>> + if (down) {
>> + switch (code.sym) {
>> + case SDLK_ESCAPE:
>> + die();
>> + return true;
>> + case SDLK_RETURN:
>> + if ((code.mod & (KMOD_LCTRL | KMOD_RCTRL))) {
>> + igb_.end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
>> + return true;
>> + }
>> + else
> We try to avoid if/else/for without {}, because leaving them out can cause bugs.
>
>> + new GameExitConfirmBox(*get_parent(), igb_);
>> + die();
>> + return true;
>> + default:
>> + break;
>> + }
>> + }
>> + return UI::Window::handle_key(down, code);
>> +}
>> +
>> GameOptionsMenu::~GameOptionsMenu() {
>> }
>>
>

[lp:~thenifker13-deactivatedaccount/widelands/trunk]

9129. By Antonino Siena on 2019-06-04

Removed unnecessary else-case

9130. By Antonino Siena on 2019-06-04

Proper ESCAPE handling

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5153. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/541384195.
Appveyor build 4935. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_thenifker13_widelands_trunk-4935.

[GunChleoc (gunchleoc)]

2 more comments.

This branch breaks map keyboard navigation with the arrow keys though as soon as a buildingwindow is open. So, BuildingWindow needs to check whether Window has handled the key and if not, delegate it to its parent_.

[Antonino Siena (thenifker13-deactivatedaccount)]

Firstly how do I check if the sibling I get from get_next_sibling() from Panel is actually one?
And from which windows should it be possible to navigate?

[Antonino Siena (thenifker13-deactivatedaccount)]

Trying in window.cc - doesnt work so far:
"
case SDLK_UP:
case SDLK_DOWN:
case SDLK_LEFT:
case SDLK_RIGHT:{
 Panel *p = get_parent();
 return p->handle_key(down, code);
}
"

[Antonino Siena (thenifker13-deactivatedaccount)]

No ideas?

[Antonino Siena (thenifker13-deactivatedaccount)]

Literally no one that has any idea?

[GunChleoc (gunchleoc)]

Sorry I didn't get back to you earlier, I don't have much time for Widelands at the moment.

I did some digging into the code, and for some weird reason, map movement with the arrow keys is implemented in InteractiveBase::think().

This is also creating problems for me in another branch, so I think I better have a look at it in a new branch, and then retest this branch once the other branch has been merged.

[GunChleoc (gunchleoc)]

Merge request for the map keyboard navigation is up: https://code.launchpad.net/~widelands-dev/widelands/arrow-keys-map-movement/+merge/370687

[GunChleoc (gunchleoc)]

The other branch has been merged now. Can you please merge trunk into this one so that we can retest?

[lp:~thenifker13-deactivatedaccount/widelands/trunk]

9131. By Antonino Siena on 2019-07-28

Merged trunk here

[Antonino Siena (thenifker13-deactivatedaccount)]

Merged trunk right now.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5289. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/564632804.
Appveyor build 5064. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_thenifker13_widelands_trunk-5064.

[Antonino Siena (thenifker13-deactivatedaccount)]

Has someone else tested it already?

[GunChleoc (gunchleoc)]

I'm sorry, Launchpad did not notify us on your comments. *headdesk* Can you contact me personally via https://launchpad.net/~gunchleoc next time, so I won't miss this again? We will be moving to GitHub too next weekend. If you don't want to move with us, just let me know and I'll finish up this branch over there.

The files src/editor/ui_menus/main_menu.cc/h and src/wui/game_options_menu.cc/h now don't exist any more, because we replace them with toolbar menus. So, you'll have to merge trunk again before we can finalize this branch.

I have done some testing, and it is working well now :)

[Antonino Siena (thenifker13-deactivatedaccount)]

Ill do so. Id rather move to GitLab but GitHub should be fine.
When trying to merge I then got this compilation error I was unsure about:
"
../src/editor/editorinteractive.cc:844:8: error: expected type-specifier before ‘EditorMainMenu’
  844 | new EditorMainMenu(*this, mainmenu_);
      | ^~~~~~~~~~~~~~
"

[GunChleoc (gunchleoc)]

That's because the class EditorMainMenu does not exist any more. It has been replaced with UI::Dropdown<MainMenuEntry> mainmenu_.

[Antonino Siena (thenifker13-deactivatedaccount)]

Ok, good to know - Ill take a look at it the next days.

Am 08.09.19 um 17:28 schrieb GunChleoc:
> That's because the class EditorMainMenu does not exist any more. It has been replaced with UI::Dropdown<MainMenuEntry> mainmenu_.

[GunChleoc (gunchleoc)]

Migrated to https://github.com/widelands/widelands/tree/windows-esc

Unmerged revisions

9131. By Antonino Siena on 2019-07-28

Merged trunk here

9130. By Antonino Siena on 2019-06-04

Proper ESCAPE handling

9129. By Antonino Siena on 2019-06-04

Removed unnecessary else-case

9128. By Antonino Siena on 2019-06-04

Added ESCAPE -> back functionality for main menu menues

9127. By Antonino Siena on 2019-06-03

Open and Close Editor menu using ESCAPE and exit from from Editor using ENTER

9126. By Antonino Siena on 2019-06-03

Allow ESCAPE to close story_message_box

9125. By Antonino Siena on 2019-06-03

When closing a window using ESCAPE will cause the next open window to gain focus

9124. By Antonino Siena on 2019-06-02

Added space to comment

9123. By Antonino Siena on 2019-06-01

Instant exit using CTRL+ENTER

9122. By Antonino Siena on 2019-06-01

Removed empty line and changed special ESCAPE handling a bit

=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc	2019-04-26 05:52:49 +0000
+++ src/editor/editorinteractive.cc	2019-07-28 13:42:36 +0000
@@ -566,7 +566,9 @@
 		case SDLK_F1:
 			helpmenu_.toggle();
 			return true;
-
+		case SDLK_ESCAPE:
+			new EditorMainMenu(*this, mainmenu_);
+			return true;
 		default:
 			break;
 		}

=== modified file 'src/editor/ui_menus/main_menu.cc'
--- src/editor/ui_menus/main_menu.cc	2019-02-23 11:00:49 +0000
+++ src/editor/ui_menus/main_menu.cc	2019-07-28 13:42:36 +0000
@@ -82,6 +82,22 @@
 		center_to_parent();
 }
 
+bool EditorMainMenu::handle_key(bool down, SDL_Keysym code) {
+	if (down) {
+		switch (code.sym) {
+			case SDLK_ESCAPE:
+				die();
+				return true;
+			case SDLK_RETURN:
+				exit_btn();
+				return true;
+			default:
+				break;
+		}
+	}
+	return UI::Window::handle_key(down, code);
+}
+
 void EditorMainMenu::new_map_btn() {
 	new MainMenuNewMap(eia());
 	die();

=== modified file 'src/editor/ui_menus/main_menu.h'
--- src/editor/ui_menus/main_menu.h	2019-02-23 11:00:49 +0000
+++ src/editor/ui_menus/main_menu.h	2019-07-28 13:42:36 +0000
@@ -32,6 +32,8 @@
 struct EditorMainMenu : public UI::UniqueWindow {
 	EditorMainMenu(EditorInteractive&, UI::UniqueWindow::Registry&);
 
+	bool handle_key(bool down, SDL_Keysym code) override;
+
 private:
 	EditorInteractive& eia();
 	UI::Box box_;

=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc	2019-07-27 11:20:37 +0000
+++ src/ui_basic/window.cc	2019-07-28 13:42:36 +0000
@@ -101,6 +101,7 @@
 	   VT_B_PIXMAP_THICKNESS, VT_B_PIXMAP_THICKNESS, TP_B_PIXMAP_THICKNESS, BT_B_PIXMAP_THICKNESS);
 	set_top_on_click(true);
 	set_layout_toplevel(true);
+	focus();
 }
 
 /**
@@ -411,6 +412,25 @@
 	return true;
 }
 
+bool Window::handle_key(bool down, SDL_Keysym code) {
+	// Handles a key input and event and will close when pressing ESC
+
+	if (down) {
+		switch (code.sym) {
+			case SDLK_ESCAPE: {
+				die();
+				Panel *ch = get_next_sibling();
+				if (ch != nullptr)
+					ch->focus();
+				return true;
+			}
+			default:
+				break;
+		}
+	}
+	return UI::Panel::handle_key(down, code);
+}
+
 /**
  * Close the window. Overwrite this virtual method if you want
  * to take some action before the window is destroyed, or to

=== modified file 'src/ui_basic/window.h'
--- src/ui_basic/window.h	2019-02-23 11:00:49 +0000
+++ src/ui_basic/window.h	2019-07-28 13:42:36 +0000
@@ -98,6 +98,7 @@
 	handle_mousemove(uint8_t state, int32_t mx, int32_t my, int32_t xdiff, int32_t ydiff) override;
 	bool handle_mousewheel(uint32_t which, int32_t x, int32_t y) override;
 	bool handle_tooltip() override;
+	bool handle_key(bool down, SDL_Keysym code) override;
 
 protected:
 	void die() override;

=== modified file 'src/ui_fsmenu/campaign_select.cc'
--- src/ui_fsmenu/campaign_select.cc	2019-05-26 17:21:15 +0000
+++ src/ui_fsmenu/campaign_select.cc	2019-07-28 13:42:36 +0000
@@ -74,6 +74,8 @@
 	table_.focus();
 	fill_table();
 	layout();
+
+	table_.cancel.connect(boost::bind(&FullscreenMenuCampaignSelect::clicked_back, boost::ref(*this)));
 }
 
 void FullscreenMenuCampaignSelect::layout() {

=== modified file 'src/ui_fsmenu/loadgame.cc'
--- src/ui_fsmenu/loadgame.cc	2019-05-26 17:21:15 +0000
+++ src/ui_fsmenu/loadgame.cc	2019-07-28 13:42:36 +0000
@@ -109,6 +109,8 @@
 	if (!load_or_save_.table().empty()) {
 		load_or_save_.table().select(0);
 	}
+
+	load_or_save_.table_.cancel.connect(boost::bind(&FullscreenMenuLoadGame::clicked_back, boost::ref(*this)));
 }
 
 void FullscreenMenuLoadGame::layout() {

=== modified file 'src/ui_fsmenu/mapselect.cc'
--- src/ui_fsmenu/mapselect.cc	2019-05-26 17:21:15 +0000
+++ src/ui_fsmenu/mapselect.cc	2019-07-28 13:42:36 +0000
@@ -298,6 +298,8 @@
 		table_.select(0);
 	}
 	set_has_selection();
+
+	table_.cancel.connect(boost::bind(&FullscreenMenuMapSelect::clicked_back, boost::ref(*this)));
 }
 
 /*

=== modified file 'src/ui_fsmenu/scenario_select.cc'
--- src/ui_fsmenu/scenario_select.cc	2019-05-26 17:21:15 +0000
+++ src/ui_fsmenu/scenario_select.cc	2019-07-28 13:42:36 +0000
@@ -118,6 +118,8 @@
 	table_.focus();
 	fill_table();
 	layout();
+
+	table_.cancel.connect(boost::bind(&FullscreenMenuScenarioSelect::clicked_back, boost::ref(*this)));
 }
 
 void FullscreenMenuScenarioSelect::layout() {

=== modified file 'src/wui/game_message_menu.cc'
--- src/wui/game_message_menu.cc	2019-06-25 07:34:58 +0000
+++ src/wui/game_message_menu.cc	2019-07-28 13:42:36 +0000
@@ -360,6 +360,10 @@
  * Handle message menu hotkeys.
  */
 bool GameMessageMenu::handle_key(bool down, SDL_Keysym code) {
+	// Special ESCAPE handling
+	// When ESCAPE is pressed down is false
+	if (code.sym == SDLK_ESCAPE)
+		return UI::Window::handle_key(true, code);
 	if (down) {
 		switch (code.sym) {
 		// Don't forget to change the tooltips if any of these get reassigned

=== modified file 'src/wui/game_options_menu.cc'
--- src/wui/game_options_menu.cc	2019-03-18 07:11:02 +0000
+++ src/wui/game_options_menu.cc	2019-07-28 13:42:36 +0000
@@ -101,6 +101,28 @@
 		center_to_parent();
 }
 
+bool GameOptionsMenu::handle_key(bool down, SDL_Keysym code) {
+	if (down) {
+		switch (code.sym) {
+			case SDLK_ESCAPE:
+				die();
+				return true;
+			case SDLK_RETURN:
+				if ((code.mod & (KMOD_LCTRL | KMOD_RCTRL))) {
+					igb_.end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
+					return true;
+				}
+				
+				new GameExitConfirmBox(*get_parent(), igb_);
+				die();
+				return true;
+			default:
+				break;
+		}
+	}
+	return UI::Window::handle_key(down, code);
+}
+
 GameOptionsMenu::~GameOptionsMenu() {
 }
 

=== modified file 'src/wui/game_options_menu.h'
--- src/wui/game_options_menu.h	2019-02-23 11:00:49 +0000
+++ src/wui/game_options_menu.h	2019-07-28 13:42:36 +0000
@@ -32,8 +32,13 @@
 	GameOptionsMenu(InteractiveGameBase&,
 	                UI::UniqueWindow::Registry&,
 	                InteractiveGameBase::GameMainMenuWindows&);
+
+	bool handle_key(bool down, SDL_Keysym code) override;
+
+
 	~GameOptionsMenu();
 
+
 private:
 	InteractiveGameBase& igb_;
 	InteractiveGameBase::GameMainMenuWindows& windows_;

=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc	2019-05-31 19:31:45 +0000
+++ src/wui/interactive_player.cc	2019-07-28 13:42:36 +0000
@@ -487,7 +487,9 @@
 			map_view()->scroll_to_field(
 			   game().map().get_starting_pos(player_number_), MapView::Transition::Smooth);
 			return true;
-
+		case SDLK_ESCAPE:
+			new GameOptionsMenu(*this, options_, main_windows_);
+			return true;
 		case SDLK_KP_ENTER:
 		case SDLK_RETURN:
 			if (chat_provider_) {

=== modified file 'src/wui/login_box.cc'
--- src/wui/login_box.cc	2019-06-01 11:29:24 +0000
+++ src/wui/login_box.cc	2019-07-28 13:42:36 +0000
@@ -81,6 +81,9 @@
 	}
 
 	eb_nickname->focus();
+
+	eb_nickname->cancel.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));
+	eb_password->cancel.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));
 }
 
 /// think function of the UI (main loop)

=== modified file 'src/wui/story_message_box.cc'
--- src/wui/story_message_box.cc	2019-02-23 11:00:49 +0000
+++ src/wui/story_message_box.cc	2019-07-28 13:42:36 +0000
@@ -97,6 +97,9 @@
 		case SDLK_RETURN:
 			clicked_ok();
 			return true;
+		case SDLK_ESCAPE:
+			clicked_ok();
+			return UI::Window::handle_key(down, code);
 		default:
 			break;  // not handled
 		}