Merge pull request #7058 from JosJuice/move-titledatabase-usage

Don't store custom names from TitleDatabase in GameFileCache

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<char>((title_id >> 24) & 0xff), static_cast<char>((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());

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;

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())

Source/Core/DolphinQt2/GameList/GameListModel.h

@@ -10,6 +10,8 @@
 #include <QAbstractTableModel>
 #include <QString>
 
+#include "Core/TitleDatabase.h"
+
 #include "DolphinQt2/GameList/GameTracker.h"
 
 #include "UICommon/GameFile.h"
@@ -63,5 +65,6 @@ private:
 
   GameTracker m_tracker;
   QList<std::shared_ptr<const UICommon::GameFile>> m_games;
+  Core::TitleDatabase m_title_database;
   QString m_term;
 };

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)

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<QString, QSet<QString>> m_tracked_files;
   Common::WorkQueueThread<Command> 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;

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,
+static int CompareGameListItems(size_t item1, size_t item2, long sortData,
-                                long sortData = GameListCtrl::COLUMN_TITLE)
+                                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<std::mutex> lk(m_cache_mutex);
-    m_cache.ForEach([this](const std::shared_ptr<const UICommon::GameFile>& game_file) {
+    m_cache.ForEach(
-      if (ShouldDisplayGameListItem(*game_file))
+        [this, &title_database](const std::shared_ptr<const UICommon::GameFile>& game_file) {
-        m_shown_files.push_back(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<std::mutex> lk(m_title_database_mutex);
     for (const auto& drive : Common::GetCDDevices())
     {
       auto file = std::make_shared<UICommon::GameFile>(drive);
       if (file->IsValid())
       {
-        if (file->CustomNameChanged(m_title_database))
+        m_shown_names.push_back(file->GetName(title_database));
-          file->CustomNameCommit();
+        m_shown_files.push_back(std::move(file));
-        m_shown_files.push_back(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<std::string> game_paths = UICommon::FindAllGamePaths(
       SConfig::GetInstance().m_ISOFolder, SConfig::GetInstance().m_RecursiveISOFolder);
 
-  // Reload the TitleDatabase
-  {
-    std::unique_lock<std::mutex> lock(m_title_database_mutex);
-    m_title_database = {};
-  }
-
   bool cache_changed = false;
 
   {
@@ -647,7 +658,7 @@ void GameListCtrl::RescanList()
 
   {
     std::unique_lock<std::mutex> 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 CompareGameListItems(item1, item2, sortData, caller);
-  // 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);
 }
 
 void GameListCtrl::OnColumnClick(wxListEvent& event)

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_ptr<GameFile>s 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<std::shared_ptr<const UICommon::GameFile>> m_shown_files;
+  std::vector<std::string> m_shown_names;
 
   int m_last_column;
   int m_last_sort;

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));
+  const std::string& name = GetName();
-  if (name.empty())
-    name = GetName();
 
   int disc_number = GetDiscNumber() + 1;
 

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{};
 };
 

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<std::string> FindAllGamePaths(const std::vector<std::string>& directories_to_scan,
                                           bool recursive_scan)
@@ -52,8 +50,7 @@ void GameFileCache::Clear()
 }
 
 std::shared_ptr<const GameFile> GameFileCache::AddOrGet(const std::string& path,
-                                                        bool* cache_changed,
+                                                        bool* cache_changed)
-                                                        const Core::TitleDatabase& title_database)
 {
   auto it = std::find_if(
       m_cached_files.begin(), m_cached_files.end(),
@@ -67,7 +64,7 @@ std::shared_ptr<const GameFile> GameFileCache::AddOrGet(const std::string& path,
     m_cached_files.emplace_back(std::move(game));
   }
   std::shared_ptr<GameFile>& 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<void(const std::shared_ptr<const GameFile>&)> game_updated)
 {
   bool cache_changed = false;
 
   for (std::shared_ptr<GameFile>& 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<GameFile>* game_file,
+bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr<GameFile>* game_file)
-                                             const Core::TitleDatabase& title_database)
 {
   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)
-  if (!wii_banner_changed && !custom_banner_changed && !custom_title_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<GameFile>* game_fil
     copy->WiiBannerCommit();
   if (custom_banner_changed)
     copy->CustomBannerCommit();
-  if (custom_title_changed)
-    copy->CustomNameCommit();
   *game_file = std::move(copy);
 
   return true;

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<const GameFile> AddOrGet(const std::string& path, bool* cache_changed,
+  std::shared_ptr<const GameFile> AddOrGet(const std::string& path, bool* cache_changed);
-                                           const Core::TitleDatabase& title_database);
 
   // These functions return true if the call modified the cache.
   bool Update(const std::vector<std::string>& all_game_paths,
               std::function<void(const std::shared_ptr<const GameFile>&)> game_added_to_cache = {},
               std::function<void(const std::string&)> game_removed_from_cache = {});
   bool UpdateAdditionalMetadata(
-      const Core::TitleDatabase& title_database,
       std::function<void(const std::shared_ptr<const GameFile>&)> game_updated = {});
 
   bool Load();
   bool Save();
 
 private:
-  bool UpdateAdditionalMetadata(std::shared_ptr<GameFile>* game_file,
+  bool UpdateAdditionalMetadata(std::shared_ptr<GameFile>* game_file);
-                                const Core::TitleDatabase& title_database);
 
   bool SyncCacheFile(bool save);
   void DoState(PointerWrap* p, u64 size = 0);