UE4SS-RE / RE-UE4SS

Injectable LUA scripting system, SDK generator, live property editor and other dumping utilities for UE4/5 games
http://docs.ue4ss.com/
MIT License
1.37k stars 187 forks source link

fix(CI): Locked glaze to 2.9.5, and we now require xmake >=2.9.3 #603

Closed UE4SS closed 4 months ago

UE4SS commented 4 months ago

Description

There was a breakage of CI with a recent commit, and this PR fixes that.

Type of change

How Has This Been Tested?

CI was run on this branch, and it succeeded: https://github.com/UE4SS-RE/RE-UE4SS/actions/runs/10145472519

Checklist

Please delete options that are not relevant. Update the list as the PR progresses.

Buckminsterfullerene02 commented 4 months ago

Don't we also need to change this? Maybe this wasn't helping or doing anything... https://github.com/UE4SS-RE/RE-UE4SS/blob/7f5b6386575a9af6e779b829725ff5506e220496/xmake.lua#L1

narknon commented 4 months ago

Thanks for figuring this out. The below isn't directly related to your changes, but some considerations with xmake shenanigans.

Should we consider whether we want to use their package for Glz at all? I assume we were using a custom package before because it is set to a version in the xmake as below. Setting the version in two places doesn't seem ideal either.

image

And does the "v" make any difference? Just a general question since most of our versions here don't have it.

image

UE4SS commented 4 months ago

Don't we also need to change this? Maybe this wasn't helping or doing anything...

https://github.com/UE4SS-RE/RE-UE4SS/blob/7f5b6386575a9af6e779b829725ff5506e220496/xmake.lua#L1

Oh I wasn't aware we had this, and I don't know if it's required, it's probably to give users a warning if they use a too old version so we should probably update this as well.

Thanks for figuring this out. The below isn't directly related to your changes, but some considerations with xmake shenanigans.

Should we consider whether we want to use their package for Glz at all? I assume we were using a custom package before because it is set to a version in the xmake as below. Setting the version in two places doesn't seem ideal either.

I don't know why we had a custom package for glaze. Perhaps it was in order to not require such a new version of xmake because glaze v2.9.5 is only available in the currently latest release of xmake, other than that, I have no idea why we had that file. With the removal of the custom glaze package, we're no longer setting the version in two places.

image

And does the "v" make any difference? Just a general question since most of our versions here don't have it.

As far as I know, yes. The xmake.lua package file that comes with xmake defines the version with a v prefix, as can be seen here: https://github.com/xmake-io/xmake-repo/blob/dev/packages/g/glaze/xmake.lua I didn't test it, so I'm just guessing that the version string has to identical. I'm not gonna test it, but obviously if you test it and find that the v prefix isn't needed, we can remove it.

narknon commented 4 months ago

Yeah, after thinking about it, I imaging the version numbers match the tags in the main repos.

narknon commented 4 months ago

I think ideally we'd have a local option for all packages, similar to PS but that's outside the scope of this. Though some repos should likely default to local if we reference them a lot.

UE4SS commented 4 months ago

I think ideally we'd have a local option for all packages, similar to PS but that's outside the scope of this. Though some repos should likely default to local if we reference them a lot.

Yeah, imgui for example should be changed, I'm so tired of not having easy access to imgui example code just because it's a "dependency" and therefore stored somewhere in AppData instead of within our project, but as you said, out of the scope of this PR.

Buckminsterfullerene02 commented 4 months ago

Also out of scope of this PR is an xmake nuke command that basically wipes everything xmake-related off your PC and gives you a clean slate just so that you don't get those horrendously painful caching issues.

narknon commented 4 months ago

I think ideally we'd have a local option for all packages, similar to PS but that's outside the scope of this. Though some repos should likely default to local if we reference them a lot.

Yeah, imgui for example should be changed, I'm so tired of not having easy access to imgui example code just because it's a "dependency" and therefore stored somewhere in AppData instead of within our project, but as you said, out of the scope of this PR.

Yep, exactly the main one that is annoying to be a package.

UE4SS commented 4 months ago

Also out of scope of this PR is an xmake nuke command that basically wipes everything xmake-related off your PC just so that you don't get those horrendously painful caching issues when debugging issues.

I don't think we can actually uninstall xmake for people, not with an xmake command anyway, and what can we even do that xrepo remove --all doesn't do ?

narknon commented 4 months ago

He's referring to this conversation, but maybe xrepo remove does what we need. https://github.com/UE4SS-RE/RE-UE4SS/issues/536

UE4SS commented 4 months ago

@Buckminsterfullerene02 I suspect my cache problem from earlier today was that I was somehow locked to glaze 2.6 locally (despite the fact that xmake was reporting 2.9.0), and so before I tried the fix-glaze branch, it just worked because it was locked to 2.6 and then fix-glaze somehow locked it to 2.9.0 which didn't exist in my version of xmake. I couldn't run xmake update, it just doesn't work for me, it fails to download every time, so I had to uninstall and reinstall xmake manually and then everything worked again.

I pushed the set_xmakever change, just fyi.

Buckminsterfullerene02 commented 4 months ago

bruh image

UE4SS commented 4 months ago

bruh image

I refuse, we're staying with 2.9.3 for now, it's working :laughing:

Buckminsterfullerene02 commented 4 months ago

I refuse, we're staying with 2.9.3 for now, it's working 😆

Yeah but the github runner will download latest version

UE4SS commented 4 months ago

I refuse, we're staying with 2.9.3 for now, it's working 😆

Yeah but the github runner will download latest version

I'm sure it'll be fine, otherwise we can lock it.

Buckminsterfullerene02 commented 4 months ago

Let's do a new test run on this branch just to be sure...

narknon commented 4 months ago

We should lock it regardless. We've had stuff break from their updates before.

UE4SS commented 4 months ago

We should lock it regardless. We've had stuff break from their updates before.

I don't know how to do that, but go for it in this PR if you want to, just make the commit message formatted properly like I did with my two commits.

narknon commented 4 months ago

I think it's here. https://github.com/UE4SS-RE/RE-UE4SS/pull/571/files I'm not available to do any commits atm.

UE4SS commented 4 months ago

I think it's here. https://github.com/UE4SS-RE/RE-UE4SS/pull/571/files I'm not available to do any commits atm.

We can do that in #591.

UE4SS commented 4 months ago

Or actually, idk if #591 deprecates experimental releases, I thought it did, but I may be wrong.

Buckminsterfullerene02 commented 4 months ago

Looks like 591 just locks xmake to 2.9.3

Buckminsterfullerene02 commented 4 months ago

Also out of scope, but maybe we should look at this system for proper package version locking? https://xmake.io/#/package/remote_package?id=dependent-package-lock-and-upgrade

UE4SS commented 4 months ago

Looks like 591 just locks xmake to 2.9.3

Yeah it introduces some other system separate from experimental and regular releases. I locked it, and I think this is ready as soon as this action completes.

UE4SS commented 4 months ago

CI has succeeded for the latest commit on this branch: https://github.com/UE4SS-RE/RE-UE4SS/actions/runs/10146933120 This PR should be ready now.