Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retroachievements activate #11752

Merged

Conversation

LillyJadeKatrin
Copy link
Contributor

This is a portion of an integration with the RetroAchievements tools and libraries for connecting to the website, downloading data, unlocking achievements and submitting to leaderboards for games running in Dolphin. In this PR, the AchievementsManager created in a previous PR is further implemented to activate achievements, leaderboards, and rich presence within the achievement runtime as retrieved from the RetroAchievements API, preparing them to be compared to memory.

@AdmiralCurtiss
Copy link
Contributor

There's a typo in the first commit message, rc_rumetime_do_frame.

You should also probably make a bool m_hardcore_mode_enabled = false; or something like that so that you don't have mysterious false flags and TODOs for that in the code. It's fine if there is currently no way to actually activate the variable.

@AdmiralCurtiss
Copy link
Contributor

You probably also want to cache the state of the various enable/disable flags instead of calling them in a loop -- keep in mind that the values returned by Config::Get() represent the current config in memory can can change at any time by being set from a different thread while the code is running, which could lead to some confusing behavior here.

Source/Core/Core/AchievementManager.h Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.h Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.h Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-activate branch 2 times, most recently from ebee592 to b344b15 Compare April 13, 2023 01:52
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.h Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.h Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Outdated Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Show resolved Hide resolved
Source/Core/Core/AchievementManager.cpp Show resolved Hide resolved
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-activate branch 6 times, most recently from 1b15ff3 to 9075cf7 Compare April 14, 2023 05:48
ActivateDeactivateAchievement is passed an Achievement ID as returned from the FetchGameData API call and determines whether to activate it, deactivate it, or leave it where it is based on its current known state and what settings are enabled.

Activating or deactivating an achievement entails calling a method provided by rcheevos that performs this on the rcheevos runtime. Activating an achievement loads its memory signature into the runtime; now the runtime will process the achievement each time the rc_runtime_do_frame function is called (this will be in a future PR) to determine when the achievement's requirements are met. Deactivating an achievement unloads it from the runtime.

The specific logic to determine whether an achievement is active operates over many fields but is documented in detail inside the function. There are multiple settings flags for which achievements are enabled (one flag for all achievements, an "unofficial" flag for enabling achievements marked as unofficial i.e. those that have logic on the site but have not yet been officially approved, and an "encore" flag that enables achievements the player has already unlocked) and this function also evaluates whether the achievement has been unlocked in hardcore mode or softcore mode (though currently every reference to the current hardcore mode state is hardcoded as false).
@AdmiralCurtiss
Copy link
Contributor

Okay... this looks basically fine to me now, but here's one thing I noticed.

Over here, when inserting into m_unlock_map, your key is an arbitrary id value returned by the API:

https://github.com/LillyJadeKatrin/dolphin-retroachievements/blob/retroachievements-activate/Source/Core/Core/AchievementManager.cpp#L170-L175

But over here, in FetchUnlockData(), you're querying the map by a consecutive integer from 0 to unlock_data.num_achievement_ids:

https://github.com/LillyJadeKatrin/dolphin-retroachievements/blob/retroachievements-activate/Source/Core/Core/AchievementManager.cpp#L307-L315

This looks... strange to me. Going by the return value definition here, I would expect the lookup to instead use auto it = m_unlock_map.find(unlock_data.achievement_ids[ix]);. Right?

FetchUnlockData is an API call to RetroAchievements that downloads a list of achievement IDs for a game that the user has already unlocked and published to the site. It accepts a parameter for whether or not hardcore or softcore achievements are being requested, so that must be provided as well. Once it has the requested list on hand, it updates each achievement's status in the unlock map and will activate or deactivate achievements as necessary.
…tManager

LoadUnlockData and ActivateDeactivateAchievements are the public API components responding to the FetchUnlocks and A/DAchievement (singular) private methods.

LoadUnlockData is asynchronous and performs both a hardcore and a softcore unlock call, updating the unlock map and the active status of any achievements returned from these calls.

ActivateDeactivateAchievements calls ActivateDeactivateAchievement on every achievement ID found in m_game_data, initializing the unlock map for each ID if not already found.

Both of these are currently called in LoadGameByFilenameAsync once the game has been loaded properly. There's a lock around this, to ensure that the unlock map is initialized properly by ActivateDeactivate Achievements before FetchUnlockData makes modifications to it without stalling the async portions of FetchUnlockData.
This activates or deactivates leaderboards in the rcheevos runtime similarly to achievements. The logic is much more straightforward - all leaderboards are active together; there is nothing requiring some leaderboards to be active while others are unactive, and even a leaderboard that has been submitted to in this session is still active to be submitted to again. The only criteria are that leaderboards must be enabled in the settings, and hardcore mode must be on, the latter of which is false until a future PR.
RetroAchievements Rich Presence is a script that is run periodically on a game's memory to provide a detailed text description of what the player is doing. Existing Discord presence on Dolphin would update a player's Discord status to say not just that they are using Dolphin but that they are playing, for example, Sonic Adventure 2 Battle; Rich Presence would detail that the player is in City Escape with 5 lives and 142 rings.

Activating this in the runtime simply entails loading that text script, as returned by the FetchGameData API call, into the runtime, here only determined by whether rich presence is enabled in the achievement settings. Deactivating this is done via the same rcheevos method by setting the rich presence to an empty string.
@LillyJadeKatrin
Copy link
Contributor Author

You're absolutely right and I've fixed it. Apologies, I admit parts of this I haven't tested as much as I'd like because test data hasn't been prepared on RetroAchievements yet and I haven't had the opportunity to look into what Dolphin does for unit testing yet.

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks fine to me now.

Not merging this because the build infrastructure is down, but I think this is good to go as soon as it's back up and passes checks.

@AdmiralCurtiss AdmiralCurtiss merged commit 361ffd5 into dolphin-emu:master Apr 18, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants