RetroPlayer: Add a cheat engine - #157
sunlollyking wants to merge 2 commits into
Conversation
|
@garbear again opening against your master and i'll keep maintaining until you think it's ready to include in master. All tested, working :) |
|
How about providing cht files in a game resource add-on? It doesn't need to be a binary add-on. The reason why game.shader.presets has any C++ code at all is because we ripped it from retroarch, which is GPLv3, hence the DLL boundary. If we had AI back in 2017 we would have just hand rolled all the code into master. |
|
This is an awesome feature! Upstreaming might take a little bit, as we probably won't branch for v23 for a little while, and the team is discussing ways to fast-track big AI features like this into v23. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c44b66409e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
c7932a9 to
4066342
Compare
|
Good idea, and no binary needed as you say. I have folded it into this PR: the engine now searches every installed game resource add-on as well as the folder setting, about forty lines, because Kodi already has kodi.resource.games, CGameResource and resource://. No change to the parsing or the dialog, which I think says the shape was right. I would keep the folder setting either way though. Anyone with their own cht files, a full libretro checkout, or a system no add-on covers still wants to point at it, same as RetroArch does. On the add-on itself, the data is the awkward part rather than the plumbing. The cht folder is 28,308 files and 250 MB, with DS alone at 108 MB over 4,204 files, so one add-on for everything is out. 27 of the 45 systems are a megabyte or less though and only 8 are over 5 MB, so per system is mostly tiny and it is a handful of consoles causing it. Licence is fine, CC-BY-SA-4.0, which the add-on rules take. Also rebased onto master while I was in there, and picked up an include ordering fix clang-format wanted. |
c92cdac to
852e101
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 852e101442
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
89146e8 to
4940b8c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4940b8c0f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4940b8c to
f457e6b
Compare
|
@garbear is there a chance we could open this for 22 - or do you think too big for the RC remit? |
|
V22 is possible due to the line counts: 1K lines added, 2 removed. Red in a PR is a big warning flag that existing code is touched, and this PR has barely any red. If we're going to upstream this, we'll need to do it ASAP. I'll see what I can do about review today. |
|
I'll redo the dialog, then cheats will just be adding a button |
|
Ok. I'm making changes aswell in the next 10 so hopefully we don't crash |
755d313 to
4c04bb8
Compare
|
Cool that's me all done. Everything should be working now. Did some manual tests ! Dialog definitely needs that polish |
|
Here's your first commit: http://github.com/garbear/xbmc/commit/new-game-osd Then do: --- a/addons/skin.estuary/xml/GameOSD.xml
+++ b/addons/skin.estuary/xml/GameOSD.xml
@@ -14,7 +14,8 @@
!Window.IsActive(InGameSaves) +
!Window.IsActive(GameAgents) +
!Window.IsActive(GameDiscManager) +
- !Window.IsActive(GameAchievements)
+ !Window.IsActive(GameAchievements) +
+ !Window.IsActive(GameCheats)
</visible>
<include>Visible_Fade</include>
<control type="group" id="10">
@@ -143,6 +144,13 @@
<description>Always show Achievements; its dialog explains sign-in requirements and missing achievement sets.</description>
<onclick>ActivateWindow(GameAchievements)</onclick>
</include>
+ <include content="GameOSDMenuButton">
+ <param name="id" value="1118" />
+ <param name="label" value="$LOCALIZE[35320]" />
+ <param name="icon" value="osd/fullscreen/buttons/cheats.png" />
+ <visible>RetroPlayer.HasCheats</visible>
+ <onclick>ActivateWindow(GameCheats)</onclick>
+ </include>
<include content="GameOSDMenuButton">
<param name="id" value="1115" />
<param name="label" value="$LOCALIZE[13007]" />(untested but testing now) |
|
The design is roughly this: |
|
Sorry, Astra High let us down. Bumping to Astra Ultra, it'll nail the new OSD. |
|
Fab let me know when you're done and I'll take the lead back over with commit ordering and rebasing. Don't want to crash into you |
|
K, got it workin good: 8e7307a No change to the patch I posted. I also made the icon smaller. It matches the others better now IMO. 
Before: 
After: 
|
4c04bb8 to
5e827fd
Compare
|
Cool order is all good now. Apologies for the 11 commits I think we just need your master to catch up |
|
OK, I pushed the latest master |
Replace the duplicated Disc and non-Disc layouts with reusable buttons in a centered grouplist. Visible controls determine the menu height together with its header. Draw one continuous background to avoid row seams at fractional GUI scales. Size the optional background from the header and menu controls, as the context menu does for its dynamic background. Preserve existing actions, help mode, and navigation. Remove the obsolete list and scrollbar machinery.
5e827fd to
424892e
Compare
|
Did you replace the icon? I think the current one is a bit too big: 
I pasted a slightly smaller one above. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 424892e775
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
I've taken care of upstreaming the refactored game OSD: xbmc#29278 If you open this PR against master before that merges, include that commit (it changed). When xbmc#29278 is merged, we'll rebase it out. |
Are those "Weapon modifier [slot]" entries supposed to be toggle buttons? |
|
Sorting, structure, and organization, leaves something to be desired. 
|
424892e to
9bb2c43
Compare
Cheats are read from libretro .cht files named after the game and offered as toggles in a dialog one level below the game OSD. The row only appears for a client that implements the cheat entry points. Only the cheats that are switched on are sent, and they are sent whole after a reset, because a cheat is identified by the slot it was given and several cores ignore the enabled flag they are handed. Files are looked for in the folder the player points the setting at, and in every game resource add-on they have installed. Cheat files are data, so an add-on carrying them needs no binary of its own and installs and updates through the repository like anything else. The setting is asked first and stays either way, for a database of somebody's own or a system no add-on covers. A cheat add-on ships one zip per system rather than tens of thousands of loose files, so an archive is searched like a folder. Kodi reads it through the same calls either way. The dialog offers to fetch that add-on when it is missing or switched off, the way the video filter offers shader presets, since the player has no other way of learning the database exists. That is also why the OSD row now shows while the offer stands rather than only once cheats have been found, and why the folder setting has moved to expert: it is the escape hatch now, not the way in. The cheats belong to the game client, as a subsystem beside its achievements, input and streams. The pack and the switches are that client's, and go when it does. Nothing global holds a pointer back to a client, so nothing here assumes only one can ever be loaded. The client's own lock is taken before the cheat state's on every path, which is the order a game being closed already holds them in, and the client cannot be torn down between the codes of a batch because closing it takes that same lock. Descriptions are written with the HTML entities that came with them from the web, so they are resolved through Kodi's own table rather than shown as written: a quoted button arrives as "A", and 428 of the database's 28,308 files carry one. The dialog keeps its own copy of the settings layout so the column beside the list can say what is switched on, which the shared one has no control for. The player would otherwise have to scroll back through several hundred rows to see what they had turned on.
9bb2c43 to
a5c53c0
Compare
"Weapon modifier — should those be toggles?" — All chats are booleans we are at mercy of jow an author describes a cheat "Other groupings we could take?" — I thought about going alphabetical but then we do lose what the author of the cheats wanted. Unfortunately there's very little data to sort by |
|
Good work! I saw the upstream PR. Only comments are to update the description:
|
|
For screenshots, get a good one of the OSD with Cheats highlighted, then show the dialog, and put a pretty game (and maybe not IP-contentious) in the background. EDIT: I noticed you did this perfectly in your description here, nice! Though the ones on master could use no debugging text and a prettier game in the background. |
3 participants
Description
Cheats are read from libretro
.chtfiles and offered as switches in a dialog one level below the game OSD.The Cheats row only appears when a cheat file was found for the game being played, so a system with no cheats never offers a list with nothing in it. That is driven by a new boolean condition,
RetroPlayer.HasCheats, which skins can use themselves. The row is added to both game OSD variants in Estuary, the cartridge one and the disc one, along with a new icon.A cheats folder is set under Settings > Games. The libretro cheat database is published a folder per system, so the setting can point straight at the database itself, and a folder of loose files the user has gathered works just as well. The file is matched by the game's name, which is how that database is published.
Two things about how the codes are sent are worth calling out, because both look like the harder option until you hit them:
enabledflag they are handed and several ignore it outright. fceumm adds every code it is given, so sending a disabled cheat applies it. With a full database loaded that shows up as a corrupted picture rather than as an error.Two limits, both from measurement rather than taste. A cheat file is read to 16 MB, and at most 1024 cheats are taken from one. The median file in the libretro database holds 6 cheats and the largest holds 30911; past a thousand nobody is reading down the list, and the ones beyond cost frames and memory to carry. At thirty thousand the dialog still opens, but the frame rate halves and the process grows by a few hundred megabytes. A file over the limit gives its first cheats rather than none at all, and says so in the log.
A cheat name longer than 55 characters is truncated in its row, because past that the text runs under the switch. The full text is handed to the settings description control, so a skin that lays out a description area shows it. Estuary's
DialogSettings.xmlhas no such control, so it is not visible in the default skin today.Motivation and context
The Game API has carried
CheatReset()andSetCheat()for a long time and nothing in Kodi has ever called them. Game clients already implement them, so the emulator side of this works today and only the frontend was missing.How has this been tested?
Linux build, against
game.libretro.fceummand Super Mario Bros. with the libretro cheat database, which gives that game 711 usable cheats out of 725 declared.What is the effect on users?
A Cheats item appears in the game OSD while playing a game that has a cheat file, listing the cheats that file declares as switches that can be turned on and off during play. Nothing changes for a game with no cheat file.
Screenshots (if appropriate):
Types of change
Checklist: