From 85e94cc5107dfa8b3b7569a4c0d8e251d9d0f98b Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 3 Jun 2018 12:07:53 +0200 Subject: [PATCH] Don't store custom names from TitleDatabase in GameFileCache This saves us from having to update the GameFileCache when the TitleDatabase changes (for instance when the user changes language). --- Source/Core/Core/TitleDatabase.cpp | 10 +-- Source/Core/Core/TitleDatabase.h | 4 +- .../DolphinQt2/GameList/GameListModel.cpp | 6 +- .../Core/DolphinQt2/GameList/GameListModel.h | 3 + .../Core/DolphinQt2/GameList/GameTracker.cpp | 4 +- Source/Core/DolphinQt2/GameList/GameTracker.h | 2 - Source/Core/DolphinWX/GameListCtrl.cpp | 67 ++++++++++--------- Source/Core/DolphinWX/GameListCtrl.h | 4 +- Source/Core/UICommon/GameFile.cpp | 31 +++------ Source/Core/UICommon/GameFile.h | 6 +- Source/Core/UICommon/GameFileCache.cpp | 20 ++---- Source/Core/UICommon/GameFileCache.h | 12 +--- 12 files changed, 75 insertions(+), 94 deletions(-) diff --git a/Source/Core/Core/TitleDatabase.cpp b/Source/Core/Core/TitleDatabase.cpp index 88113854f8..32871eba30 100644 --- a/Source/Core/Core/TitleDatabase.cpp +++ b/Source/Core/Core/TitleDatabase.cpp @@ -19,6 +19,8 @@ namespace Core { +static const std::string EMPTY_STRING; + static std::string GetLanguageCode(DiscIO::Language language) { switch (language) @@ -157,16 +159,16 @@ TitleDatabase::TitleDatabase() TitleDatabase::~TitleDatabase() = default; -std::string TitleDatabase::GetTitleName(const std::string& game_id, TitleType type) const +const std::string& TitleDatabase::GetTitleName(const std::string& game_id, TitleType type) const { const auto& map = IsWiiTitle(game_id) ? m_wii_title_map : m_gc_title_map; const std::string key = type == TitleType::Channel && game_id.length() == 6 ? game_id.substr(0, 4) : game_id; const auto iterator = map.find(key); - return iterator != map.end() ? iterator->second : ""; + return iterator != map.end() ? iterator->second : EMPTY_STRING; } -std::string TitleDatabase::GetChannelName(u64 title_id) const +const std::string& TitleDatabase::GetChannelName(u64 title_id) const { const std::string id{ {static_cast((title_id >> 24) & 0xff), static_cast((title_id >> 16) & 0xff), @@ -176,7 +178,7 @@ std::string TitleDatabase::GetChannelName(u64 title_id) const std::string TitleDatabase::Describe(const std::string& game_id, TitleType type) const { - const std::string title_name = GetTitleName(game_id, type); + const std::string& title_name = GetTitleName(game_id, type); if (title_name.empty()) return game_id; return StringFromFormat("%s (%s)", title_name.c_str(), game_id.c_str()); diff --git a/Source/Core/Core/TitleDatabase.h b/Source/Core/Core/TitleDatabase.h index abc2cd51f1..0eb6cd1d9e 100644 --- a/Source/Core/Core/TitleDatabase.h +++ b/Source/Core/Core/TitleDatabase.h @@ -26,10 +26,10 @@ public: // Get a user friendly title name for a game ID. // This falls back to returning an empty string if none could be found. - std::string GetTitleName(const std::string& game_id, TitleType = TitleType::Other) const; + const std::string& GetTitleName(const std::string& game_id, TitleType = TitleType::Other) const; // Same as above, but takes a title ID instead of a game ID, and can only find names of channels. - std::string GetChannelName(u64 title_id) const; + const std::string& GetChannelName(u64 title_id) const; // Get a description for a game ID (title name if available + game ID). std::string Describe(const std::string& game_id, TitleType = TitleType::Other) const; diff --git a/Source/Core/DolphinQt2/GameList/GameListModel.cpp b/Source/Core/DolphinQt2/GameList/GameListModel.cpp index 795290f57d..412bce53fe 100644 --- a/Source/Core/DolphinQt2/GameList/GameListModel.cpp +++ b/Source/Core/DolphinQt2/GameList/GameListModel.cpp @@ -80,7 +80,7 @@ QVariant GameListModel::data(const QModelIndex& index, int role) const case COL_TITLE: if (role == Qt::DisplayRole || role == Qt::InitialSortOrderRole) { - QString name = QString::fromStdString(game.GetName()); + QString name = QString::fromStdString(game.GetName(m_title_database)); const int disc_nr = game.GetDiscNumber() + 1; if (disc_nr > 1) { @@ -161,8 +161,10 @@ bool GameListModel::ShouldDisplayGameListItem(int index) const const UICommon::GameFile& game = *m_games[index]; if (!m_term.isEmpty() && - !QString::fromStdString(game.GetName()).contains(m_term, Qt::CaseInsensitive)) + !QString::fromStdString(game.GetName(m_title_database)).contains(m_term, Qt::CaseInsensitive)) + { return false; + } const bool show_platform = [&game] { switch (game.GetPlatform()) diff --git a/Source/Core/DolphinQt2/GameList/GameListModel.h b/Source/Core/DolphinQt2/GameList/GameListModel.h index 137cdc46ad..daf5058978 100644 --- a/Source/Core/DolphinQt2/GameList/GameListModel.h +++ b/Source/Core/DolphinQt2/GameList/GameListModel.h @@ -10,6 +10,8 @@ #include #include +#include "Core/TitleDatabase.h" + #include "DolphinQt2/GameList/GameTracker.h" #include "UICommon/GameFile.h" @@ -63,5 +65,6 @@ private: GameTracker m_tracker; QList> m_games; + Core::TitleDatabase m_title_database; QString m_term; }; diff --git a/Source/Core/DolphinQt2/GameList/GameTracker.cpp b/Source/Core/DolphinQt2/GameList/GameTracker.cpp index 0072317072..b585fe88fd 100644 --- a/Source/Core/DolphinQt2/GameList/GameTracker.cpp +++ b/Source/Core/DolphinQt2/GameList/GameTracker.cpp @@ -105,7 +105,7 @@ void GameTracker::StartInternal() m_initial_games_emitted_event.Wait(); bool cache_updated = m_cache.Update(paths, emit_game_loaded, emit_game_removed); - cache_updated |= m_cache.UpdateAdditionalMetadata(m_title_database, emit_game_updated); + cache_updated |= m_cache.UpdateAdditionalMetadata(emit_game_updated); if (cache_updated) m_cache.Save(); } @@ -256,7 +256,7 @@ void GameTracker::LoadGame(const QString& path) if (!DiscIO::ShouldHideFromGameList(converted_path)) { bool cache_changed = false; - auto game = m_cache.AddOrGet(converted_path, &cache_changed, m_title_database); + auto game = m_cache.AddOrGet(converted_path, &cache_changed); if (game) emit GameLoaded(std::move(game)); if (cache_changed) diff --git a/Source/Core/DolphinQt2/GameList/GameTracker.h b/Source/Core/DolphinQt2/GameList/GameTracker.h index 4f5c87bf4b..0504e06bab 100644 --- a/Source/Core/DolphinQt2/GameList/GameTracker.h +++ b/Source/Core/DolphinQt2/GameList/GameTracker.h @@ -14,7 +14,6 @@ #include "Common/Event.h" #include "Common/WorkQueueThread.h" -#include "Core/TitleDatabase.h" #include "UICommon/GameFile.h" #include "UICommon/GameFileCache.h" @@ -75,7 +74,6 @@ private: QMap> m_tracked_files; Common::WorkQueueThread m_load_thread; UICommon::GameFileCache m_cache; - Core::TitleDatabase m_title_database; Common::Event m_cache_loaded_event; Common::Event m_initial_games_emitted_event; bool m_initial_games_emitted = false; diff --git a/Source/Core/DolphinWX/GameListCtrl.cpp b/Source/Core/DolphinWX/GameListCtrl.cpp index 1a70fe5b28..9e9ee59d11 100644 --- a/Source/Core/DolphinWX/GameListCtrl.cpp +++ b/Source/Core/DolphinWX/GameListCtrl.cpp @@ -82,9 +82,18 @@ public: static bool sorted = false; -static int CompareGameListItems(const UICommon::GameFile* iso1, const UICommon::GameFile* iso2, - long sortData = GameListCtrl::COLUMN_TITLE) +static int CompareGameListItems(size_t item1, size_t item2, long sortData, + const GameListCtrl* caller) { + // return 1 if item1 > item2 + // return -1 if item1 < item2 + // return 0 for identity + const UICommon::GameFile* iso1 = caller->GetISO(item1); + const UICommon::GameFile* iso2 = caller->GetISO(item2); + + if (iso1 == iso2) + return 0; + int t = 1; if (sortData < 0) @@ -136,7 +145,8 @@ static int CompareGameListItems(const UICommon::GameFile* iso1, const UICommon:: if (sortData != GameListCtrl::COLUMN_TITLE) t = 1; - int name_cmp = strcasecmp(iso1->GetName().c_str(), iso2->GetName().c_str()) * t; + int name_cmp = + strcasecmp(caller->GetShownName(item1).c_str(), caller->GetShownName(item2).c_str()) * t; if (name_cmp != 0) return name_cmp; @@ -383,28 +393,34 @@ void GameListCtrl::RefreshList() if (Core::GetState() != Core::State::Uninitialized) return; + // Use a newly loaded title database (it might have gotten updated) + const Core::TitleDatabase title_database; + + m_shown_names.clear(); m_shown_files.clear(); { std::unique_lock lk(m_cache_mutex); - m_cache.ForEach([this](const std::shared_ptr& game_file) { - if (ShouldDisplayGameListItem(*game_file)) - m_shown_files.push_back(game_file); - }); + m_cache.ForEach( + [this, &title_database](const std::shared_ptr& game_file) { + if (ShouldDisplayGameListItem(*game_file)) + { + m_shown_names.push_back(game_file->GetName(title_database)); + m_shown_files.push_back(game_file); + } + }); } // Drives are not cached. Not sure if this is required, but better to err on the // side of caution if cross-platform issues could come into play. if (SConfig::GetInstance().m_ListDrives) { - std::unique_lock lk(m_title_database_mutex); for (const auto& drive : Common::GetCDDevices()) { auto file = std::make_shared(drive); if (file->IsValid()) { - if (file->CustomNameChanged(m_title_database)) - file->CustomNameCommit(); - m_shown_files.push_back(file); + m_shown_names.push_back(file->GetName(title_database)); + m_shown_files.push_back(std::move(file)); } } } @@ -501,7 +517,8 @@ void GameListCtrl::RefreshList() // Update the column content of the item at index void GameListCtrl::UpdateItemAtColumn(long index, int column) { - const auto& iso_file = *GetISO(GetItemData(index)); + const size_t item_data = GetItemData(index); + const auto& iso_file = *GetISO(item_data); switch (column) { @@ -527,7 +544,7 @@ void GameListCtrl::UpdateItemAtColumn(long index, int column) } case COLUMN_TITLE: { - wxString name = StrToWxStr(iso_file.GetName()); + wxString name = StrToWxStr(GetShownName(item_data)); int disc_number = iso_file.GetDiscNumber() + 1; if (disc_number > 1 && @@ -628,12 +645,6 @@ void GameListCtrl::RescanList() const std::vector game_paths = UICommon::FindAllGamePaths( SConfig::GetInstance().m_ISOFolder, SConfig::GetInstance().m_RecursiveISOFolder); - // Reload the TitleDatabase - { - std::unique_lock lock(m_title_database_mutex); - m_title_database = {}; - } - bool cache_changed = false; { @@ -647,7 +658,7 @@ void GameListCtrl::RescanList() { std::unique_lock lk(m_cache_mutex); - if (m_cache.UpdateAdditionalMetadata(m_title_database)) + if (m_cache.UpdateAdditionalMetadata()) { cache_changed = true; QueueEvent(new wxCommandEvent(DOLPHIN_EVT_REFRESH_GAMELIST)); @@ -701,19 +712,15 @@ const UICommon::GameFile* GameListCtrl::GetISO(size_t index) const return nullptr; } +const std::string& GameListCtrl::GetShownName(size_t index) const +{ + return m_shown_names[index]; +} + static GameListCtrl* caller; static int wxCALLBACK wxListCompare(wxIntPtr item1, wxIntPtr item2, wxIntPtr sortData) { - // return 1 if item1 > item2 - // return -1 if item1 < item2 - // return 0 for identity - const UICommon::GameFile* iso1 = caller->GetISO(item1); - const UICommon::GameFile* iso2 = caller->GetISO(item2); - - if (iso1 == iso2) - return 0; - - return CompareGameListItems(iso1, iso2, sortData); + return CompareGameListItems(item1, item2, sortData, caller); } void GameListCtrl::OnColumnClick(wxListEvent& event) diff --git a/Source/Core/DolphinWX/GameListCtrl.h b/Source/Core/DolphinWX/GameListCtrl.h index f81ded6846..25df4c094d 100644 --- a/Source/Core/DolphinWX/GameListCtrl.h +++ b/Source/Core/DolphinWX/GameListCtrl.h @@ -35,6 +35,7 @@ public: void BrowseForDirectory(); const UICommon::GameFile* GetISO(size_t index) const; + const std::string& GetShownName(size_t index) const; const UICommon::GameFile* GetSelectedISO() const; static bool IsHidingItems(); @@ -111,13 +112,12 @@ private: UICommon::GameFileCache m_cache; // Locks the cache object, not the shared_ptrs obtained from it std::mutex m_cache_mutex; - Core::TitleDatabase m_title_database; - std::mutex m_title_database_mutex; std::thread m_scan_thread; Common::Event m_scan_trigger; Common::Flag m_scan_exiting; // UI thread's view into the cache std::vector> m_shown_files; + std::vector m_shown_names; int m_last_column; int m_last_sort; diff --git a/Source/Core/UICommon/GameFile.cpp b/Source/Core/UICommon/GameFile.cpp index 90f87bb016..64d4740a0e 100644 --- a/Source/Core/UICommon/GameFile.cpp +++ b/Source/Core/UICommon/GameFile.cpp @@ -141,20 +141,6 @@ bool GameFile::IsValid() const return true; } -bool GameFile::CustomNameChanged(const Core::TitleDatabase& title_database) -{ - const auto type = m_platform == DiscIO::Platform::WiiWAD ? - Core::TitleDatabase::TitleType::Channel : - Core::TitleDatabase::TitleType::Other; - m_pending.custom_name = title_database.GetTitleName(m_game_id, type); - return m_custom_name != m_pending.custom_name; -} - -void GameFile::CustomNameCommit() -{ - m_custom_name = std::move(m_pending.custom_name); -} - void GameBanner::DoState(PointerWrap& p) { p.Do(buffer); @@ -191,7 +177,6 @@ void GameFile::DoState(PointerWrap& p) m_volume_banner.DoState(p); m_custom_banner.DoState(p); - p.Do(m_custom_name); } bool GameFile::IsElfOrDol() const @@ -280,11 +265,17 @@ void GameFile::CustomBannerCommit() m_custom_banner = std::move(m_pending.custom_banner); } +const std::string& GameFile::GetName(const Core::TitleDatabase& title_database) const +{ + const auto type = m_platform == DiscIO::Platform::WiiWAD ? + Core::TitleDatabase::TitleType::Channel : + Core::TitleDatabase::TitleType::Other; + const std::string& custom_name = title_database.GetTitleName(m_game_id, type); + return custom_name.empty() ? GetName() : custom_name; +} + const std::string& GameFile::GetName(bool long_name) const { - if (!m_custom_name.empty()) - return m_custom_name; - const std::string& name = long_name ? GetLongName() : GetShortName(); if (!name.empty()) return name; @@ -323,9 +314,7 @@ std::string GameFile::GetUniqueIdentifier() const if (GetRevision() != 0) info.push_back("Revision " + std::to_string(GetRevision())); - std::string name(GetLongName(lang)); - if (name.empty()) - name = GetName(); + const std::string& name = GetName(); int disc_number = GetDiscNumber() + 1; diff --git a/Source/Core/UICommon/GameFile.h b/Source/Core/UICommon/GameFile.h index 51a5e1b84e..60d874a375 100644 --- a/Source/Core/UICommon/GameFile.h +++ b/Source/Core/UICommon/GameFile.h @@ -44,6 +44,7 @@ public: bool IsValid() const; const std::string& GetFilePath() const { return m_file_path; } const std::string& GetFileName() const { return m_file_name; } + const std::string& GetName(const Core::TitleDatabase& title_database) const; const std::string& GetName(bool long_name = true) const; const std::string& GetMaker(bool long_maker = true) const; const std::string& GetShortName(DiscIO::Language l) const { return Lookup(l, m_short_names); } @@ -79,8 +80,6 @@ public: void WiiBannerCommit(); bool CustomBannerChanged(); void CustomBannerCommit(); - bool CustomNameChanged(const Core::TitleDatabase& title_database); - void CustomNameCommit(); private: static const std::string& Lookup(DiscIO::Language language, @@ -121,8 +120,6 @@ private: GameBanner m_volume_banner{}; GameBanner m_custom_banner{}; - // Overridden name from TitleDatabase - std::string m_custom_name{}; // The following data members allow GameFileCache to construct updated versions // of GameFiles in a threadsafe way. They should not be handled in DoState. @@ -130,7 +127,6 @@ private: { GameBanner volume_banner; GameBanner custom_banner; - std::string custom_name; } m_pending{}; }; diff --git a/Source/Core/UICommon/GameFileCache.cpp b/Source/Core/UICommon/GameFileCache.cpp index e2e2848f10..7a71ca75f1 100644 --- a/Source/Core/UICommon/GameFileCache.cpp +++ b/Source/Core/UICommon/GameFileCache.cpp @@ -20,15 +20,13 @@ #include "Common/FileSearch.h" #include "Common/FileUtil.h" -#include "Core/TitleDatabase.h" - #include "DiscIO/DirectoryBlob.h" #include "UICommon/GameFile.h" namespace UICommon { -static constexpr u32 CACHE_REVISION = 10; // Last changed in PR 6429 +static constexpr u32 CACHE_REVISION = 11; // Last changed in PR 7058 std::vector FindAllGamePaths(const std::vector& directories_to_scan, bool recursive_scan) @@ -52,8 +50,7 @@ void GameFileCache::Clear() } std::shared_ptr GameFileCache::AddOrGet(const std::string& path, - bool* cache_changed, - const Core::TitleDatabase& title_database) + bool* cache_changed) { auto it = std::find_if( m_cached_files.begin(), m_cached_files.end(), @@ -67,7 +64,7 @@ std::shared_ptr GameFileCache::AddOrGet(const std::string& path, m_cached_files.emplace_back(std::move(game)); } std::shared_ptr& result = found ? *it : m_cached_files.back(); - if (UpdateAdditionalMetadata(&result, title_database) || !found) + if (UpdateAdditionalMetadata(&result) || !found) *cache_changed = true; return result; @@ -135,14 +132,13 @@ bool GameFileCache::Update( } bool GameFileCache::UpdateAdditionalMetadata( - const Core::TitleDatabase& title_database, std::function&)> game_updated) { bool cache_changed = false; for (std::shared_ptr& file : m_cached_files) { - const bool updated = UpdateAdditionalMetadata(&file, title_database); + const bool updated = UpdateAdditionalMetadata(&file); cache_changed |= updated; if (game_updated && updated) game_updated(file); @@ -151,13 +147,11 @@ bool GameFileCache::UpdateAdditionalMetadata( return cache_changed; } -bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr* game_file, - const Core::TitleDatabase& title_database) +bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr* game_file) { const bool wii_banner_changed = (*game_file)->WiiBannerChanged(); const bool custom_banner_changed = (*game_file)->CustomBannerChanged(); - const bool custom_title_changed = (*game_file)->CustomNameChanged(title_database); - if (!wii_banner_changed && !custom_banner_changed && !custom_title_changed) + if (!wii_banner_changed && !custom_banner_changed) return false; // If a cached file needs an update, apply the updates to a copy and delete the original. @@ -168,8 +162,6 @@ bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr* game_fil copy->WiiBannerCommit(); if (custom_banner_changed) copy->CustomBannerCommit(); - if (custom_title_changed) - copy->CustomNameCommit(); *game_file = std::move(copy); return true; diff --git a/Source/Core/UICommon/GameFileCache.h b/Source/Core/UICommon/GameFileCache.h index 3a23f5211b..6e36838dd0 100644 --- a/Source/Core/UICommon/GameFileCache.h +++ b/Source/Core/UICommon/GameFileCache.h @@ -16,11 +16,6 @@ class PointerWrap; -namespace Core -{ -class TitleDatabase; -} - namespace UICommon { class GameFile; @@ -36,23 +31,20 @@ public: void Clear(); // Returns nullptr if the file is invalid. - std::shared_ptr AddOrGet(const std::string& path, bool* cache_changed, - const Core::TitleDatabase& title_database); + std::shared_ptr AddOrGet(const std::string& path, bool* cache_changed); // These functions return true if the call modified the cache. bool Update(const std::vector& all_game_paths, std::function&)> game_added_to_cache = {}, std::function game_removed_from_cache = {}); bool UpdateAdditionalMetadata( - const Core::TitleDatabase& title_database, std::function&)> game_updated = {}); bool Load(); bool Save(); private: - bool UpdateAdditionalMetadata(std::shared_ptr* game_file, - const Core::TitleDatabase& title_database); + bool UpdateAdditionalMetadata(std::shared_ptr* game_file); bool SyncCacheFile(bool save); void DoState(PointerWrap* p, u64 size = 0);