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 Load Game Data #11730

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 generate and resolve the RetroAchievements hash for the requested game, open a session, and download the necessary information to process achievements, leaderboards, and rich presence for the game.

@AdmiralCurtiss
Copy link
Contributor

One thing immediately strikes me here: Dolphin works with a large amount of different file formats for games, from plain ISOs to a lot of compressed and container formats to even launching an extracted disc filesystem. Wii software might even be installed in the virtual NAND and launched from there. Collecting all that into a single filename and attempting to implement a consistent hash for that on the library side seems somewhat... ill-advised for that. Are you sure you want to do it like this? If yes, what will be the limitations of this approach, ie. what file formats are you willing to support? Keep in mind that anything less than what Dolphin itself supports will cause a number of users to not have access to achievements even if they have a copy of the game (and probably angry support requests, which I'd rather not deal with).

Secondly, I find the way you're storing all the API replies as class members a bit weird. Do they really all need to live until game shutdown? Why can't you destroy them immediately after the function call is done?

@LillyJadeKatrin
Copy link
Contributor Author

Put discussion about the file being used to hash in the Discord channel as that one will be a doozy. As for this:

Secondly, I find the way you're storing all the API replies as class members a bit weird. Do they really all need to live until game shutdown? Why can't you destroy them immediately after the function call is done?

Was mostly keeping them around for convenience, so I didn't have to extract and keep around the individual pieces of data I'm getting from the API replies. I suppose I could do this the not-lazy way if that's preferable.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch from 33df841 to dc14781 Compare April 7, 2023 13:43
@LillyJadeKatrin
Copy link
Contributor Author

Please DO NOT run checks right now! Currently fifoci will fail until rcheevos is hotfixed, which is in progress.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch 6 times, most recently from 282a80d to e7c1e94 Compare April 10, 2023 14:04
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
@AdmiralCurtiss
Copy link
Contributor

I still think you're storing too much state in the class that could just as well be local to the function. But this is mostly fine, so don't let me block you if no one else has problems with this.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch from e7c1e94 to a206f3b Compare April 10, 2023 23:00
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch from a206f3b to 63636c3 Compare April 11, 2023 04:04
@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Apr 11, 2023

I think your first two commits should be squashed together.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch 2 times, most recently from 7569dad to 3417d20 Compare April 11, 2023 21:14
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch from 3417d20 to ec5d662 Compare April 11, 2023 23:35
Added the ResolveHash method to AchievementManager. This is a blocking function to send a hash string to the RetroAchievements server to verify it and get a game ID back.
Added a call to the RetroAchievements Start Session API to AchievementManager. This is primarily for client-side activation, so it doesn't return much of value, aside from its success/error information, but I'm storing the return structure in case this changes in the future.
FetchGameData is the big one - this retrieves the logic for all the achievements, leaderboards, and rich presence, and all the relevant metadata for the game.
LoadGameByFilenameAsync sets up a volume reader and hashes the volume, then uses that hash to make the three consecutive API requests to resolve hash, start session and load game data.

CloseGame resets the m_is_game_loaded flag, wipes the queue, and destroys all the game data responses.
Update the rcheevos submodule to the most recent origin/develop, which contains a fix to the gamecube hash algorithm to politely exit if it is unable to open a file.
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-load-game-data branch from ec5d662 to 6982f52 Compare April 12, 2023 07:09
@AdmiralCurtiss AdmiralCurtiss merged commit c5bbe0a into dolphin-emu:master Apr 12, 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
2 participants