RetroPlayer: Add RetroAchievements Game API callbacks - #150
Conversation
|
@garbear I know you're very busy with Beta 2 - I'd be keen when you're doing the next round of Retroplayer builds based on the release to see if we could include this and kodi-game/game.libretro#162 so we're properly able to test unlocking across all platforms. If working then we can start hacking out features into V23 :) |
|
Indeed, busy with Beta 2, I'll report back here after release (tho realistically prolly before) with a build that includes the open PRs. Just get any PRs open and fully updated and when I can do a build it'll include them all. I'm still doing my own local testing against |
17aa36d to
bf78933
Compare
06ee351 to
5c15304
Compare
e893bf7 to
c5bf624
Compare
|
Failed test cases, needed: --- a/xbmc/guilib/test/TestGamesGUIInfo.cpp
+++ b/xbmc/guilib/test/TestGamesGUIInfo.cpp
@@ -123,18 +123,20 @@ TEST_F(TestGamesGUIInfo, MarkEarnedOnlyCountsTheFirstUnlock)
achievementRuntime.SetState(MakeAchievementState());
bool newlyEarned = false;
- AchievementState state = achievementRuntime.MarkEarned(2, newlyEarned);
+ AchievementState state = achievementRuntime.MarkEarned(2, "2026-08-05 19:20", newlyEarned);
EXPECT_TRUE(newlyEarned);
EXPECT_EQ(state.unlockedAchievements, 2U);
+ EXPECT_EQ(state.achievements[1].unlockedDate, "2026-08-05 19:20");
// The achievement runtime re-reports achievements earned in an earlier
- // session, which must not inflate the count
- state = achievementRuntime.MarkEarned(2, newlyEarned);
+ // session, which must not inflate the count or replace the unlock date
+ state = achievementRuntime.MarkEarned(2, "2026-08-06 08:15", newlyEarned);
EXPECT_FALSE(newlyEarned);
EXPECT_EQ(state.unlockedAchievements, 2U);
+ EXPECT_EQ(state.achievements[1].unlockedDate, "2026-08-05 19:20");
// An unknown ID must not change anything
- state = achievementRuntime.MarkEarned(99, newlyEarned);
+ state = achievementRuntime.MarkEarned(99, "2026-08-07 12:00", newlyEarned);
EXPECT_FALSE(newlyEarned);
EXPECT_EQ(state.unlockedAchievements, 2U);
}I'm starting the process of making new RP builds. Still working through build/test failures: https://jenkins.kodi.tv/view/Automation/job/BuildMulti-All/3343/ |
|
Probably retarget my |
5c15304 to
3fad5fa
Compare
|
Thanks @garbear — both points addressed, and the branch is updated. Testing: I've had a LibreELEC Generic x86_64 build running with all of this and it works well — achievements unlock and submit across Genesis and NES titles, progress bars track measured achievements, and unlock notifications carry the right badge. |
3fad5fa to
41bc2e0
Compare
41bc2e0 to
4254f1c
Compare
|
Fixed the two baby rebase conflicts. I'll include this in my test builds today. |
|
Test builds are up! https://github.com/garbear/xbmc/releases/tag/retroplayer-22beta2-20260819 Included everything so far, all work, except for HW rendering. The new dialog works great. Next I'm working on an achievement to test unlocking. We should get this work against master soon. I'll do a review here first then we can upstream. |
|
Amazing. Yep I've been using it for some time now and really keen on building on it I've just been waiting for the master merge. It feels rock solid though it's more just making sure your using the rom version that aligns with what's in the RA dB. Open to more feature suggestions and tightening |
| /// | ||
| /// @note Added in Game API 7.1.0 | ||
| /// | ||
| void KodiRCOnGameLoaded(const game_rc_game_loaded& data) |
There was a problem hiding this comment.
Please remove branding (the Kodi word) from API calls.
There was a problem hiding this comment.
Done in 438c224. The C table already named these RCOnGameLoaded and friends — only the C++ wrapper carried the prefix, so the two halves now agree, and it matches what 7.1.1 did to GetPlaybackSpeed.
Left KodiInputEvent alone: it predates this PR.
One knock-on worth flagging — kodi-game/game.libretro#162 calls these by the old names, so it needs the same rename before the pair will build together. I'll push that once you're happy with the naming here.
| <value condition="!RetroPlayer.DiscEjected">60FFFFFF</value> | ||
| <value>FFFFFFFF</value> | ||
| </variable> | ||
| <variable name="GameAchievementBadgeTintVar"> |
There was a problem hiding this comment.
I think this can go right above <include name="GameAchievementItem">. That would be a better place for it than the generic Variables.xml.
There was a problem hiding this comment.
Moved in 438c224 — it now sits directly above <include name="GameAchievementItem"> in Includes_Games.xml, which is the only thing that references it. Both files still parse.
| /* | ||
| * RetroAchievements events, added in Game API 7.1.0. | ||
| * | ||
| * New members must be appended here so that the offsets of the members | ||
| * above stay valid for add-ons built against an older version of this | ||
| * header. | ||
| */ |
There was a problem hiding this comment.
Please remove this comment. AI gets too verbose with comments, so do a review pass after generation and strip out what isn't needed.
There was a problem hiding this comment.
Removed in 438c224. Fair — it restated what the struct layout already implies.
|
Looks like the % calculation is broken. 
|
| Add(new GAME::CDialogInGameSaves); | ||
| Add(new GAME::CGUIAgentWindow); | ||
| Add(new GAME::CDialogGameDiscManager); | ||
| Add(new GAME::CDialogGameAchievements); |
There was a problem hiding this comment.
Missing delete call (deletes via WINDOW_DIALOG_GAME_ACHIEVEMENTS constant)
There was a problem hiding this comment.
Good catch, fixed in 438c224. It was added in CreateWindows() and never torn down, so it outlived DestroyWindows() while every other game dialog was destroyed. DestroyWindow(WINDOW_DIALOG_GAME_ACHIEVEMENTS) now sits with the rest.
With this it'll genuinely be because there are so many achievements in that game and the weighting of the ones you've unlocked haven't triggered it over 0% because diff achievements have different points attached. I can look Into it further but it does work |
Four things from review on garbear#150. Drop the Kodi prefix from the eight RetroAchievements callbacks in the C++ wrapper. The C table already named them RCOnGameLoaded and so on; only the wrapper carried the branding, so this makes the two halves agree and follows what Game API 7.1.1 did for GetPlaybackSpeed. KodiInputEvent is left alone, being none of this PR's business. Destroy the achievements dialog. It was added in CreateWindows() and never torn down, so it outlived DestroyWindows() while every other game dialog did not. Move GameAchievementBadgeTintVar out of the generic Variables.xml and put it directly above GameAchievementItem in Includes_Games.xml, beside the only thing that uses it. Drop the comment about appending to the callback table. It restated what the struct layout already implies.
Review on garbear/xbmc#150 asked for the Kodi prefix to come off the RetroAchievements callbacks, so the C++ wrapper now matches the C table that already named them RCOnGameLoaded and friends. Follow that here. Eleven call sites, no behaviour change. This has to land with the Kodi side rather than before it -- the old names no longer exist.
|
Let's do version 8.0.0, what do you think? versions.h needs a rebase conflict fixed after game api v7.2.0. Do the other RC functions need to be dropped? |
RetroAchievements support currently lives in Kodi, which means Kodi drives the legacy dorequest.php API directly and every game client inherits whatever it implements. Move the runtime into the game add-on and give it a way to report back, so the add-on owns identification, memory access and unlocking, and Kodi owns presentation. Game API 6.1.0 appends eight callbacks to AddonToKodiFuncTable_Game: - RCOnGameLoaded, once the add-on has identified the game and resolved its achievement set - RCOnAchievementTriggered, when an achievement is earned - RCOnGameCompleted, when the whole set is earned - RCOnRichPresenceUpdated, for what the player is currently doing - RCOnLoginResult, for the outcome of a login attempt - RCOnAchievementProgress, for achievements that count something - RCOnServerError, for an API failure that will not be retried - RCOnConnectionChanged, for the transition in and out of offline play Members are appended rather than inserted, and the existing structs are left alone, so add-ons built against 6.0.0 keep working. MIN stays at 6.0.0. CAchievementRuntime holds the resulting state for the current game. Its mutating accessors are targeted rather than read-modify-write so that updates from the game thread and the GUI thread cannot clobber each other, and the info-label providers read single fields rather than copying the achievement list on every rendered frame. The achievements dialog lists the set with badges, points, rarity and unlock dates, and draws a progress bar for achievements that count something and haven't been earned. Progress is a snapshot taken when the dialog opens. Achievements whose trigger is a plain threshold have no counter and report nothing, which is why only some sets show a bar. Notifications cover unlocking, completion, login, server errors, and going offline and back. Only the unlock notification plays a sound, through the same path as every other Kodi notification. Badges are pre-cached when the game loads so an unlock draws its own badge immediately rather than briefly showing the game icon left over from the load notification. When RetroAchievements doesn't recognise a ROM's hash it still resolves it, to a placeholder game titled "Unsupported Game Version" holding a single always-true achievement. That would otherwise fire immediately and read as an unlock, then as having mastered a one-achievement game; it is now treated as a game without achievements. The achievements button in the game OSD is always shown. A game being unsupported, or the player not being signed in, is explained by a notification rather than by the button quietly disappearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four things from review on garbear#150. Drop the Kodi prefix from the eight RetroAchievements callbacks in the C++ wrapper. The C table already named them RCOnGameLoaded and so on; only the wrapper carried the branding, so this makes the two halves agree and follows what Game API 7.1.1 did for GetPlaybackSpeed. KodiInputEvent is left alone, being none of this PR's business. Destroy the achievements dialog. It was added in CreateWindows() and never torn down, so it outlived DestroyWindows() while every other game dialog did not. Move GameAchievementBadgeTintVar out of the generic Variables.xml and put it directly above GameAchievementItem in Includes_Games.xml, beside the only thing that uses it. Drop the comment about appending to the callback table. It restated what the struct layout already implies.
438c224 to
9dfd4bc
Compare
|
Rebased onto Worth recording why 8.0.0 is right rather than 7.3.0, because I checked while resolving: master's 7.2.0 is your "Allow setting game FPS/samplerate" ( Yes, the other RC functions should be droppedThey are already dead, which I had not appreciated until you asked. Seven legacy entry points survive in Kodi still calls all seven, from the old GAME_ERROR CGameLibRetro::RCEnableRichPresence(const std::string& script)
{
return GAME_ERROR_NOT_IMPLEMENTED;
}So with both PRs in, Kodi runs a second, older achievements implementation that drives an API the add-on has deliberately stopped answering. Two implementations, one of them talking to a wall. That is the strongest argument for the major bump: removing those seven changes the table layout, so nothing built against 7.x can be loaded against it, which is exactly what But it is bigger than deleting seven pointersThe functions are the small part. Behind them sits Kodi's own achievements implementation:
The new How would you like it split? Three options as I see them:
My preference is 3 — the version bump and the ABI change land together, which is the bit that has to be atomic, and the 1,643 lines of dead implementation follow separately without an ABI consequence. Say which you want and I will do it. One sequencing note#151 currently also claims 7.2.0, for the hardware framebuffer size — a different layout under the same number as your FPS/samplerate change. I am moving that one to 7.3.0 as you asked. If this PR lands first at 8.0.0, I will re-target it to 8.1.0 instead; either order works as long as they do not both sit on 7.2.0. |
|
Rip out as much as you can! |
The add-on owns rc_client now, so Kodi's own achievements implementation is dead weight: game.libretro answers every one of the legacy entry points with GAME_ERROR_NOT_IMPLEMENTED, while Kodi carried on calling them. Two implementations, one of them talking to a wall. Gone from the Game API: RCGenerateHashFromFile, RCGetGameIDUrl, RCGetPatchFileUrl, RCPostRichPresenceUrl, RCEnableRichPresence, RCGetRichPresenceEvaluation and RCResetRuntime, with their C++ wrappers and thunks. That is what makes this 8.0.0 rather than an additive bump -- the offsets of everything after them move. Gone with them: cores/RetroPlayer/cheevos entirely, and the legacy half of CGameClientCheevos. What remains there are the eight On* handlers the new callbacks deliver into, which never depended on any of it. Rich presence still reaches savestate captions. It arrives through RCOnRichPresenceUpdated and is kept in CAchievementRuntime, so ReversiblePlayback reads it from there rather than asking the client for an evaluation it no longer computes.
|
Ripped out. What went:
What stayed, and why it had to: One thing needed re-plumbing rather than deleting. Savestate captions were taking their text from I checked |
Review on garbear/xbmc#150 asked for the Kodi prefix to come off the RetroAchievements callbacks, so the C++ wrapper now matches the C table that already named them RCOnGameLoaded and friends. Follow that here. Eleven call sites, no behaviour change. This has to land with the Kodi side rather than before it -- the old names no longer exist.
Two things, both following from review elsewhere. garbear/xbmc#150 removes the seven legacy RetroAchievements entry points from the Game API now that rc_client lives here, so the stubs that answered them with GAME_ERROR_NOT_IMPLEMENTED override nothing any more. Gone, along with their declarations. The login guard was one-way. m_loginStarted is set when a login begins and was only cleared in Deinitialize(), so a rejected token left it set and corrected credentials could not start another attempt until the game was unloaded. It exists to stop a second login racing one already in flight, not to make failure permanent, so the failure path clears it. SetCredentials() already calls BeginLogin(), so a corrected username or token now retries on the spot.
Carry the unlock as a CDateTime rather than preformatted text. The source is a Unix timestamp from rc_client, not a string, and the only consumer is the achievements dialog, so formatting it at the edge meant the runtime held locale-dependent text nothing else could reason about. The date now travels as a date and is localized where it is shown. The rewind buffer comment was wrong. It does not hold a serialized state per frame -- CDeltaPairMemoryStream stores xor deltas, and only the words that changed -- so the logged figure is a ceiling rather than a cost. Says so now. Comment tidying from review: the second sentence about add-ons declining optional entry points is gone, the stdint include no longer names the types it provides, the two localized-string references are a line each, and the removals in the last commit left eleven consecutive blank lines behind.
2 participants
Description
Extends the Game add-on API with add-on-to-Kodi callbacks for RetroAchievements events, keeping rcheevos as a binary add-on dependency (in game.libretro) rather than compiled into Kodi. This is the Kodi-side counterpart to the game.libretro rc-client-integration PR.
Per garbear's architectural guidance, rcheevos stays in game.libretro. Kodi receives events and handles display only — it performs no RetroAchievements network I/O and holds no knowledge of the achievement runtime. No rcheevos source is compiled into Kodi.
Flow:
game.libretro (owns rc_client) → Game API callbacks → CGameClientCheevos → CAchievementRuntime → OSD dialogGame API additions (kodi-dev-kit), 6.0.0 → 6.1.0:
game_rc_achievement,game_rc_game_loaded,game_rc_achievement_triggered,game_rc_login_result, plusGAME_RC_UNLOCK_STATEAddonToKodiFuncTable_Game:RCOnGameLoaded,RCOnAchievementTriggered,RCOnGameCompleted,RCOnRichPresenceUpdated,RCOnLoginResultGame.hfor add-on-side useAddonToKodiFuncTable_Gameso the offsets used by add-ons built against 6.0.0 stay valid, andKodiToAddonFuncTable_Gameis untouched.ADDON_INSTANCE_VERSION_GAME_MINtherefore stays at 6.0.0 and existing add-ons keep working.Kodi-side implementation:
GameClient.cpp: static callback functions wired inInitialize(), forwarding toCGameClientCheevosGameClientCheevos.cpp: event receivers that validate every pointer and count from the add-on, cap what they copy, publish to the achievement runtime and post localized notifications. Badge, icon and avatar URLs are handed to Kodi's texture cache rather than downloaded, so no background threads are involved.AchievementRuntime.h/cpp: extends garbear's existing runtime with targeted accessors for the two fields the info label reads, sinceGetState()copies the whole achievement list and skins query info labels once per frame per controlGamesGUIInfo.cpp: handler forRetroPlayer.AchievementsProgressGameSettings.cpp:SetAchievementsLoggedIn()so a rejected token can't leave the UI claiming the player is signed inOSD dialog:
DialogGameAchievements: achievement list with badges, points, rarity and unlock dates, refreshed live when one is earned. ReusesDialogGameControllers.xmlas its window file, as that file documents, so skins without the new layout still work.Skin (Estuary):
GameOSD.xml: an achievements button, shown unconditionally so the dialog can explain whether the player needs to sign in or the game has no achievement set. The menu grows by one row to fit it.Includes_Games.xml: the dialog layout, with the focused row scrolling its title and criteria since achievement criteria are frequently longer than the columnBugs fixed along the way:
CDateTime::GetAsLocalizedDate()was called with a string literal, which binds to theGetAsLocalizedDate(bool)overload astrue— the format was silently discarded and the long date usedGAME_ERROR_NOT_IMPLEMENTEDwas logged at ERROR. Declining an optional part of the API is not a failure, so it now logs at DEBUG and ERROR is reserved for genuine onesCGameSettings::LoginToRA()are now localized, closing a review comment deferred from RetroPlayer: Add RetroAchievements integration xbmc/xbmc#28496Motivation and context
RetroAchievements integration for Kodi RetroPlayer. Previously rcheevos was vendored directly into the Kodi source tree, which garbear identified as architecturally wrong — third-party libraries used by binary add-ons should stay in the add-on, not in Kodi core. This PR restructures the integration so Kodi only receives and displays events, while game.libretro owns all rcheevos logic.
Addresses garbear's review feedback on #149, and builds on the achievement runtime introduced by #28496.
How has this been tested?
Built and tested on Ubuntu 26.04 x86_64, against the companion game.libretro branch.
Verified end to end: signed in, loaded Sonic the Hedgehog 2 (Mega Drive), earned Hold Your Breath in play, and confirmed via the log that the event reached Kodi, the toast appeared with its badge, the award was submitted to RetroAchievements and the score updated. Also verified on Super Mario Bros. (NES) that the achievement list, counts and progress label populate correctly.
TestGamesGUIInfo, covering the info label, the empty case, andCAchievementRuntime::MarkEarned()not double-counting a re-reported unlockclang-formatcleanPlatforms tested: Ubuntu 26.04 (development), LibreELEC Generic x86_64 (previous branch iterations)
Requires the companion game.libretro#162 for the events to arrive; without it Kodi simply shows no achievement data.
What is the effect on users?
Adds RetroAchievements support to Kodi RetroPlayer. Users with a RetroAchievements account can:
No impact on users who don't use RetroAchievements. The achievements button is always present in the game OSD, and tells the player they need to sign in rather than silently hiding — the same reasoning as offering port 2 on a one-player game.
Leaderboards are deliberately out of scope. They are redundant until hardcore mode lands and are better reviewed alongside it.
Screenshots (if appropriate):
Types of change
Checklist: