RetroAchievements - Default Badges #12778

LillyJadeKatrin · 2024-05-13T23:25:38Z

This PR adds the infrastructure to use default Achievements-related badges/icons located in the Sys folder when badges normally downloaded from the RetroAchievements website are unavailable. The badges provided here are placeholders for when a graphics designer within Dolphin can provide better replacements.

iwubcode · 2024-05-13T23:34:49Z

I think I originally gave you the instruction to put your resources in the "Load" folder. However, I wasn't remembering correctly from my reference implementation (editor PR). I'd like to keep the Load folder rather clean. As to me that folder should mirror the "User/Load" folder and refers to loaded content that Dolphin team maintains.

Not sure if we need another folder ("resources"?) to keep the root from getting too chaotic.

EDIT: Oh we have a resources folder already. Maybe we just need better organization under that.

iwubcode · 2024-05-13T23:41:01Z

Source/Core/DolphinQt/Achievements/AchievementHeaderWidget.cpp

@@ -68,24 +68,20 @@ void AchievementHeaderWidget::UpdateData()

  QString user_name = QtUtils::FromStdString(instance.GetPlayerDisplayName());
  QString game_name = QtUtils::FromStdString(instance.GetGameDisplayName());
-  AchievementManager::BadgeStatus player_badge = instance.GetPlayerBadge();
-  AchievementManager::BadgeStatus game_badge = instance.GetGameBadge();
+  AchievementManager::Badge player_badge = instance.GetPlayerBadge();


const AchievementManager::Badge& player_badge = instance.GetPlayerBadge();

iwubcode · 2024-05-13T23:41:21Z

Source/Core/DolphinQt/Achievements/AchievementHeaderWidget.cpp

-  AchievementManager::BadgeStatus player_badge = instance.GetPlayerBadge();
-  AchievementManager::BadgeStatus game_badge = instance.GetGameBadge();
+  AchievementManager::Badge player_badge = instance.GetPlayerBadge();
+  AchievementManager::Badge game_badge = instance.GetGameBadge();


const AchievementManager::Badge& game_badge = instance.GetGameBadge();

iwubcode · 2024-05-13T23:48:36Z

Source/Core/VideoCommon/Assets/CustomTextureData.cpp

+  return LoadPNGTexture(level, buffer);
+}
+
+bool LoadPNGTexture(CustomTextureData::ArraySlice::Level* level, const std::vector<u8> buffer)


bool LoadPNGTexture(CustomTextureData::ArraySlice::Level* level, const std::vector<u8>& buffer)

mbc07

AFAICT the default badges aren't intended to be user replaceable, so I don't think those PNGs should reside anywhere under Sys\Load, they should all go to Sys\Resources, after all that's where the other image assets we use in Dolphin's GUI are stored...

LillyJadeKatrin · 2024-05-14T02:05:17Z

I think I originally gave you the instruction to put your resources in the "Load" folder. However, I wasn't remembering correctly from my reference implementation (editor PR). I'd like to keep the Load folder rather clean. As to me that folder should mirror the "User/Load" folder and refers to loaded content that Dolphin team maintains.

Not sure if we need another folder ("resources"?) to keep the root from getting too chaotic.

EDIT: Oh we have a resources folder already. Maybe we just need better organization under that.

Instructions unclear, moved files to Licenses folder :p

Kidding, but where should I put these? Resources/RetroAchievements?

mbc07 · 2024-05-14T02:30:47Z

I'd just drop them under Sys\Resources, without a subfolder. The existing ones aren't separated anyway, and this can be handled in a later PR if it bothers enough...

MayImilae · 2024-05-19T10:29:44Z

Is there anything else needed to merge this? I'm already working on the permanent badges, and this will need to be merged before I can make a PR with those.

@iwubcode Please review!

AdmiralCurtiss · 2024-05-19T10:33:24Z

I swear someone else pointed this out but I don't see it... but the trophy and trophy_color icons should be called something like trophy_locked and trophy_unlocked instead.

Also, shouldn't they be achievement rather than trophy? Trophy is Sony terminology.

MayImilae · 2024-05-19T10:36:19Z

The current plan as I discussed with Lilly is for them to be padlocks, so that will change. But I could delete the trophies in the upcoming PR with the new icons. Depends on what Lilly wants to do.

AdmiralCurtiss · 2024-05-19T10:42:48Z

Source/Core/Core/AchievementManager.cpp

+  for (u32 bx = 0; bx < achievement_list->num_buckets; bx++)
+  {
+    auto& bucket = achievement_list->buckets[bx];
+    for (u32 achievement = 0; achievement < bucket.num_achievements; achievement++)
+    {
+      u32 achievement_id = bucket.achievements[achievement]->id;
+      // I need these defaults to be set before making any FetchBadge calls, because FetchBadge
+      // will update the UI and attempt to retrieve these badges.
+      m_unlocked_badges[achievement_id] = m_default_unlocked_badge;
+      m_locked_badges[achievement_id] = m_default_locked_badge;
+    }
+  }


This seems rather wasteful of memory, and also unnecessary. You already have a GetAchievementBadge() function that falls back to the default icon if it's not in the m_unlocked_badges/m_locked_badges maps, just use that consistently everywhere?

AdmiralCurtiss · 2024-05-19T10:46:23Z

The current plan as I discussed with Lilly is for them to be padlocks, so that will change. But I could delete the trophies in the upcoming PR with the new icons. Depends on what Lilly wants to do.

The point is that currently one would need to update the code when the contents of the images changes, since the filenames refer to a specific object rather than a generic concept.

AdmiralCurtiss · 2024-05-19T10:53:37Z

Source/Core/Core/AchievementManager.cpp

