Nexus-Mods / Vortex

Vortex Development
GNU General Public License v3.0
898 stars 131 forks source link

Review: Frostpunk 2 #16447

Closed ChemGuy1611 closed 3 days ago

ChemGuy1611 commented 5 days ago

Nexus Username

ChemBoy1

Extension URL

https://www.nexusmods.com/site/mods/960

Game URL

https://www.nexusmods.com/frostpunk2

Existing Extension URL

NONE

New features

This is based on a standard UE5 mod installation model. Supports all game store versions of the game, including Game Pass.

There is a modding SDK, FrostKit, that will be released soon. There may be additional or alternate ways to install mods when that comes out. I will update the extension to accommodate this.

Information

Packaging

Testing

If a task fails, contact the author to request changes before continuing.

When reviewed and passed, please complete the following tasks:

IDCs commented 4 days ago

Hey there @ChemGuy1611,

As usual, thank you for your contributions.

I can see that you're building up the requiredFiles array while querying the executable - please note that's not going to work, by the time the executable functor gets queried, the game registration call had already been processed. And the required files array had already been set in the state.

Fortunately the array had been set to be empty so it will not block users from managing the game as long as auto-discovery works (appears to be fine at a glance, but I can't test it on my end). In cases like these, it's better to find a single common file between the different game store versions and use that file in the requiredFiles array instead of the executable.

Have a look at the depots when trying to ascertain which file to use: https://steamdb.info/depot/1601581/ https://www.gogdb.org/manifest/4e3eb41d13923736a37cf6ac7a381f3f

ChemGuy1611 commented 3 days ago

Yes, I do understand what you are saying. I learned about this method from insomnius's Palworld extension. For the executable and required files, this uses a basically identical method to that extension. I don't know enough about the API to understand exactly why this structure works, but I have tested it with quite a few games now that I went back and updated to integrate Xbox version support into, and it does work well. It seems that having the required files array empty until it is filled with the executable is necessary. It took me a loooong time to discover this fact.

So basically if you choose another common file to put in required files, the getExecutable function won't work properly. The array needs to remain empty until filled by the discovered executable.

I could certainly be wrong on this. It might be OK to have another common file in requiredFiles and it would still work. I've also tried it with my extensions as I set up test game folders to test extensions before I have the real game files, so I'm selecting that location manually, and the getExectable function works there as well.

It will also work for manual discovery, as I found out. First time I tried using Palworld extension with Game Pass version it didn't discover automatically, so I had to select it and everything worked.

Just for reference, here's a list of extensions I've made that use a structure like this for setting executable and required files on different game versions.

IDCs commented 3 days ago

The problem with not providing a file in the requiredFiles array is that you can manually set the game folder to any directory (even C:/Windows/System32/) and Vortex will allow it as it can't check if the directory the user is setting is actually the game or not. It also may fail when the user chooses to unmanage the game (as again, Vortex can't check if the game is even installed)

It's not a big deal and as you said there are several game extensions that do not provide a populated array due Vortex's limitation of not being able to provide this array dynamically. But it's usually practiced when the game directory structure differs greatly between different game stores.

Palworld for example has a Win64 directory for Steam and all other non-xbox game stores; while xbox expects the files to be located inside WinGDK. That's not something that Vortex can support currently which is why that extension has an empty array.

The Frostpunk 2 directory structure for xbox is almost identical to the other game stores though and they do include common files that can be tested against.

I did notice that some of the game extensions you wrote had the same pattern, but assumed that the directory structure is different between game stores.

I'll add your extension to the manifest, but please be aware that you should have that array populated on Vortex start-up unless the game structure is widely different like in the case of Palworld.

IDCs commented 3 days ago

Added - thanks again.

ChemGuy1611 commented 3 days ago

Added - thanks again.

Just for reference, the Win64/WinGDK folder difference is present on basically all Unreal Engine based games. Frostpunk 2 has this just like Palworld does.

I will run some tests and see if I can add a common file to requiredFiles and see if everything still works.

I tested the Palworld extension as well and you are right, Vortex does not stop me from selecting any folder, even if the game isn't there.

Thanks for the review and feedback!

ChemGuy1611 commented 3 days ago

I did a quick test and it seems like there is no problem with adding another common file to requiredFiles that is not the executable. This prevents users from selecting a non-game folder in manual discovery. I will update those extensions I listed to include this just to stop users from setting random folders as the game folder. A good option for Unreal Games is you can just use the "Epic Code Name" folder as the required file, so in this case it would be "Frostpunk2".

Might want to tip off @insomnious that the Palworld extension could use an update for this issue as well.