Merge lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands

[Klaus Halfmann]

Avoid null access in Window::center_to_parent()

Simple check if Window has a parent instead of crashing.

This leaves the window in the top-lfet corner, but avoids the crash.

Gun: can we center to some global window instead.

[GunChleoc (gunchleoc)]

No, we can't center to a global parent, because the direct parent is already nullptr.

I am wondering whether this fix doesn't just mask the real bug - it would be interesting to track down why that particular window has no parent. There's something wrong with the control flow there.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4076. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/435018665.
Appveyor build 3872. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1794339_center_wo_parent-3872.

[Klaus Halfmann (klaus-halfmann)]

In my case the root cause was a savegame incompatibilty.

void GameClient::disconnect in network/gameclient.cc creates the messagebox as:
UI::WLMessageBox mmb(d->modal, ....

d is a GameClientImpl* and modal is explicily set as nullpointer in the CTor.
modal is always set to the currently show window and then back to nullptr.

there ist a catch all in GameClient:run() that will call disconnect("CLIENT_CRASHED");

I assume the execption is raised in game.init_savegame(loader_ui, d->settings);

I will try setting d->modal to InteractiveGameBase as early as possible.

[GunChleoc (gunchleoc)]

Another possible solution might be to do 2 things:

1. Set modal to nullptr only after calling disconnect
2. Showing the message only if model != nullptr and logging to console otherwise

So,

  d->modal = nullptr;
  WLApplication::emergency_save(game);
  d->game = nullptr;
  disconnect("CLIENT_CRASHED");
  // We will bounce back to the main menu, so we better log out
  if (internet_) {
   InternetGaming::ref().logout("CLIENT_CRASHED");
  }

Could become

  WLApplication::emergency_save(game);
  d->game = nullptr;
  disconnect("CLIENT_CRASHED");
  // We will bounce back to the main menu, so we better log out
  if (internet_) {
   InternetGaming::ref().logout("CLIENT_CRASHED");
  }
  d->modal = nullptr;

[Klaus Halfmann (klaus-halfmann)]

This works for me, I dont think it will break anything, well.

=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc	2018-07-08 13:53:45 +0000
+++ src/ui_basic/window.cc	2018-09-29 14:33:07 +0000
@@ -221,13 +221,17 @@
 }
 
 /**
- * Move the window so that it is centered wrt the parent.
-*/
+ * Move the window so that it is centered inside the parent.
+ *
+ * Do nothing if window has no parent.
+ */
 void Window::center_to_parent() {
-	Panel& parent = *get_parent();
+	Panel* parent = get_parent();
 
-	set_pos(Vector2i((static_cast<int32_t>(parent.get_inner_w()) - get_w()) / 2,
-	                 (static_cast<int32_t>(parent.get_inner_h()) - get_h()) / 2));
+    if (parent) {
+        set_pos(Vector2i((static_cast<int32_t>(parent->get_inner_w()) - get_w()) / 2,
+                         (static_cast<int32_t>(parent->get_inner_h()) - get_h()) / 2));
+    }
 }
 
 /**