From ea9b0bff08dab1df92c9aca2da3e748942d85955 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 24 May 2022 14:43:46 -0700 Subject: [PATCH 1/2] NetPlay: Delete NetPlayClient::GetPlayerList It's been unused since DolphinWX was removed in 44b22c90df9c05d18a34667dc41e8d05af24683f. Prior to that, it was used in Source/Core/DolphinWX/NetPlay/NetWindow.cpp. But the new equivalent in Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp uses NetPlayClient::GetPlayers instead. Stringifying (or creating a table, as is done now) should be done by the UI in any case. --- Source/Core/Core/NetPlayClient.cpp | 43 ------------------------------ Source/Core/Core/NetPlayClient.h | 1 - 2 files changed, 44 deletions(-) diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 75f02fa7f8..20c05ce4cc 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/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& 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(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 NetPlayClient::GetPlayers() { diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index 684d1a2616..dc96ac388b 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/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& pid_list); std::vector GetPlayers(); const NetSettings& GetNetSettings() const; From 2341ff00ab3484360de03e5dc1c43dbe522f361f Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Fri, 20 May 2022 22:25:33 -0700 Subject: [PATCH 2/2] NetPlay: Make messages about non-matching games clearer --- Source/Core/Core/SyncIdentifier.h | 7 ++- .../Core/DolphinQt/NetPlay/NetPlayDialog.cpp | 33 ++++++++--- Source/Core/UICommon/GameFile.cpp | 58 ++++++++++++++----- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/Source/Core/Core/SyncIdentifier.h b/Source/Core/Core/SyncIdentifier.h index 7175f073c4..a144eb58e1 100644 --- a/Source/Core/Core/SyncIdentifier.h +++ b/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, }; diff --git a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp index 02f7497b82..4a297acea3 100644 --- a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp +++ b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp @@ -611,25 +611,42 @@ 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 player_status{ - {NetPlay::SyncIdentifierComparison::SameGame, tr("OK")}, - {NetPlay::SyncIdentifierComparison::DifferentVersion, tr("Wrong Version")}, - {NetPlay::SyncIdentifierComparison::DifferentGame, tr("Not Found")}, - }; + static const std::map> + 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++) { 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) ? - player_status.at(p->game_status) : - QStringLiteral("?")); + name_item->setToolTip(name_item->text()); + const auto& status_info = player_status.count(p->game_status) ? + player_status.at(p->game_status) : + 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}) { diff --git a/Source/Core/UICommon/GameFile.cpp b/Source/Core/UICommon/GameFile.cpp index d87fb9d168..96628af087 100644 --- a/Source/Core/UICommon/GameFile.cpp +++ b/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