Merge pull request #7938 from Techjar/netplay-pending-start-stall

NetPlay: Fix hosting being stuck if player leaves during pending start

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]);
+    if (m_chunked_data_receive_queue.count(cid))
-    m_chunked_data_receive_queue.erase(m_chunked_data_receive_queue.find(cid));
+    {
-    m_dialog->HideChunkedProgressDialog();
+      OnData(m_chunked_data_receive_queue[cid]);
+      m_chunked_data_receive_queue.erase(cid);
+      m_dialog->HideChunkedProgressDialog();
 
-    sf::Packet complete_packet;
+      sf::Packet complete_packet;
-    complete_packet << static_cast<MessageId>(NP_MSG_CHUNKED_DATA_COMPLETE);
+      complete_packet << static_cast<MessageId>(NP_MSG_CHUNKED_DATA_COMPLETE);
-    complete_packet << cid;
+      complete_packet << cid;
-    Send(complete_packet, CHUNKED_DATA_CHANNEL);
+      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;
+      while (!packet.endOfPacket())
-      packet >> byte;
+      {
-      m_chunked_data_receive_queue[cid] << byte;
+        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<MessageId>(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,
+  case NP_MSG_CHUNKED_DATA_ABORT:
-                                 m_chunked_data_receive_queue[cid].getDataSize());
+  {
+    u32 cid;
+    packet >> cid;
 
-    sf::Packet progress_packet;
+    if (m_chunked_data_receive_queue.count(cid))
-    progress_packet << static_cast<MessageId>(NP_MSG_CHUNKED_DATA_PROGRESS);
+    {
-    progress_packet << cid;
+      m_chunked_data_receive_queue.erase(cid);
-    progress_packet << sf::Uint64{m_chunked_data_receive_queue[cid].getDataSize()};
+      m_dialog->HideChunkedProgressDialog();
-    Send(progress_packet, CHUNKED_DATA_CHANNEL);
+    }
   }
   break;
 

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;

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,

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
+  // game is currently running or game start is pending
-  if (m_is_running)
+  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;
@@ -1338,6 +1347,16 @@ bool NetPlayServer::StartGame()
   return true;
 }
 
+void NetPlayServer::AbortGameStart()
+{
+  if (m_start_pending)
+  {
+    m_dialog->OnGameStartAborted();
+    ChunkedDataAbort();
+    m_start_pending = false;
+  }
+}
+
 // called from ---GUI--- thread
 bool NetPlayServer::SyncSaveData()
 {
@@ -1954,10 +1973,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 +2023,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<double> 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<MessageId>(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 +2064,7 @@ void NetPlayServer::ChunkedDataThreadFunc()
         }
       } while (index < e.packet.getDataSize());
 
+      if (!m_abort_chunked_data)
       {
         sf::Packet pac;
         pac << static_cast<MessageId>(NP_MSG_CHUNKED_DATA_END);
@@ -2029,9 +2072,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 +2096,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

Source/Core/Core/NetPlayServer.h

@@ -52,6 +52,7 @@ public:
   bool DoAllPlayersHaveIPLDump() const;
   bool StartGame();
   bool RequestStartGame();
+  void AbortGameStart();
 
   PadMappingArray GetPadMapping() const;
   void SetPadMapping(const PadMappingArray& mappings);
@@ -137,8 +138,9 @@ private:
   std::vector<std::pair<std::string, std::string>> 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 +187,7 @@ private:
   std::thread m_chunked_data_thread;
   u32 m_next_chunked_data_id;
   std::unordered_map<u32, unsigned int> m_chunked_data_complete_count;
+  bool m_abort_chunked_data = false;
 
   ENetHost* m_server = nullptr;
   TraversalClient* m_traversal_client = nullptr;

Source/Core/DolphinQt/NetPlay/ChunkedProgressDialog.cpp

@@ -53,15 +53,18 @@ void ChunkedProgressDialog::CreateWidgets()
   m_main_layout = new QVBoxLayout;
   m_progress_box = new QGroupBox;
   m_progress_layout = new QVBoxLayout;
+  m_button_box = new QDialogButtonBox(QDialogButtonBox::NoButton);
 
   m_progress_box->setLayout(m_progress_layout);
 
   m_main_layout->addWidget(m_progress_box);
+  m_main_layout->addWidget(m_button_box);
   setLayout(m_main_layout);
 }
 
 void ChunkedProgressDialog::ConnectWidgets()
 {
+  connect(m_button_box, &QDialogButtonBox::rejected, this, &ChunkedProgressDialog::reject);
 }
 
 void ChunkedProgressDialog::show(const QString& title, const u64 data_size,
@@ -89,6 +92,21 @@ void ChunkedProgressDialog::show(const QString& title, const u64 data_size,
   if (!client)
     return;
 
+  if (Settings::Instance().GetNetPlayServer())
+  {
+    m_button_box->setStandardButtons(QDialogButtonBox::Cancel);
+    QPushButton* cancel_button = m_button_box->button(QDialogButtonBox::Cancel);
+    cancel_button->setAutoDefault(false);
+    cancel_button->setDefault(false);
+  }
+  else
+  {
+    m_button_box->setStandardButtons(QDialogButtonBox::Close);
+    QPushButton* close_button = m_button_box->button(QDialogButtonBox::Close);
+    close_button->setAutoDefault(false);
+    close_button->setDefault(false);
+  }
+
   for (const auto* player : client->GetPlayers())
   {
     if (std::find(players.begin(), players.end(), player->pid) == players.end())
@@ -121,3 +139,13 @@ void ChunkedProgressDialog::SetProgress(const int pid, const u64 progress)
                                          QString::fromStdString(StringFromFormat("%.2f", total))));
   m_progress_bars[pid]->setValue(prog);
 }
+
+void ChunkedProgressDialog::reject()
+{
+  auto server = Settings::Instance().GetNetPlayServer();
+
+  if (server)
+    server->AbortGameStart();
+
+  QDialog::reject();
+}

Source/Core/DolphinQt/NetPlay/ChunkedProgressDialog.h

@@ -12,6 +12,7 @@
 
 #include "Common/CommonTypes.h"
 
+class QDialogButtonBox;
 class QGroupBox;
 class QLabel;
 class QProgressBar;
@@ -27,6 +28,8 @@ public:
   void show(const QString& title, u64 data_size, const std::vector<int>& players);
   void SetProgress(int pid, u64 progress);
 
+  void reject() override;
+
 private:
   void CreateWidgets();
   void ConnectWidgets();
@@ -38,4 +41,5 @@ private:
   QGroupBox* m_progress_box;
   QVBoxLayout* m_progress_layout;
   QVBoxLayout* m_main_layout;
+  QDialogButtonBox* m_button_box;
 };

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); });
 }
@@ -1094,7 +1096,7 @@ void NetPlayDialog::ShowChunkedProgressDialog(const std::string& title, const u6
 {
   QueueOnObject(this, [this, title, data_size, players] {
     if (m_chunked_progress_dialog->isVisible())
-      m_chunked_progress_dialog->close();
+      m_chunked_progress_dialog->done(QDialog::Accepted);
 
     m_chunked_progress_dialog->show(QString::fromStdString(title), data_size, players);
   });
@@ -1102,7 +1104,7 @@ void NetPlayDialog::ShowChunkedProgressDialog(const std::string& title, const u6
 
 void NetPlayDialog::HideChunkedProgressDialog()
 {
-  QueueOnObject(this, [this] { m_chunked_progress_dialog->close(); });
+  QueueOnObject(this, [this] { m_chunked_progress_dialog->done(QDialog::Accepted); });
 }
 
 void NetPlayDialog::SetChunkedProgress(const int pid, const u64 progress)

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;