Merge pull request #10685 from Pokechu22/netplay-sync-identifier-comparison

Netplay: Make messages about non-matching games clearer

Source/Core/Core/NetPlayClient.cpp

@@ -1587,49 +1587,6 @@ void NetPlayClient::ThreadFunc()
   return;
 }
 
-// called from ---GUI--- thread
-void NetPlayClient::GetPlayerList(std::string& list, std::vector<int>& pid_list)
-{
-  std::lock_guard lkp(m_crit.players);
-
-  std::ostringstream ss;
-
-  for (const auto& entry : m_players)
-  {
-    const Player& player = entry.second;
-    ss << player.name << "[" << static_cast<int>(player.pid) << "] : " << player.revision << " | "
-       << GetPlayerMappingString(player.pid, m_pad_map, m_gba_config, m_wiimote_map) << " |\n";
-
-    ss << "Ping: " << player.ping << "ms\n";
-    ss << "Status: ";
-
-    switch (player.game_status)
-    {
-    case SyncIdentifierComparison::SameGame:
-      ss << "ready";
-      break;
-
-    case SyncIdentifierComparison::DifferentVersion:
-      ss << "wrong game version";
-      break;
-
-    case SyncIdentifierComparison::DifferentGame:
-      ss << "game missing";
-      break;
-
-    default:
-      ss << "unknown";
-      break;
-    }
-
-    ss << "\n\n";
-
-    pid_list.push_back(player.pid);
-  }
-
-  list = ss.str();
-}
-
 // called from ---GUI--- thread
 std::vector<const Player*> NetPlayClient::GetPlayers()
 {

Source/Core/Core/NetPlayClient.h

@@ -111,7 +111,6 @@ public:
                 const std::string& name, const NetTraversalConfig& traversal_config);
   ~NetPlayClient();
 
-  void GetPlayerList(std::string& list, std::vector<int>& pid_list);
   std::vector<const Player*> GetPlayers();
   const NetSettings& GetNetSettings() const;
 

Source/Core/Core/SyncIdentifier.h

@@ -34,10 +34,15 @@ struct SyncIdentifier
   bool operator!=(const SyncIdentifier& s) const { return !operator==(s); }
 };
 
+// The order of entries in this enum matters, as the lowest value is
+// treated as the "best" available option.
 enum class SyncIdentifierComparison
 {
   SameGame,
-  DifferentVersion,
+  DifferentHash,
+  DifferentDiscNumber,
+  DifferentRevision,
+  DifferentRegion,
   DifferentGame,
   Unknown,
 };

Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp

@@ -611,10 +611,21 @@ void NetPlayDialog::UpdateGUI()
       {tr("Player"), tr("Game Status"), tr("Ping"), tr("Mapping"), tr("Revision")});
   m_players_list->setRowCount(m_player_count);
 
