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 |
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.
garbear
left a comment
garbear
left a comment
There was a problem hiding this comment.
Commits can be squashed. Everything here LGTM now. Time to upstream? I wanna do B2 this weekend.
|
I needed this to fix clang static analysis (https://jenkins.kodi.tv/job/android-arm64-docker/17484/clang/source.31108e58-e5e7-410a-9672-8bea76e806cf/#38): --- a/xbmc/games/addons/cheevos/GameClientCheevos.cpp
+++ b/xbmc/games/addons/cheevos/GameClientCheevos.cpp
@@ -34,30 +34,6 @@
namespace
{
-// C ABI expects a raw callback + context pointer, so we bridge to std::function here
-void __cdecl GetCheevoUrlIdCallback(const void* context,
- const char* achievementUrl,
- unsigned int cheevoId)
-{
- if (context == nullptr)
- return;
-
- const auto* callback = static_cast<
- const std::function<void(const std::string& achievementUrl, unsigned int cheevoId)>*>(
- context);
- if (!(*callback))
- return;
-
- try
- {
- (*callback)(achievementUrl != nullptr ? std::string{achievementUrl} : std::string{}, cheevoId);
- }
- catch (...)
- {
- // Never allow exceptions to unwind through the C ABI callback boundary
- }
-}
-
// The add-on is not trusted to send sane counts, so cap what we copy out of it.
// The largest sets published by RetroAchievements are an order of magnitude
// below these limits. |
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.
8a283a0 to
3c2ac3f
Compare
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.
3c2ac3f to
c7e9eae
Compare
|
Done, and rebased onto master. Should be good now Also dropped the header's |
|
Time to PR against master? Clean up history slop, comment slop clearly explains all design decisions so the squashed commit message can be minimal (or at least useful). |
ec3a90a to
0771867
Compare
Kodi hashed the ROM, talked to the RetroAchievements server and ran the achievement runtime itself. The add-on is better placed for all three: it already owns the emulator's memory, and rcheevos ships with it. Kodi now receives events instead. Eight callbacks report what the runtime decided - game loaded, achievement triggered, progress, challenge and leaderboard indicators, rich presence, connection state - and Kodi keeps the state they describe for skins to show. Achievement state travels in its own savestate field, sized from what the runtime reports rather than a fixed reserve, leaving the emulator's blob the exact size the core reports. The implementation this replaces is removed, along with the seven legacy entry points it drove. That is what earns Game API 8.0.0. Pairs with kodi-game/game.libretro#162.
0771867 to
6521685
Compare
|
Raised xbmc#29074 !!!! Really excited to get this in to unlock functionality for everyone, see what skinners can produce and also to bring down my diff on my big spaghetti build thats just continued to buidl on this! |
|
Closing as completed by xbmc#29074. Great work! |
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: