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

Package *.lck core lock file-extension. #16556

Merged

Conversation

Misunderstood-Wookiee
Copy link
Contributor

@Misunderstood-Wookiee Misunderstood-Wookiee commented May 19, 2024

Description

Re-introduce .lck file-type extension into .vcxproj & vcxproj.filters for RetroArch-msvcUWP.

Example: For demonstrative purpose.
image
The LCK file will be packed with the .appx package when published, which is a use case for locking cores included with specialised builds by default.

Thoughts
I am not entirely sure what reason there was to not have the lock file included with UWP configuration as this is particularly useful with Xbox specific builds and required to protect custom cores included in special builds in order for those cores to be skipped by Online Updater. You can still unlock the core manually from the Cores Settings page.

Related Issues

No Issue

Related Pull Requests

None

Includes *.lck alongside the rest of the cores/$platform/cores so that bundled cores can be locked at time of publish.
@hizzlekizzle
Copy link
Contributor

Hmm, Android failure appears unrelated.

@LibretroAdmin
Copy link
Contributor

Why are you doing this though for this core? I don't understand why we need to prevent it from getting updated, this seems undesirable.

@hizzlekizzle
Copy link
Contributor

the PR just allows lock files to be included, right? not specifying scummvm, specifically?

But yeah, I agree with LibretroAdmin that ScummVM core should be update-able, right? Some, like mupen-via-ANGLE, need to be locked, though.

@Misunderstood-Wookiee
Copy link
Contributor Author

Misunderstood-Wookiee commented May 20, 2024

the PR just allows lock files to be included, right? not specifying scummvm, specifically?

But yeah, I agree with LibretroAdmin that ScummVM core should be update-able, right? Some, like mupen-via-ANGLE, need to be locked, though.

Correct, this PR is not core specific, it simply provides the necessary change so that the UWP will compile adding any included core locks that you have added for any specific reason. On it's own this does NOT pre-lock anything.

Exactly like the Angle UWP builds which require a specific Mupen core I needed to lock ScummVM as the latest version on Mesa seems break on RA 1.18 however some testers found a version of it which worked so in the example image that is what you are seeing, however it not related to what this PR has changed.

This is what prompted me to lock the core in my specific build.
image
image

I had to make the changes this PR makes in order to compile the build with the core locked. I just thought it was worth making the changes here as without them the package won't include any lock files so if you try to compile with locked cores the cores get added but will not be locked. Ideally we would fix the core but for now I simply have included the older version and locked it, but no this request won't lock the core it simply makes the change necessary to compile and add the core lock file if they are present.

@Misunderstood-Wookiee
Copy link
Contributor Author

Misunderstood-Wookiee commented May 20, 2024

Hmm, Android failure appears unrelated.

Correct, Android and any other build is un-related, this PR only effects the UWP version.

@Misunderstood-Wookiee
Copy link
Contributor Author

Misunderstood-Wookiee commented May 20, 2024

Why are you doing this though for this core? I don't understand why we need to prevent it from getting updated, this seems undesirable.

I am sure you aware by now that Xbox ports are bit of a unique beast following the passing of the changes to the Mesa for 4k resolution fix. I feel a misunderstanding has taken place, image posted in the OP is but an example for a circumstance I needed to lock that core.

This PR does not include or touch cores specifically it simply adds
image

giving the developer who compiles RetroArch the choice of adding .lck files to the cores deployment path which they will be bundled with the appx. It's use case is circumstantial however without this PR merged these lock files are skipped when compiling. I hope this better explains why this PR exists.

@hizzlekizzle
Copy link
Contributor

Yeah, seems fine to me.

@Misunderstood-Wookiee
Copy link
Contributor Author

Misunderstood-Wookiee commented May 25, 2024

@LibretroAdmin So what is the verdict on this?
I would very much like to not have to change this for each branch or whatever in future :)

@Mew-Lew
Copy link

Mew-Lew commented May 29, 2024

The adjustments make it possible to lock cores using pre-generated *.lck files, simplifying the process for new forks and custom builds. This is particularly beneficial for developers who need to maintain locked cores. It may be a specific issue to UWP that requires this but it is still an important addition.
For instance, the Angle build for version 1.17 required cores to be locked for Xbox One users because updating the included core would break compatibility for the Xbox One. By using the *.lck files, we can ensure that the correct core version is maintained, preventing such issues.

@LibretroAdmin LibretroAdmin merged commit 64680cb into libretro:master May 30, 2024
27 checks passed
@Misunderstood-Wookiee Misunderstood-Wookiee deleted the AddCoreLock2Package branch June 5, 2024 13:59
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.

None yet

4 participants