+  if (m_default_player_badge.data.empty())
+  {
+    if (!LoadPNGTexture(&m_default_player_badge,
+                        directory + static_cast<std::string>(DEFAULT_PLAYER_BADGE_FILENAME)))


Err what? Can you even cast a string_view to a string...? This feels super sketchy, please just use a constructor or fmt::format or anything else here.

AdmiralCurtiss · 2024-05-19T10:58:14Z

Source/Core/Core/AchievementManager.cpp

+  OSD::AddMessage(
+      fmt::format("Unlocked: {} ({})", client_event->achievement->title,
+                  client_event->achievement->points),
+      OSD::Duration::VERY_LONG,
+      (rc_client_get_hardcore_enabled(AchievementManager::GetInstance().m_client)) ?
+          OSD::Color::YELLOW :
+          OSD::Color::CYAN,
+      &AchievementManager::GetInstance().m_unlocked_badges[client_event->achievement->id]);


For readability I would suggest using a local reference for multiple calls to AchievementManager::GetInstance(). Something like:

auto& instance = AchievementManager::GetInstance(); OSD::AddMessage(fmt::format("Unlocked: {} ({})", client_event->achievement->title, client_event->achievement->points), OSD::Duration::VERY_LONG, (rc_client_get_hardcore_enabled(instance.m_client)) ? OSD::Color::YELLOW : OSD::Color::CYAN, &instance.m_unlocked_badges[client_event->achievement->id]);

mbc07 · 2024-05-19T16:46:21Z

I swear someone else pointed this out but I don't see it...

It was me, but I commented on Discord. I'll leave the message here too, for reference:

achievements_player.png and achievements_game.png closely resembles the const names used in code (DEFAULT_PLAYER_BADGE_FILENAME and DEFAULT_GAME_BADGE_FILENAME), but achievements_trophy.png and achievements_trophy_color.png do not

In the previous PR regarding the default badges (from before the rc_client refactor), May pointed out Dolphin assets doesn't use color to represent different states, so with that in mind, I'd suggest updating the placeholder trophy filenames to achievements_locked.png and achievements_unlocked.png, respectively.

Was informed by the RetroAchievements team that this isn't an option in most emulators, and as the next commits will be to enable default icons, there will always be something to display.

Achievement badges/icons are refactored into the type CustomTextureData::ArraySlice::Level as that is the data type images loaded from the filesystem will be. This includes everything that uses the badges in the Qt UI and OnScreenDisplay, and similarly removes the OSD::Icon type because Level already contains that information.

The defaults get loaded in by Dolphin at emulator start, and are used if the badge that would normally be displayed has not for whatever reason been downloaded yet. Badges attached to this PR are placeholders (MayIMilae is designing permanent badges) and reside in Sys\Load\RetroAchievements.

The names attached to the BadgeStatus object are obsolete and unneeded and are removed from everything that uses them. All BadgeStatus references are updated to just Badge.

iwubcode reviewed May 13, 2024

View reviewed changes

mbc07 suggested changes May 14, 2024

View reviewed changes

LillyJadeKatrin force-pushed the retroachievements-default-badges-v2 branch 2 times, most recently from 6660638 to 2970af6 Compare May 14, 2024 04:19

mbc07 approved these changes May 14, 2024

View reviewed changes

AdmiralCurtiss reviewed May 19, 2024

View reviewed changes

LillyJadeKatrin force-pushed the retroachievements-default-badges-v2 branch from 2970af6 to bc6cf9e Compare May 23, 2024 14:21

LillyJadeKatrin added 2 commits May 23, 2024 10:40

Remove Badges Enabled option from Achievements

75465f0

Was informed by the RetroAchievements team that this isn't an option in most emulators, and as the next commits will be to enable default icons, there will always be something to display.

LillyJadeKatrin force-pushed the retroachievements-default-badges-v2 branch from bc6cf9e to fbb6be2 Compare May 23, 2024 14:41

LillyJadeKatrin added 2 commits May 23, 2024 21:29

Remove names from badge objects

1e9e0cd

The names attached to the BadgeStatus object are obsolete and unneeded and are removed from everything that uses them. All BadgeStatus references are updated to just Badge.

AdmiralCurtiss force-pushed the retroachievements-default-badges-v2 branch from fbb6be2 to 1e9e0cd Compare May 23, 2024 19:29

AdmiralCurtiss approved these changes May 23, 2024

View reviewed changes

AdmiralCurtiss merged commit f991610 into dolphin-emu:master May 23, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RetroAchievements - Default Badges #12778

RetroAchievements - Default Badges #12778

LillyJadeKatrin commented May 13, 2024

iwubcode commented May 13, 2024 •

edited

iwubcode May 13, 2024

iwubcode May 13, 2024

iwubcode May 13, 2024

mbc07 left a comment

LillyJadeKatrin commented May 14, 2024

mbc07 commented May 14, 2024

MayImilae commented May 19, 2024

AdmiralCurtiss commented May 19, 2024

MayImilae commented May 19, 2024

AdmiralCurtiss May 19, 2024

AdmiralCurtiss commented May 19, 2024

AdmiralCurtiss May 19, 2024

AdmiralCurtiss May 19, 2024 •

edited

mbc07 commented May 19, 2024 •

edited

RetroAchievements - Default Badges #12778

RetroAchievements - Default Badges #12778

Conversation

LillyJadeKatrin commented May 13, 2024

iwubcode commented May 13, 2024 • edited

iwubcode May 13, 2024

Choose a reason for hiding this comment

iwubcode May 13, 2024

Choose a reason for hiding this comment

iwubcode May 13, 2024

Choose a reason for hiding this comment

mbc07 left a comment

Choose a reason for hiding this comment

LillyJadeKatrin commented May 14, 2024

mbc07 commented May 14, 2024

MayImilae commented May 19, 2024

AdmiralCurtiss commented May 19, 2024

MayImilae commented May 19, 2024

AdmiralCurtiss May 19, 2024

Choose a reason for hiding this comment

AdmiralCurtiss commented May 19, 2024

AdmiralCurtiss May 19, 2024

Choose a reason for hiding this comment

AdmiralCurtiss May 19, 2024 • edited

Choose a reason for hiding this comment

mbc07 commented May 19, 2024 • edited

iwubcode commented May 13, 2024 •

edited

AdmiralCurtiss May 19, 2024 •

edited

mbc07 commented May 19, 2024 •

edited