-  static const std::map<NetPlay::SyncIdentifierComparison, QString> player_status{
-      {NetPlay::SyncIdentifierComparison::SameGame, tr("OK")},
-      {NetPlay::SyncIdentifierComparison::DifferentVersion, tr("Wrong Version")},
-      {NetPlay::SyncIdentifierComparison::DifferentGame, tr("Not Found")},
+  static const std::map<NetPlay::SyncIdentifierComparison, std::pair<QString, QString>>
+      player_status{
+          {NetPlay::SyncIdentifierComparison::SameGame, {tr("OK"), tr("OK")}},
+          {NetPlay::SyncIdentifierComparison::DifferentHash,
+           {tr("Wrong hash"),
+            tr("Game file has a different hash; right-click it, select Properties, switch to the "
+               "Verify tab, and select Verify Integrity to check the hash")}},
+          {NetPlay::SyncIdentifierComparison::DifferentDiscNumber,
+           {tr("Wrong disc number"), tr("Game has a different disc number")}},
+          {NetPlay::SyncIdentifierComparison::DifferentRevision,
+           {tr("Wrong revision"), tr("Game has a different revision")}},
+          {NetPlay::SyncIdentifierComparison::DifferentRegion,
+           {tr("Wrong region"), tr("Game region does not match")}},
+          {NetPlay::SyncIdentifierComparison::DifferentGame,
+           {tr("Not found"), tr("No matching game was found")}},
       };
 
   for (int i = 0; i < m_player_count; i++)
@@ -622,14 +633,20 @@ void NetPlayDialog::UpdateGUI()
     const auto* p = players[i];
 
     auto* name_item = new QTableWidgetItem(QString::fromStdString(p->name));
-    auto* status_item = new QTableWidgetItem(player_status.count(p->game_status) ?
+    name_item->setToolTip(name_item->text());
+    const auto& status_info = player_status.count(p->game_status) ?
                                   player_status.at(p->game_status) :
-                                                 QStringLiteral("?"));
+                                  std::make_pair(QStringLiteral("?"), QStringLiteral("?"));
+    auto* status_item = new QTableWidgetItem(status_info.first);
+    status_item->setToolTip(status_info.second);
     auto* ping_item = new QTableWidgetItem(QStringLiteral("%1 ms").arg(p->ping));
+    ping_item->setToolTip(ping_item->text());
     auto* mapping_item =
         new QTableWidgetItem(QString::fromStdString(NetPlay::GetPlayerMappingString(
             p->pid, client->GetPadMapping(), client->GetGBAConfig(), client->GetWiimoteMapping())));
+    mapping_item->setToolTip(mapping_item->text());
     auto* revision_item = new QTableWidgetItem(QString::fromStdString(p->revision));
+    revision_item->setToolTip(revision_item->text());
 
     for (auto* item : {name_item, status_item, ping_item, mapping_item, revision_item})
     {

Source/Core/UICommon/GameFile.cpp

@@ -720,31 +720,57 @@ GameFile::CompareSyncIdentifier(const NetPlay::SyncIdentifier& sync_identifier)
   if ((is_elf_or_dol ? m_file_size : 0) != sync_identifier.dol_elf_size)
     return NetPlay::SyncIdentifierComparison::DifferentGame;
 
-  const auto trim = [](const std::string& str, size_t n) {
-    return std::string_view(str.data(), std::min(n, str.size()));
-  };
-
-  if (trim(m_game_id, 3) != trim(sync_identifier.game_id, 3))
+  if (m_is_datel_disc != sync_identifier.is_datel)
     return NetPlay::SyncIdentifierComparison::DifferentGame;
 
-  if (m_disc_number != sync_identifier.disc_number || m_is_datel_disc != sync_identifier.is_datel)
+  if (m_game_id.size() != sync_identifier.game_id.size())
     return NetPlay::SyncIdentifierComparison::DifferentGame;
 
-  const NetPlay::SyncIdentifierComparison mismatch_result =
-      is_elf_or_dol || m_is_datel_disc ? NetPlay::SyncIdentifierComparison::DifferentGame :
-                                         NetPlay::SyncIdentifierComparison::DifferentVersion;
-
-  if (m_game_id != sync_identifier.game_id)
+  if (!is_elf_or_dol && !m_is_datel_disc && m_game_id.size() >= 4 && m_game_id.size() <= 6)
   {
-    const bool game_id_is_title_id = m_game_id.size() > 6 || sync_identifier.game_id.size() > 6;
-    return game_id_is_title_id ? NetPlay::SyncIdentifierComparison::DifferentGame : mismatch_result;
+    // This is a game ID, following specific rules which we can use to give clearer information to
+    // the user.
+
+    // If the first 3 characters don't match, then these are probably different games.
+    // (There are exceptions; in particular Japanese-region games sometimes use a different ID;
+    // for instance, GOYE69, GOYP69, and GGIJ13 are used by GoldenEye: Rogue Agent.)
+    if (std::string_view(&m_game_id[0], 3) != std::string_view(&sync_identifier.game_id[0], 3))
+      return NetPlay::SyncIdentifierComparison::DifferentGame;
+
+    // If the first 3 characters match but the region doesn't match, reject it as such.
+    if (m_game_id[3] != sync_identifier.game_id[3])
+      return NetPlay::SyncIdentifierComparison::DifferentRegion;
+
+    // If the maker code is present, and doesn't match between the two but the main ID does,
+    // these might be different revisions of the same game (e.g. a rerelease with a different
+    // publisher), which we should treat as a different revision.
+    if (std::string_view(&m_game_id[4]) != std::string_view(&sync_identifier.game_id[4]))
+      return NetPlay::SyncIdentifierComparison::DifferentRevision;
+  }
+  else
+  {
+    // Numeric title ID or another generated ID that does not follow the rules above
+    if (m_game_id != sync_identifier.game_id)
+      return NetPlay::SyncIdentifierComparison::DifferentGame;
   }
 
   if (m_revision != sync_identifier.revision)
-    return mismatch_result;
+    return NetPlay::SyncIdentifierComparison::DifferentRevision;
 
-  return GetSyncHash() == sync_identifier.sync_hash ? NetPlay::SyncIdentifierComparison::SameGame :
-                                                      mismatch_result;
+  if (m_disc_number != sync_identifier.disc_number)
+    return NetPlay::SyncIdentifierComparison::DifferentDiscNumber;
+
+  if (GetSyncHash() != sync_identifier.sync_hash)
+  {
+    // Most Datel titles re-use the same game ID (GNHE5d, with that lowercase D, or DTLX01).
+    // So if the hash differs, then it's probably a different game even if the game ID matches.
+    if (m_is_datel_disc)
+      return NetPlay::SyncIdentifierComparison::DifferentGame;
+    else
+      return NetPlay::SyncIdentifierComparison::DifferentHash;
+  }
+
+  return NetPlay::SyncIdentifierComparison::SameGame;
 }
 
 std::string GameFile::GetWiiFSPath() const