wesnothd_connection: while waiting for things, spin the loading screen.

This will not work from any thread other than the main thread. In these
cases the spin function will simply do nothing.

This could also be done outside wesnothd_connection. Calls would have
to be wrapped. One such call was previously being wrapped. However this
was only one call, and has thus been unwrapped. If it is desired that
a wesnothd_connection never touches the loading screen, then outside
wrappers need to be added to its blocking calls that can call
loading_screen::spin().

src/events.cpp

@@ -470,9 +470,16 @@ static bool remove_on_resize(const SDL_Event& a)
 // TODO: I'm uncertain if this is always safe to call at static init; maybe set in main() instead?
 static const std::thread::id main_thread = std::this_thread::get_id();
 
+// this should probably be elsewhere, but as the main thread is already
+// being tracked here, this went here.
+bool is_in_main_thread()
+{
+	return std::this_thread::get_id() == main_thread;
+}
+
 void pump()
 {
-	if(std::this_thread::get_id() != main_thread) {
+	if(!is_in_main_thread()) {
 		// Can only call this on the main thread!
 		return;
 	}
@@ -868,7 +875,7 @@ void peek_for_resize()
 
 void call_in_main_thread(const std::function<void(void)>& f)
 {
-	if(std::this_thread::get_id() == main_thread) {
+	if(is_in_main_thread()) {
 		// nothing special to do if called from the main thread.
 		f();
 		return;

src/events.hpp

@@ -126,6 +126,9 @@ void focus_handler(const sdl_handler* ptr);
 
 bool has_focus(const sdl_handler* ptr, const SDL_Event* event);
 
+// whether the currently executing thread is the main thread.
+bool is_in_main_thread();
+
 void call_in_main_thread(const std::function<void (void)>& f);
 
 //event_context objects control the handler objects that SDL events are sent

src/game_initialization/multiplayer.cpp

@@ -186,47 +186,31 @@ mp_manager::mp_manager(const std::optional<std::string> host)
 
 			gui2::dialogs::loading_screen::progress(loading_stage::download_lobby_data);
 
-			std::promise<void> received_initial_gamelist;
+			config data;
 
-			network_worker = std::thread([this, &received_initial_gamelist]() {
-				config data;
+			while(!stop) {
+				connection->wait_and_receive_data(data);
 
-				while(!stop) {
-					connection->wait_and_receive_data(data);
+				if(const auto error = data.optional_child("error")) {
+					throw wesnothd_error((*error)["message"]);
+				}
 
-					if(const auto error = data.optional_child("error")) {
-						throw wesnothd_error((*error)["message"]);
-					}
+				else if(data.has_child("gamelist")) {
+					this->lobby_info.process_gamelist(data);
+					break;
+				}
 
-					else if(data.has_child("gamelist")) {
-						this->lobby_info.process_gamelist(data);
+				else if(const auto gamelist_diff = data.optional_child("gamelist_diff")) {
+					this->lobby_info.process_gamelist_diff(*gamelist_diff);
+				}
 
-						try {
-							received_initial_gamelist.set_value();
-							// TODO: only here while we transition away from dialog-bound timer-based handling
-							return;
-						} catch(const std::future_error& e) {
-							if(e.code() == std::future_errc::promise_already_satisfied) {
-								// We only need this for the first gamelist
-							}
-						}
-					}
-
-					else if(const auto gamelist_diff = data.optional_child("gamelist_diff")) {
-						this->lobby_info.process_gamelist_diff(*gamelist_diff);
-					}
-
-					else {
-						// No special actions to take. Pass the data on to the network handlers.
-						for(const auto& handler : process_handlers) {
-							handler(data);
-						}
+				else {
+					// No special actions to take. Pass the data on to the network handlers.
+					for(const auto& handler : process_handlers) {
+						handler(data);
 					}
 				}
-			});
-
-			// Wait at the loading screen until the initial gamelist has been processed
-			received_initial_gamelist.get_future().wait();
+			}
 		});
 	}
 

src/gui/dialogs/loading_screen.cpp

@@ -132,6 +132,11 @@ void loading_screen::spin()
 		return;
 	}
 
+	// If we're not the main thread, do nothing.
+	if (!events::is_in_main_thread()) {
+		return;
+	}
+
 	// Restrict actual update rate.
 	int elapsed = SDL_GetTicks() - last_spin_;
 	if (elapsed > 10 || elapsed < 0) {

src/wesnothd_connection.cpp

@@ -18,6 +18,7 @@
 #include "wesnothd_connection.hpp"
 
 #include "gettext.hpp"
+#include "gui/dialogs/loading_screen.hpp"
 #include "log.hpp"
 #include "serialization/parser.hpp"
 #include "tls_root_store.hpp"
@@ -57,6 +58,8 @@ struct mptest_log
 using boost::system::error_code;
 using boost::system::system_error;
 
+using namespace std::chrono_literals; // s, ms, etc
+
 // main thread
 wesnothd_connection::wesnothd_connection(const std::string& host, const std::string& service)
 	: worker_thread_()
@@ -274,9 +277,18 @@ void wesnothd_connection::wait_for_handshake()
 
 	try {
 		// TODO: make this duration customizable. Should default to 1 minute.
-		const std::chrono::seconds timeout { 60 };
+		auto timeout = 60s;
 
-		switch(auto future = handshake_finished_.get_future(); future.wait_for(timeout)) {
+		auto future = handshake_finished_.get_future();
+		for(auto time = 0ms;
+			future.wait_for(10ms) == std::future_status::timeout
+				&& time < timeout;
+			time += 10ms)
+		{
+			gui2::dialogs::loading_screen::spin();
+		}
+
+		switch(future.wait_for(0ms)) {
 		case std::future_status::ready:
 			// This is a void future, so this just serves to re-throw any system_error exceptions
 			// stored by the worker thread. Additional handling occurs in the catch block below.
@@ -542,7 +554,11 @@ bool wesnothd_connection::wait_and_receive_data(config& data)
 {
 	{
 		std::unique_lock<std::mutex> lock(recv_queue_mutex_);
-		recv_queue_lock_.wait(lock, [this]() { return has_data_received(); });
+		while(!recv_queue_lock_.wait_for(
+		      lock, 10ms, [this]() { return has_data_received(); }))
+		{
+			gui2::dialogs::loading_screen::spin();
+		}
 	}
 
 	return receive_data(data);