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

Qt: Allow loading custom pnach patches and bundled patches simultaneously #10322

Merged

Conversation

Daniel-McCarthy
Copy link
Contributor

Description of Changes

Implements #10242

Allow loading patch files from /patches folder and the bundled patches.zip file simultaneously.

  • If a patch in the /patches folder and the patches.zip have the same name, it loads the /patches folder version instead.
  • If there are no collisions, all patches from each are loaded.
  • If an unlabeled patch from the /patches folder is loaded, no bundled patches will be loaded. (Since we can't guarantee they won't collide with a bundled patch)

Also, updates the patches UI to display a warning when unlabeled patches are loaded and causing the bundled patches to not display in the Patches Settings window.

Rationale behind Changes

To allow users to use the bundled no-interlacing & widescreen patches with their custom patches. As long as there's no collisions, there's no need to stop them from using both.

Suggested Testing Steps

  • Find a game that has bundled patches.
  • Create a patch file with an unlabelled patch and a labeled patch for that game.
  • Since there's an unlabeled patch, verify only the labeled /patches patch is shown in Patch Settings window.
  • Comment out the unlabeled patch and save the patches file.
  • Reload Patches to verify that the custom labeled patch and the bundled patches are showing.
  • Rename the custom labeled patch to the name of one of the bundled patches and save.
  • Reload Patches and verify that the folder patch is loaded instead of the bundled patch of the same name. (Easiest way to do this is by adding a description=Should override bundled patch field to the patch).
In my case, I tested this in GTA 3 with this pnach file:
// Unlabeled patch - this should prevent bundled patches from loading unless commented out
patch=2,EE,2027CEA4,extended,28420001

// Rename this to a name that does not collide with a bundled patch to test, then name it the same as a bundled patch to test overriding it [Widescreen 16:9] description=FromFile, should get preference // set cutscene camera pos patch=2,EE,20251628,extended,44800000

Currently if one has patches from the patch folder, no bundled patches from the .zip file will be displayed.

This changes it so that we can display both the folder patches AND the .zip bundled patches as long as their names don't collide. If there's a collision, the folder patch will override the bundled patch.

Also, if an unlabeled patch is loaded from the folder then all bundled patches will be hidden (like they did prior) since we can't guarantee there's no collision.
Bundled patches don't display when unlabeled patches are loaded (since we can't guarantee they don't collide). This warns the user that bundled patches are hidden when unlabeled patches are loaded. (The warning hides when no unlabeled patches are loaded)
@Daniel-McCarthy Daniel-McCarthy changed the title Qt: Allow custom patches with default patches Qt: Allow loading custom pnach patches and bundled patches simultaneously Nov 26, 2023
Fixes syntax error with console write format to display warning when bundled patches are hidden due to unlabeled patches being loaded.
Copy link
Contributor

@CookiePLMonster CookiePLMonster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea looks good, just leaving notes about some stylistic details. Untested.

pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@CookiePLMonster CookiePLMonster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two more comments, the implementation looks optimal.

pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
Adds a better worded log statement for informing the user that a patch with a duplicate name was found and not loaded to avoid conflict.

The common expected reason this can happen is a bundled patch having the same name as a /patches/ pnach patch having the same name. By default it will prioritize the folder patch over the bundled patch.

Makes it more clear/less confusing in phrasing.
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
pcsx2/Patch.cpp Outdated Show resolved Hide resolved
Addresses feedback to use consistent naming conventions as most variables are using snake_case. Also no longer passes string_view by reference as per feedback.
Fixed unintentionally non-sensical wording in log warning to inform the user that a patch was skipped over since a patch with the same name had already been loaded.
Copy link
Contributor

@CookiePLMonster CookiePLMonster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested on my end, but your testing steps sound reasonable to me.

@stenzek stenzek merged commit f0bf525 into PCSX2:master Dec 4, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants