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

Embed GameDB and widescreen patches into libretro core #47

Merged
merged 7 commits into from
Jun 9, 2021

Conversation

covey-j
Copy link
Contributor

@covey-j covey-j commented Jun 7, 2021

  • The libretro core can now utilize GameDB features like game fixes without needing to copy any files from the standalone
  • GameDB was updated to use a YAML-based implementation, modified from the standalone's approach
  • Enable Widescreen Patches no longer needs a copy of cheats_ws.zip to function
  • If cheats_ws.zip is present, it's now ignored. Users can still supply their own widescreen patches using the cheats_ws directory
  • As a side effect, enabling widescreen patches no longer causes long boot times, partially resolving Function _ApplySettings in gui-libretro/AppCoreThread.cpp called multiple times #36

@bslenul
Copy link
Contributor

bslenul commented Jun 7, 2021

Would it be possible to include portable.ini as well? I feel like this could be very confusing (and could potentially mess with standalone) if people are not setting up the pcsx2 folder properly, looks like it uses C:\User\[user]\Documents\PCSX2\ atm if it's missing.

@inactive123
Copy link
Contributor

inactive123 commented Jun 7, 2021

@bslenul Personally I'm a big fan of not requiring the user to dump the entire PCSX2 asset structure inside system, that seems like such a user inconvenient way of going about things in the future that is going to really limit the userbase for this core. Users really expect a core like this to work out of the box without having to drag in a dozen asset files from standalone.

Sure, there could always be a fallback to having it use all these system-supplied files, but perhaps that should be explicitly hidden behind a core option setting, and by default it should just use the baked-in assets instead.

@bslenul
Copy link
Contributor

bslenul commented Jun 7, 2021

Well I feel like this PR is a really good start for that :D

From https://github.com/libretro/pcsx2/tree/main/bin , what's really needed in the system/pcsx2/ folder anyway?

  • Langs/, I'm guessing this is only useful for standalone?
  • docs/, docs for standalone, idk anything about license tho, does GPL.html have to be bundled?
  • shaders/, probably useless for RetroArch?
  • GameIndex.dbf, won't be needed anymore thanks to this PR.
  • PCSX2_keys.ini.default, with how RetroArch works, I'm guessing this is completely ignored?
  • cheats_ws.zip, this is optional but can be baked in if I understood correctly?
  • portable.ini, absolutely needed or else it will create the config/memcard files and all somewhere else (C:\User\[user]\Documents\ on Windows for example), like I said in my comment above.
  • run_spu2_replay.cmd, I have no idea what this is.

So if portable.ini was included (or maybe there's another way to run the emulator in portable mode), with this PR only BIOS would be required to be provided by the user, and the widescreen cheats if the user wants to use them 👍

@bslenul
Copy link
Contributor

bslenul commented Jun 7, 2021

I wanted to test the PR, but the build fails for me, here's the 2 last lines:

Generating cheats_ws.h from cheats_ws.zip '-i' is not recognized as an internal or external command, operable program or batch file.

G:\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(240,5): error MSB8066: custom build for 'G:\msys64\home\B-S\pcsx2\build\CMakeFiles\73ba57b7172d8d0860a81ab134f179b3\cheats_ws.h.rule;G:\msys64\home\B-S\pcsx2\build\CMakeFiles\73ba57b7172d8d0860a81ab134f179b3\GameIndex.h.rule' has stopped. Code 9009. [G:\msys64\home\B-S\pcsx2\build\pcsx2\pcsx2_libretro.vcxproj]

Did I do something wrong? I'm using these 2 commands as usual (after updating submodules needed by the PR):

cmake .. -DLIBRETRO=ON -DCMAKE_BUILD_TYPE=Release
cmake --build . --target pcsx2_libretro --config Release

I can build the "main" branch with no issue.

@covey-j
Copy link
Contributor Author

covey-j commented Jun 7, 2021

Well I feel like this PR is a really good start for that :D

From https://github.com/libretro/pcsx2/tree/main/bin , what's really needed in the system/pcsx2/ folder anyway?

  • Langs/, I'm guessing this is only useful for standalone?
  • docs/, docs for standalone, idk anything about license tho, does GPL.html have to be bundled?
  • shaders/, probably useless for RetroArch?
  • GameIndex.dbf, won't be needed anymore thanks to this PR.
  • PCSX2_keys.ini.default, with how RetroArch works, I'm guessing this is completely ignored?
  • cheats_ws.zip, this is optional but can be baked in if I understood correctly?
  • portable.ini, absolutely needed or else it will create the config/memcard files and all somewhere else (C:\User\[user]\Documents\ on Windows for example), like I said in my comment above.
  • run_spu2_replay.cmd, I have no idea what this is.

So if portable.ini was included (or maybe there's another way to run the emulator in portable mode), with this PR only BIOS would be required to be provided by the user, and the widescreen cheats if the user wants to use them 👍

I believe the only things useful from that list for the libretro core after this PR are portable.ini and cheats_ws.zip. I'm maybe halfway done with baking in the latter. As for portable.ini, it'd be better to refractor AppUserMode as to not need it in the first place. In theory it should run without needing any extra files aside from the bios.

As far as falling back on assets from standalone, I think the way to go here is to use the baked-in cheats_ws.zip unless there are user-supplied widescreen cheats in the cheats_ws directory.

@covey-j
Copy link
Contributor Author

covey-j commented Jun 7, 2021

I wanted to test the PR, but the build fails for me, here's the 2 last lines:

Generating cheats_ws.h from cheats_ws.zip '-i' is not recognized as an internal or external command, operable program or batch file.

G:\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(240,5): error MSB8066: custom build for 'G:\msys64\home\B-S\pcsx2\build\CMakeFiles\73ba57b7172d8d0860a81ab134f179b3\cheats_ws.h.rule;G:\msys64\home\B-S\pcsx2\build\CMakeFiles\73ba57b7172d8d0860a81ab134f179b3\GameIndex.h.rule' has stopped. Code 9009. [G:\msys64\home\B-S\pcsx2\build\pcsx2\pcsx2_libretro.vcxproj]

Did I do something wrong? I'm using these 2 commands as usual (after updating submodules needed by the PR):

cmake .. -DLIBRETRO=ON -DCMAKE_BUILD_TYPE=Release
cmake --build . --target pcsx2_libretro --config Release

I can build the "main" branch with no issue.

So the way the baking in works is via xxd -i, which takes the source cheats_ws.zip and spits out a C header file containing an unsigned char array of the hex dump of cheats_ws.zip.

Seems like your issue is either you don't have xxd (seems unlikely) or it's reading xxd and -i separately (seems very likely). Could you try going into pcsx2/CMakeLists.txt, finding the two add_custom_command statements I added, and putting the xxd -i command in quotes? That should hopefully fix it for you. Let me know if it does

EDIT: Disregard the above; add in VERBATIM, as so:

add_custom_command(
	OUTPUT ${db_res_bin}/cheats_ws.h
	COMMAND ${XXD} -i cheats_ws.zip cheats_ws.h
	COMMAND mv cheats_ws.h ${db_res_bin}/cheats_ws.h
	WORKING_DIRECTORY ${db_res_src}
	COMMENT "Generating cheats_ws.h from cheats_ws.zip"
	DEPENDS ${db_res_src}/cheats_ws.zip
	VERBATIM
)
add_custom_command(
	OUTPUT  ${db_res_bin}/GameIndex.h
	COMMAND ${XXD} -i GameIndex.yaml GameIndex.h
	COMMAND mv GameIndex.h  ${db_res_bin}/GameIndex.h
	WORKING_DIRECTORY  ${db_res_src}
	COMMENT "Generating GameIndex.h from GameIndex.yaml"
	DEPENDS  ${db_res_src}/GameIndex.yaml
	VERBATIM
)

@bslenul
Copy link
Contributor

bslenul commented Jun 7, 2021

Generating cheats_ws.h from cheats_ws.zip
'" -i cheats_ws.zip cheats_ws.h"'

Looks like ${XXD} is empty, replacing ${XXD} -i with xxd -i worked however.

edit: OK, I'll try VERBATIM as well!

@covey-j
Copy link
Contributor Author

covey-j commented Jun 7, 2021

Generating cheats_ws.h from cheats_ws.zip
'" -i cheats_ws.zip cheats_ws.h"'

Looks like ${XXD} is empty, replacing ${XXD} -i with xxd -i worked however.

Ohhhhh, good catch. I initially set ${XXD} in a statement that checked if xxd was installed but removed it for a bit while debugging CMake. Forgot to add it back in!

@inactive123
Copy link
Contributor

Alright, you guys can let me know when this is ready to be merged.

Also, it'd be awesome when we can add a list for the no-interlacing patches. I already mentioned this to @covey-j as something I was personally interested in. I do feel that for the no-interlacing patches, there should be a core option so that if the user doesn't want it to be auto-applied (for games that are in the list), he can turn it off.

As for where we can get the no-interlacing patches from, there is a big list here -

https://forums.pcsx2.net/Thread-No-interlacing-codes

Note - the no-interlacing stuff is something for a separate PR, it's not necessary for this PR.

@bslenul
Copy link
Contributor

bslenul commented Jun 7, 2021

OK so I've tried the PR, and maybe I'm missing something but the database file doesn't seem to be used at all anymore 😅 The best example I can give is Jak and Daxter, this is with main branch + database file in pcsx2/ dir:

image

image

And this is with the PR (with or without the database file):

image

image

@covey-j
Copy link
Contributor Author

covey-j commented Jun 7, 2021

OK so I've tried the PR, and maybe I'm missing something but the database file doesn't seem to be used at all anymore 😅 The best example I can give is Jak and Daxter, this is with main branch + database file in pcsx2/ dir:

Thanks for testing. I'll try to replicate the issue and see what I find. If I had to guess, it's either

  1. I forgot to stage one of the needed changes last night (I'm still working on stuff locally for the widescreen patches)
  2. Jak & Daxter had a game fix removed at some point -- it should be noted that the GameIndex.yaml I used is the same one as in the standalone. As they've made accuracy improvements to the emulator, they've removed some of the game fixes that used to be necessary for standalone but are still necessary for us. I could use some help tracking these down!

@covey-j
Copy link
Contributor Author

covey-j commented Jun 7, 2021

Yep, there was a bug preventing the GameDB from loading. The log showed 0 games loaded. It should be fixed now -- I tested it out and it loaded all 10372 games in the database, as expected. I also tried a game that needed the VU0KickstartHack (same as what Jak and Daxter needs, though I didn't test that game specifically) and it correctly found and applied the hack.

@bslenul Could you try again with my latest commit?

@covey-j covey-j changed the title Embed GameDB into libretro core Embed GameDB and widescreen patches into libretro core Jun 8, 2021
@covey-j
Copy link
Contributor Author

covey-j commented Jun 8, 2021

OK, so I have the GameDB and embedded widescreen patches working on my end, both on Windows 10 and on Xbox. Could somebody else test the latest commit and make sure it's working on their end too?

Important: make sure the cheats_ws folder is empty while testing. Patches in cheats_ws take precedence over the embedded widescreen patches

@bslenul
Copy link
Contributor

bslenul commented Jun 8, 2021

Deleted the database and the ws cheats (both the .zip and the folder), worked like a charm on my end (Windows 10)! Tested with 4-5 different games.

Thank you for your work, this is a really nice addition!

@covey-j
Copy link
Contributor Author

covey-j commented Jun 8, 2021

@twinaphex OK, it should be ready for merge.

Some notes for later:

  • The GameDB implementation is close to what the standalone does, but I commented out a few lines related to the InstantVU1SpeedHack. These should be reintroduced when VU: Synchronise VU1, added speedhack for old behaviour PCSX2/pcsx2#3998 is backported
  • As I mentioned before, resources/GameIndex.yaml is now in line with the standalone. This means there may be a handful of patches that were removed, but are still needed for the libretro core. These will eventually be unneeded on our end too as things from standalone get backported, so I don't think it's worth tracking all these down. If users encounter issues because of these missing game fixes, we can either add them back into resources/GameIndex.yaml or we can backport the changes made to standalone that made those game fixes unnecessary. Preferably the latter
  • The archive/compression-related files from wxwidgets3.0-libretro should be safe to remove now, since they were only used for extracting cheats_ws.zip and compressing/decompressing save states (when save states get re-introduced into this core, we can handle the compression/decompression without wx). Probably better to do that separately from this PR in case that breaks anything
  • After this gets merged, the only thing left to do to make the core run "out of the box" (aside from placing the bios) is to refactor AppUserMode to either create the needed folder structure in system/pcsx2 (the folders can be empty; they just need to exist. Only file that's actually needed is portable.ini) or no longer require those files/folders. Preferably the latter, though we should retain the cheats_ws and cheats directories so that users can supply their own custom patches if they want

@inactive123
Copy link
Contributor

Very cool, thanks for all your hard work and effort. I have read your post, thanks for informing me what the followup steps are going to be.

@inactive123 inactive123 merged commit 4d07a1b into libretro:main Jun 9, 2021
@inactive123
Copy link
Contributor

Hmm , there are some build failures now -

hopefully these can be resolved -

https://git.libretro.com/libretro/pcsx2/-/jobs/464918

@covey-j
Copy link
Contributor Author

covey-j commented Jun 9, 2021

OK, I made xxd optional in #50. Let me know if this resolves the build issue. I couldn't find an elegant way in CMake to make it update GameIndex.h after changes are made to GameIndex.yaml, but that's a minor inconvenience that can be dealt with later. More important to get this playing nicely with buildbot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants