RetroPlayer: Add Achievements #126

Shardul555 · 2021-07-11T17:00:02Z

Description

This Pull request adds support for Achievements in RetroPlayer, data fetched from RetroAchievements API have information about the achievements provided for a particular game, so we used that data for activating achievements, obtaining achievement state for every frame and then awarding it whenever it triggered.
User will be notified about an unlocked achievement through a pop-up notification and also through their RetroAchievement.org profile.
This PR will use kodi-game/game.libretro#73 for calling rcheevos functions.

Motivation and Context

Last year we have added support for RCheevos in RetroPlayer, so adding support for Achievements is one another task that was needed to be accomplished.

How Has This Been Tested?

Screenshots (if appropriate):

Notifying user through pop-up notification:

Information updated in RetroAchievements profile:

Types of change

Bug fix (non-breaking change which fixes an issue)
Clean up (non-breaking change which removes non-working, unmaintained functionality)
Improvement (non-breaking change which improves existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that will cause existing functionality to change)
Cosmetic change (non-breaking change that doesn't touch code)
None of the above (please explain below)

Checklist:

My code follows the Code Guidelines of this project
My change requires a change to the documentation, either Doxygen or wiki
I have updated the documentation accordingly
I have read the Contributing document
I have added tests to cover my change
All new and existing tests passed

garbear · 2021-07-17T20:52:26Z

Sorry, I missed this PR when you opened it 6 days ago. Congratulations on the first PR of the summer, even if it is for debugging purposes! It helps greatly to see your progress as time goes on. It looks like you're on the right track. Let me know when you want more specific feedback on the code.

Shardul555 · 2021-07-18T03:33:56Z

Hey @garbear , I am facing an issue while building these functions in VS 2019, as soon as I am opening any game, Kodi execution is breaking. Because of it, I am not able to debug various changes done till now.
As soon as I am removing the functions I have written, Kodi is running fine.
If you have any idea related to it then please guide.
Screenshot for the issue:

log file for it: https://paste.kodi.tv/pejiqepuyi.kodi

garbear · 2021-07-18T05:38:21Z

I've had this happen before. The problem is a mismatch in the API used for game.libretro and the API used in Kodi. Kodi is definitely loading a stale version of game.libretro.

When you add a function to the API, then the "ABI", application binary interface, is changed. Adding a function means you have to recompile against the correct headers. You're seeing 0x000000 because the added function pointer is missing in the stale game.libretro - it doesn't know about the new functions, so it doesn't assign them pointers, leaving them NULL.

Because adding a function breaks ABI in an incompatible way, to follow semver, you should change this version to 3.0.0: https://github.com/garbear/xbmc/blob/retroplayer-19.1/xbmc/addons/kodi-dev-kit/include/kodi/versions.h#L100. This change to 3.0.0 should appear in the diff of this PR.

You know you bumped to 3.0.0 correctly when the generated addon.xml file replaces ADDON_DEPENDS here with kodi.game 3.0.0. Look in the game.libretro folder for addon.xml to make sure it's generated against the correct version of Kodi's Game API.

There are two ways to build binary add-ons for Windows, which way are you using to build game.libretro? One way is better suited for development - once you get that working, it'll be easier to keep game.libretro in sync.

Shardul555 · 2021-07-18T10:59:13Z

@garbear ,Firstly commenting on how I build game.libretro : I followed https://github.com/NikosSiak/game.libretro#developing-on-windows it for developing and after these commands one additional command: cmake --install . --config Debug --prefix {path_to_kodi_build_folder}\addons from location {path_to_kodi_source_folder}\cmake\addons\build\game.libretro-prefix\src\game.libretro-build . This procedure builds my game.libretro.

Now about changing version to 3.0.0, even after changing version to 3.0.0 and building it again, the generated addon.xml file in xbmc\cmake\addons\build\game.libretro\game.libretro location does not change as expected, it is same as previous.
addon.xml.in file content:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?> <addon id="game.libretro" name="Libretro Compatibility" version="2.3.0" provider-name="Team Kodi"> <backwards-compatibility abi="1.0.0"/> <requires>@ADDON_DEPENDS@</requires> <extension point="kodi.gameclient" library_@PLATFORM@="@LIBRARY_FILENAME@"> </extension> <extension point="xbmc.addon.metadata"> <summary lang="de_DE">Libretro-Kompatibilitätsschicht für Kodi's Spiele API</summary> <summary lang="en_GB">Libretro compatibility layer for Kodi's Game API</summary> <description lang="de_DE">Dieses Add-On bietet einen Umwandler für Libretro-Kerne, mit dem sie als Spielsystem Add-On's für Kodi geladen und genutzt werden können.</description> <description lang="en_GB">This add-on provides a wrapper for libretro cores, allowing them to be loaded as game add-ons.</description> <platform>@PLATFORM@</platform> <license>GPL-2.0-or-later</license> <source>https://github.com/kodi-game/game.libretro</source> <assets> <icon>icon.png</icon> </assets> </extension> </addon>

garbear · 2021-07-19T06:07:27Z

xbmc/addons/kodi-dev-kit/include/kodi/versions.h

@@ -97,7 +97,7 @@
 #define ADDON_INSTANCE_VERSION_AUDIOENCODER_DEPENDS   "c-api/addon-instance/audio_encoder.h" \
                                                      "addon-instance/AudioEncoder.h"

-#define ADDON_INSTANCE_VERSION_GAME                   "2.1.0"
+#define ADDON_INSTANCE_VERSION_GAME                   "3.0.0"
 #define ADDON_INSTANCE_VERSION_GAME_MIN               "2.1.0"


You should also change MIN to 3.0.0

@garbear , On changing min to 3.0.0, game.libretro addon is disabled, I have done this yesterday also. This was the screenshot of the same.

Good, we want old versions of game.libretro to be rejected (we require a compatible version, otherwise we get your segfault, and segfaults should be avoided). I'll fire up my windows PC later today and figure out where the game.libretro build should be.

Sorry, I'm pressed hard at work and can't find a moment to spare for testing. From what I remember, the prepare script runs the CMake command and generates VS project files. Instead of building with cmake --install, open the .sln in VS, right click on the project, and compile it that way. Hunt down where it places the compiled files. Either copy them to addons/, or to simplify your life, make a symlink. That should cause game.libretro dependent on 3.0.0 to appear in addons/

Thanks @garbear , I have not done exactly the same but done something which I think is working, First step I do was to change game version in file versions.h to 3.0.0 from game.libetro.sln (I have changed it in kodi.sln but I realize today that it was 2.1.0 here in game.libretro.sln) then I have changed individual files at kodi-build/addons/game.libretro by the files I obtained after building game.libretro.sln. It seems to work right now, and if there will any problem I will comment here later on

Hello @garbear , On adding one new function, again I realise that same problem is coming, Kodi was searching for stale version, so I have changed game versions to 3.1.1, also tried 3.2.0 but it is not working(game.libretro is disabled now),so is there any specific pattern to which these version should be changed?

Yes there's a pattern, which is called "semver" or "semantic versioning". Semver defines the three numbers as major, minor, patch. Patch is no change to API, minor is backwards-compatible change, and major is backwards-incompatible change.

You can bump versions how you see fit for development, and we'll reset to 3.0 when we merge after the summer.

So you're still getting stale game.libretro? I hate that my workload is so high, I'd like to hop on windows and document how to make it not stale and always working. I'll do my best to get some documentation written after work tonight.

garbear · 2021-08-06T19:57:51Z

@Shardul555 Can you hit "Edit" next to the PR title and change the base target branch to feature_achievements?

xbmc/addons/kodi-dev-kit/include/kodi/c-api/addon-instance/game.h

xbmc/cores/RetroPlayer/cheevos/Cheevos.cpp

NikosSiak · 2021-08-08T11:50:29Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.cpp

+      cheevoid_list.push_back(data[PATCH_DATA][ACHIEVEMENTS][i][CHEEVO_ID].asUnsignedInteger());
+      cheevo_memaddr.push_back(data[PATCH_DATA][ACHIEVEMENTS][i][MEM_ADDR].asString());
+      cheevo_title.push_back(data[PATCH_DATA][ACHIEVEMENTS][i][CHEEVO_TITLE].asString().c_str());
+      cheevo_badge.push_back(data[PATCH_DATA][ACHIEVEMENTS][i][BADGE_NAME].asString().c_str());


you should store the achievements in one unordered map with the cheevo_id as key and the whole achievement as a value so you won't do a for loop each time you want to access the achievement's data

NikosSiak · 2021-08-08T11:52:21Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.cpp

+  if (cheevo_id !=0 && achievement_map.find(cheevo_id) == achievement_map.end())
+  {
+    achievement_map[cheevo_id] = "triggered";


here you are searching the key twice (once with find and once with [], you could store the value before the if statement.

NikosSiak · 2021-08-08T11:55:47Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.cpp

+    char url[URL_SIZE];
+    strcpy(url, achievement_url);


you shouldn't use char arrays/pointers to store a string, you can use the string constructor which takes a char * and copies its contents to a new string

NikosSiak · 2021-08-08T11:57:21Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.h

@@ -12,6 +12,9 @@

 #include <map>
 #include <string>
+#include <vector>
+#include<unordered_map>


minor: add a space before <

NikosSiak · 2021-08-08T12:00:03Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.h

@@ -12,6 +12,9 @@

 #include <map>
 #include <string>
+#include <vector>
+#include<unordered_map>
+using std::vector;


try to avoid importing unrelated namespaces outside of the ones this class is declared within

NikosSiak · 2021-08-08T12:00:50Z

xbmc/cores/RetroPlayer/cheevos/Cheevos.h

+  vector<unsigned> cheevoid_list;
+  vector<std::string> cheevo_memaddr;
+  vector<const char*> cheevo_title;
+  vector<const char*> cheevo_badge;


instance variables must start with m_ prefix

Shardul555 · 2021-08-14T05:01:22Z

@Shardul555 Can you hit "Edit" next to the PR title and change the base target branch to feature_achievements?

Sorry @garbear , I was not active on the github account so I missed this message , but yes I will do it till tomorrow

garbear · 2021-08-22T00:31:34Z

xbmc/interfaces/swig/CMakeLists.txt

@@ -14,11 +14,14 @@ function(generate_file file)
  if(CLANGFORMAT_FOUND)
    set(CLANG_FORMAT_COMMAND COMMAND ${CLANG_FORMAT_EXECUTABLE} ARGS -i ${CPP_FILE})
  endif()
+  if(Java_VERSION_MAJOR GREATER 8)
+    set(JAVA_OPEN_OPTS --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.regex=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED  --add-opens java.base/java.net=ALL-UNNAMED)


Should this be upstreamed?

garbear · 2021-08-22T00:32:09Z

tools/buildsteps/windows/make-addons.bat

@@ -98,7 +98,7 @@ IF "%store%" NEQ "" (
 )

 rem execute cmake to generate makefiles processable by nmake
-cmake "%ADDONS_PATH%" -G "NMake Makefiles" ^
+cmake "%ADDONS_PATH%" -G "Visual Studio 16 2019" -A x64 ^


Note: we'll drop this before merge.

I've upstreamed this change as part of xbmc#20075

garbear · 2021-08-22T00:39:39Z

@Shardul555 Aside from the two files I commented on, can you squash everything into a single commit? Besides needing Kodi to compile between all commits for bisection, for major add-on version bumps, I try to keep all the code for a feature together.

After squashing, I would expect the first commit to the feature addition, followed by the two commits I commented on. The expectation is that the "temporary" commits will be upstreamed or discarded before merge.

Finally, your squashed commit should follow good commit message style. A possible commit title is Add Achievements in RetroPlayer, short and simple. Extra detail can go in a description.

garbear · 2021-10-12T23:24:03Z

Technically, review should have continued in this PR instead of creating a new, clean one, but it's not a problem because PRs can be linked. Can you close this PR and link to the new one?

Hedda · 2022-01-25T15:10:50Z

Any updates/news on kodi-game/game.libretro#73 and #126 or #127 now that kodi-game/game.libretro#67 has been merged?

garbear · 2022-01-25T23:53:15Z

Keep an eye on master!

garbear · 2024-05-10T06:37:23Z

Superseded by #127

Shardul555 requested a review from garbear as a code owner July 11, 2021 17:00

Shardul555 force-pushed the gsoc2021 branch from 6618508 to ab5127d Compare July 17, 2021 17:22

garbear reviewed Jul 19, 2021

View reviewed changes

Shardul555 force-pushed the gsoc2021 branch from f7b719a to d5958de Compare August 4, 2021 18:03

garbear force-pushed the retroplayer-19.1 branch from cc6171f to 9c3ea01 Compare August 6, 2021 03:33

NikosSiak reviewed Aug 8, 2021

View reviewed changes

garbear force-pushed the retroplayer-19.1 branch from 9c3ea01 to cc6171f Compare August 13, 2021 20:08

Shardul555 changed the base branch from retroplayer-19.1 to feature_achievements August 14, 2021 15:54

Shardul555 changed the title ~~Gsoc2021, PR for debug purpose~~ Adding feature of Achievements in RetroPlayer Aug 17, 2021

Shardul555 mentioned this pull request Aug 20, 2021

Implementing support for Achievements in RetroPlayer kodi-game/game.libretro#73

Open

Shardul555 force-pushed the gsoc2021 branch from e556e2e to fd22107 Compare August 21, 2021 08:36

garbear reviewed Aug 22, 2021

View reviewed changes

garbear changed the title ~~Adding feature of Achievements in RetroPlayer~~ RetroPlayer: Achievements Oct 9, 2021

RetroPlayer: Add Achievements

314bb47

Shardul555 force-pushed the gsoc2021 branch from fd22107 to 314bb47 Compare October 12, 2021 16:38

garbear changed the title ~~RetroPlayer: Achievements~~ RetroPlayer: Add Achievements Oct 12, 2021

Shardul555 mentioned this pull request Feb 1, 2023

Game API v4.0.0: Achievement support for RetroPlayer kodi-game/game.libretro#104

Open

garbear mentioned this pull request Feb 2, 2023

Retrolayer: Add Achievements xbmc/xbmc#22652

Open

13 tasks

garbear closed this May 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RetroPlayer: Add Achievements #126

RetroPlayer: Add Achievements #126

Shardul555 commented Jul 11, 2021 •

edited

Loading

garbear commented Jul 17, 2021

Shardul555 commented Jul 18, 2021

garbear commented Jul 18, 2021 •

edited

Loading

Shardul555 commented Jul 18, 2021 •

edited

Loading

garbear Jul 19, 2021

Shardul555 Jul 19, 2021 •

edited

Loading

garbear Jul 19, 2021

garbear Jul 21, 2021

Shardul555 Jul 21, 2021 •

edited

Loading

Shardul555 Jul 28, 2021 •

edited

Loading

garbear Jul 28, 2021

garbear commented Aug 6, 2021 •

edited

Loading

NikosSiak Aug 8, 2021

NikosSiak Aug 8, 2021

NikosSiak Aug 8, 2021

NikosSiak Aug 8, 2021

NikosSiak Aug 8, 2021

NikosSiak Aug 8, 2021

Shardul555 commented Aug 14, 2021 •

edited

Loading

garbear Aug 22, 2021

garbear Aug 22, 2021

garbear Aug 25, 2021

garbear commented Aug 22, 2021 •

edited

Loading

garbear commented Oct 12, 2021

Hedda commented Jan 25, 2022 •

edited

Loading

garbear commented Jan 25, 2022

garbear commented May 10, 2024

RetroPlayer: Add Achievements #126

RetroPlayer: Add Achievements #126

Conversation

Shardul555 commented Jul 11, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

Checklist:

garbear commented Jul 17, 2021

Shardul555 commented Jul 18, 2021

garbear commented Jul 18, 2021 • edited Loading

Shardul555 commented Jul 18, 2021 • edited Loading

Choose a reason for hiding this comment

Shardul555 Jul 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shardul555 Jul 21, 2021 • edited Loading

Choose a reason for hiding this comment

Shardul555 Jul 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garbear commented Aug 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shardul555 commented Aug 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garbear commented Aug 22, 2021 • edited Loading

garbear commented Oct 12, 2021

Hedda commented Jan 25, 2022 • edited Loading

garbear commented Jan 25, 2022

garbear commented May 10, 2024

Shardul555 commented Jul 11, 2021 •

edited

Loading

garbear commented Jul 18, 2021 •

edited

Loading

Shardul555 commented Jul 18, 2021 •

edited

Loading

Shardul555 Jul 19, 2021 •

edited

Loading

Shardul555 Jul 21, 2021 •

edited

Loading

Shardul555 Jul 28, 2021 •

edited

Loading

garbear commented Aug 6, 2021 •

edited

Loading

Shardul555 commented Aug 14, 2021 •

edited

Loading

garbear commented Aug 22, 2021 •

edited

Loading

Hedda commented Jan 25, 2022 •

edited

Loading