-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix logic for fetching MO2 and Vortex version #592
Conversation
Yay, thank you for the patch! I already suspected a broken upstream url - glad you already sorted it out 😀 👍 |
If the version for other upstream projects are fetched in a similar way (from the "Assets" section of GitHub, which is now lazily loaded and can't be retrieved with Luckily with how the patch is written, provided they use tags and releases in a similar way to MO2 and Vortex (which I think projects that distribute a binary release would), the function I created should work for them too. |
I just took a closer look and just now realize that you only changed the version check of the upstream program and not the actual download itself. The PR looks good so far, but I'd suggest
(the |
Ah, oops, I hadn't meant to remove that. I did during tests but I can add that back in.
I think that makes sense, so we use the new variables as a base to build those download URLs. I didn't do this for fear of potentially breaking compatibility but sure, I can try to do that 😄 So in places like this: # MO2DLURL is $GHURL/ModOrganizer2/modorganizer/releases
DLURL="$MO2DLURL/download/v$MO2VERSION/$MO2SETUP" We would do this: # MO2PROJURL is $GHURL/ModOrganizer2/modorganizer
DLURL="$MO2PROJURL/releases/download/v$MO2VERSION/$MO2SETUP" I wonder if maybe it could be simpler to just change the I'll look into it, thanks!
Ah, thanks for the clarification 😄 |
A potential problem with changing the download URLs for MO2 and Vortex is that there seem to be options for changing the download URL for these projects. If we skip using I guess we could just change this option too, so it points to the MO2 and Vortex project URLs. But then we might have to update translations, and it might take a while for other translations to be updated to reflect that this is the MO2 and Vortex repo URLs and not the download URLs. Though maybe differentiating in that way doesn't matter too much. |
Just pushed a commit to fix the Vortex variable not using |
Thanks for your thoughts! Yeah, you're right in theory regarding the user-changeable urls (best gui for editing urls is by the way: |
GitHub changed how assets are loaded on the releases page, they are loaded after the page with lazy loading, so we can't wget on the releases page and get the executable. Instead, we can get the latest tag version from the release tags page and get the release executable name from the expanded_assets page for that version. On this page, we can use the previous logic to get the setup executable name. This solution uses a generic function that works for both MO2 and Vortex. This function only changes how we fetch the latest version for these tools, the actual download logic is the same since that URL did not change.
Okay, I made an attempt to update the variables. Please let me know if I did anything incorrectly or missed anything 😅 I did a quick test and both MO2 and Vortex seem to download correctly now (I used the Basically, now:
I tested this on my PC and laptop with existing STL installations and there didn't seem to be any compatibility issues 😄 |
Yay, you're fast! Thanks a lot! Looks good for me.👍 merging |
Fixes #591, #589. Will be needed for #540.
Problem
GitHub changed how assets are loaded on the releases page, they are loaded after the page with lazy loading, so we can't
wget
on the releases page and get the executable when trying to get the executable version and name for MO2 and Vortex Mod Manager. This means MO2 and Vortex can't be downloaded, as STL doesn't catch their version.MO2SETUP
andVORTEXSETUP
, which hold the setup executable, end up blank since thewget
code fails.Solution
Instead, we can get the latest tag version from the release tags page and get the release executable name from the
expanded_assets
page for that version. On this page, we can use the previous logic to get the setup executable name.This solution uses a generic function that works for both MO2 and Vortex. This function only changes how we fetch the latest version for these tools, the actual download logic is the same since that URL did not change.
How This Works
(adding this in an edit because I forgot to include it initially in my rush to get this up, oops!)
Basically, the way we get the version now is as follows. See my comment with an initial attempt at getting this working and my initial generic function attempt for some background/context on how this solution came to be.
We have a new function called
getLatestGitHubExeVer
. This function takes two inputs: The exe name (SETUPNAME
) to look for, and the project URL (PROJURL
). This is why the two new project URLs were added, they were added to be passed to this function.We check the tags page (e.g., for MO2 this is https://github.com/ModOrganizer2/modorganizer/tags) and grep all of the elements that have a specific tag link. For example with MO2 this would be getting all elements on the page that have
/ModOrganizer2/modorganizer/releases/tag
. We take the first one which should always be the latest release (we are only looking for tags associated with a release, and in the case these two projects, this seems to work as we don't get any MO2 development tags).There were probably other ways to check this but the tags page seemed to return the least amount of data, so I chose that method.
This will return a bunch of elements that have paths that for MO2 might look like
/ModOrganizer2/modorganizer/releases/tag/v2.4.4
, then one withv2.4.3
,v2.4.2
, and so on. We strip out the version from these paths and we build a path to theexpanded_assets
URL for that version. This is where GitHub pulls the Assets information that it displays on the releases page. For example, see this URL for MO2: https://github.com/ModOrganizer2/modorganizer/releases/expanded_assets/v2.4.4We get the
/ModOrganizer2/modorganizer/releases/tag
path by using theRELEASESURL
variable and stripping off theGHURL
prefix, then adding/tag
to the end. That way it doesn't have to be hardcoded if the URL ever changes.We build the URL to the
expanded_assets
release page with the version because the relevant version needs to be put onto the end of that path. So in future when2.4.5
comes out, we'll get the latest version from the tags and then we'll build a path for theexpanded_assets/v2.4.5
download.Then we use the previous existing logic that was used to get the setup executable from the releases page, to get the setup executable from the
expanded_assets
page.Hope that explanation made sense, feel free to ask me to elaborate. Sometimes I'm not very good at explaining it in words so if you need more info please ask!
Concerns
This PR adds two new URLs, which are the project URLs for MO2 and Vortex. Will these new URLs automatically populate for users when they update? No existing URLs should've been altered, the URLs are new and added.
If I need to do any work to make sure they populate for the user, please let me know. I want this change to be as simple for users as updating STL 😃
A couple of people have tried out this patch already and it seems to work for them.
I am very inexperienced with Bash, so please don't hesitate to call out anything that could be done better. The new function was kind of hacked together from a test script I made while drinking coffee.
I wanted to get some sort of fix out ASAP for this bug and "it works on my machine"(TM). Vortex was able to start installing and MO2 got to the point where it asked me to create an instance and choose my games.
As usual, all feedback is appreciated. Thanks! 😄
P.S. It was talked about before but soon after this PR or a similar sort of fix for this issue is merged, maybe we should think about rolling a new release. I only say this because fixing this issue is slightly-medium-priority in my mind, it might be good to get it out in stable for users ASAP.