From 78707040873aec639c0e3e1d7c4c310003765d67 Mon Sep 17 00:00:00 2001 From: Techjar Date: Thu, 28 Mar 2019 02:32:06 -0400 Subject: [PATCH] NetPlay: Fix hosting being stuck if player leaves during pending start The logic didn't account for the case where a player leaves, so the host would be left in a dangling state where the UI is disabled but the game won't start, requiring a full restart of Dolphin to fix. --- Source/Core/Core/NetPlayClient.cpp | 58 +++++++++++++------ Source/Core/Core/NetPlayClient.h | 2 +- Source/Core/Core/NetPlayProto.h | 1 + Source/Core/Core/NetPlayServer.cpp | 51 ++++++++++++++-- Source/Core/Core/NetPlayServer.h | 4 +- .../Core/DolphinQt/NetPlay/NetPlayDialog.cpp | 4 +- Source/Core/DolphinQt/NetPlay/NetPlayDialog.h | 2 +- 7 files changed, 95 insertions(+), 27 deletions(-) diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index a2e6dcc562..2ff12f3f22 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -85,6 +85,9 @@ NetPlayClient::~NetPlayClient() m_MD5_thread.join(); m_do_loop.Clear(); m_thread.join(); + + m_chunked_data_receive_queue.clear(); + m_dialog->HideChunkedProgressDialog(); } if (m_server) @@ -365,14 +368,17 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) u32 cid; packet >> cid; - OnData(m_chunked_data_receive_queue[cid]); - m_chunked_data_receive_queue.erase(m_chunked_data_receive_queue.find(cid)); - m_dialog->HideChunkedProgressDialog(); + if (m_chunked_data_receive_queue.count(cid)) + { + OnData(m_chunked_data_receive_queue[cid]); + m_chunked_data_receive_queue.erase(cid); + m_dialog->HideChunkedProgressDialog(); - sf::Packet complete_packet; - complete_packet << static_cast(NP_MSG_CHUNKED_DATA_COMPLETE); - complete_packet << cid; - Send(complete_packet, CHUNKED_DATA_CHANNEL); + sf::Packet complete_packet; + complete_packet << static_cast(NP_MSG_CHUNKED_DATA_COMPLETE); + complete_packet << cid; + Send(complete_packet, CHUNKED_DATA_CHANNEL); + } } break; @@ -381,21 +387,37 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) u32 cid; packet >> cid; - while (!packet.endOfPacket()) + if (m_chunked_data_receive_queue.count(cid)) { - u8 byte; - packet >> byte; - m_chunked_data_receive_queue[cid] << byte; + while (!packet.endOfPacket()) + { + u8 byte; + packet >> byte; + m_chunked_data_receive_queue[cid] << byte; + } + + m_dialog->SetChunkedProgress(m_local_player->pid, + m_chunked_data_receive_queue[cid].getDataSize()); + + sf::Packet progress_packet; + progress_packet << static_cast(NP_MSG_CHUNKED_DATA_PROGRESS); + progress_packet << cid; + progress_packet << sf::Uint64{m_chunked_data_receive_queue[cid].getDataSize()}; + Send(progress_packet, CHUNKED_DATA_CHANNEL); } + } + break; - m_dialog->SetChunkedProgress(m_local_player->pid, - m_chunked_data_receive_queue[cid].getDataSize()); + case NP_MSG_CHUNKED_DATA_ABORT: + { + u32 cid; + packet >> cid; - sf::Packet progress_packet; - progress_packet << static_cast(NP_MSG_CHUNKED_DATA_PROGRESS); - progress_packet << cid; - progress_packet << sf::Uint64{m_chunked_data_receive_queue[cid].getDataSize()}; - Send(progress_packet, CHUNKED_DATA_CHANNEL); + if (m_chunked_data_receive_queue.count(cid)) + { + m_chunked_data_receive_queue.erase(cid); + m_dialog->HideChunkedProgressDialog(); + } } break; diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index 9afa624dd4..3afaadd815 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -53,7 +53,7 @@ public: virtual void OnConnectionError(const std::string& message) = 0; virtual void OnTraversalError(TraversalClient::FailureReason error) = 0; virtual void OnTraversalStateChanged(TraversalClient::State state) = 0; - virtual void OnSaveDataSyncFailure() = 0; + virtual void OnGameStartAborted() = 0; virtual void OnGolferChanged(bool is_golfer, const std::string& golfer_name) = 0; virtual bool IsRecording() = 0; diff --git a/Source/Core/Core/NetPlayProto.h b/Source/Core/Core/NetPlayProto.h index a517cdd9ba..1e66e708e3 100644 --- a/Source/Core/Core/NetPlayProto.h +++ b/Source/Core/Core/NetPlayProto.h @@ -122,6 +122,7 @@ enum NP_MSG_CHUNKED_DATA_PAYLOAD = 0x42, NP_MSG_CHUNKED_DATA_PROGRESS = 0x43, NP_MSG_CHUNKED_DATA_COMPLETE = 0x44, + NP_MSG_CHUNKED_DATA_ABORT = 0x45, NP_MSG_PAD_DATA = 0x60, NP_MSG_PAD_MAPPING = 0x61, diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index abaf5446ba..685af6594a 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -353,8 +353,8 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket, sf::Packet& rpac) if (npver != Common::scm_rev_git_str) return CON_ERR_VERSION_MISMATCH; - // game is currently running - if (m_is_running) + // game is currently running or game start is pending + if (m_is_running || m_start_pending) return CON_ERR_GAME_RUNNING; // too many players @@ -483,6 +483,13 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player) } } + if (m_start_pending) + { + ChunkedDataAbort(); + m_dialog->OnGameStartAborted(); + m_start_pending = false; + } + sf::Packet spac; spac << (MessageId)NP_MSG_PLAYER_LEAVE; spac << pid; @@ -1046,7 +1053,8 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) { m_dialog->AppendChat( StringFromFormat(GetStringT("%s failed to synchronize.").c_str(), player.name.c_str())); - m_dialog->OnSaveDataSyncFailure(); + m_dialog->OnGameStartAborted(); + ChunkedDataAbort(); m_start_pending = false; } break; @@ -1089,6 +1097,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) { m_dialog->AppendChat(StringFromFormat(GetStringT("%s failed to synchronize codes.").c_str(), player.name.c_str())); + m_dialog->OnGameStartAborted(); m_start_pending = false; } break; @@ -1954,10 +1963,21 @@ void NetPlayServer::ChunkedDataThreadFunc() { m_chunked_data_event.Wait(); + if (m_abort_chunked_data) + { + // thread-safe clear + while (!m_chunked_data_queue.Empty()) + m_chunked_data_queue.Pop(); + + m_abort_chunked_data = false; + } + while (!m_chunked_data_queue.Empty()) { if (!m_do_loop) return; + if (m_abort_chunked_data) + break; auto& e = m_chunked_data_queue.Front(); const u32 id = m_next_chunked_data_id++; @@ -1993,15 +2013,27 @@ void NetPlayServer::ChunkedDataThreadFunc() const float bytes_per_second = (std::max(Config::Get(Config::NETPLAY_CHUNKED_UPLOAD_LIMIT), 1u) / 8.0f) * 1024.0f; const std::chrono::duration send_interval(CHUNKED_DATA_UNIT_SIZE / bytes_per_second); + bool skip_wait = false; size_t index = 0; do { if (!m_do_loop) return; + if (m_abort_chunked_data) + { + sf::Packet pac; + pac << static_cast(NP_MSG_CHUNKED_DATA_ABORT); + pac << id; + ChunkedDataSend(std::move(pac), e.target_pid, e.target_mode); + break; + } if (e.target_mode == TargetMode::Only) { if (m_players.find(e.target_pid) == m_players.end()) + { + skip_wait = true; break; + } } auto start = std::chrono::steady_clock::now(); @@ -2022,6 +2054,7 @@ void NetPlayServer::ChunkedDataThreadFunc() } } while (index < e.packet.getDataSize()); + if (!m_abort_chunked_data) { sf::Packet pac; pac << static_cast(NP_MSG_CHUNKED_DATA_END); @@ -2029,9 +2062,10 @@ void NetPlayServer::ChunkedDataThreadFunc() ChunkedDataSend(std::move(pac), e.target_pid, e.target_mode); } - while (m_chunked_data_complete_count[id] < player_count && m_do_loop) + while (m_chunked_data_complete_count[id] < player_count && m_do_loop && + !m_abort_chunked_data && !skip_wait) m_chunked_data_complete_event.Wait(); - m_chunked_data_complete_count.erase(m_chunked_data_complete_count.find(id)); + m_chunked_data_complete_count.erase(id); m_dialog->HideChunkedProgressDialog(); m_chunked_data_queue.Pop(); @@ -2052,4 +2086,11 @@ void NetPlayServer::ChunkedDataSend(sf::Packet&& packet, const PlayerId pid, SendAsyncToClients(std::move(packet), pid, CHUNKED_DATA_CHANNEL); } } + +void NetPlayServer::ChunkedDataAbort() +{ + m_abort_chunked_data = true; + m_chunked_data_event.Set(); + m_chunked_data_complete_event.Set(); +} } // namespace NetPlay diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index baec187324..0c749845e6 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -137,8 +137,9 @@ private: std::vector> GetInterfaceListInternal() const; void ChunkedDataThreadFunc(); void ChunkedDataSend(sf::Packet&& packet, PlayerId pid, const TargetMode target_mode); - void SetupIndex(); + void ChunkedDataAbort(); + void SetupIndex(); bool PlayerHasControllerMapped(PlayerId pid) const; NetSettings m_settings; @@ -185,6 +186,7 @@ private: std::thread m_chunked_data_thread; u32 m_next_chunked_data_id; std::unordered_map m_chunked_data_complete_count; + bool m_abort_chunked_data = false; ENetHost* m_server = nullptr; TraversalClient* m_traversal_client = nullptr; diff --git a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp index e9592ca1fb..814aafdf39 100644 --- a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp +++ b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp @@ -553,6 +553,8 @@ void NetPlayDialog::show(std::string nickname, bool use_traversal) m_game_button->setEnabled(is_hosting); m_kick_button->setEnabled(false); + SetOptionsEnabled(true); + QDialog::show(); UpdateGUI(); } @@ -973,7 +975,7 @@ void NetPlayDialog::OnTraversalStateChanged(TraversalClient::State state) } } -void NetPlayDialog::OnSaveDataSyncFailure() +void NetPlayDialog::OnGameStartAborted() { QueueOnObject(this, [this] { SetOptionsEnabled(true); }); } diff --git a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.h b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.h index 558990cb1d..e6d601afaa 100644 --- a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.h +++ b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.h @@ -57,7 +57,7 @@ public: void OnConnectionError(const std::string& message) override; void OnTraversalError(TraversalClient::FailureReason error) override; void OnTraversalStateChanged(TraversalClient::State state) override; - void OnSaveDataSyncFailure() override; + void OnGameStartAborted() override; void OnGolferChanged(bool is_golfer, const std::string& golfer_name) override; bool IsRecording() override;