Nexus-Mods / NexusMods.App

Home of the development of the Nexus Mods App
https://nexus-mods.github.io/NexusMods.App/
GNU General Public License v3.0
911 stars 45 forks source link

Stardew Valley - SMAPI EXE name #1296

Open Pickysaurus opened 5 months ago

Pickysaurus commented 5 months ago

Spike

Proposal

Explore if renaming StardewModdingAPI.exe to StardewValley.exe when installing SMAPI is the correct approach. We can discuss both the pros and cons of doing this.

Pros

Cons

Related code

https://github.com/Nexus-Mods/NexusMods.App/blob/main/src/Games/NexusMods.Games.StardewValley/Installers/SMAPIInstaller.cs#L184 https://github.com/Nexus-Mods/NexusMods.App/blob/main/src/Games/NexusMods.Games.StardewValley/StardewValley.cs#L43

Supporting Information

I have raised this question in the Stardew Valley Discord server where the majority of responses preferred to keep SMAPI named as StardewModdingAPI.exe.

I have spoken with @pathoschild who said:

...it's pretty non-standard. It'll technically work (since the actual game code is in StardewValley.dll), but not sure whether game updates will overwrite the file and break SMAPI. Xbox App is a special case due to how the app works. It doesn't have the equivalent of Steam launch options to run a separate executable. (But if the Nexus app prefers users launch the game through it instead, that wouldn't be an issue anyway.)

erri120 commented 5 months ago

Cons Potentially unexpected behaviour.

What is the expected behavior, and why would users care?

When the game updates the EXE (containing SMAPI) is overwritten.

This isn't exclusive to this. Everything will break when the game updates, we haven't dealt with that yet.

Can cause confusion when uninstalling or updating SMAPI manually.

I don't really understand this point. Either users use the mod manager or they mod manually. We don't support a weird mix of both.

Pickysaurus commented 5 months ago

What is the expected behavior, and why would users care?

No other tool installs SMAPI this way, so if a user is expecting to see SMAPI's EXE in their folder they will be confused (I know I was!). The expected behaviour would probably be installing it in a way that follows the general guidance given by SMAPI and the known patterns of the modding community.

I think we should focus on what the benefits of this method are against doing it the way SMAPI would install itself. If the benefits outweigh the drawbacks and the community is accepting of this, then we know we've done it right.

erri120 commented 5 months ago

We replace the game executable with the SMAPI executable because a modded Stardew Valley game only works if you start the game with SMAPI. By replacing the game file with the SMAPI executable, we're guaranteeing that all game launches will launch SMAPI and no additional custom code has to be added. It's simple.

Not replacing the game executable with the SMAPI executable only brings with it more work, both for us developers and for users. Users won't be able to launch the game through their launcher of choice without manually changing the launcher configs, as detailed in the SMAPI Wiki. For the devs, this approach would require changing how we launch games on both Windows and Linux. Launching games on Linux is currently done through Steam only. Overall, this approach doesn't provide any benefits, it just makes everything more complicated.

Pathoschild commented 5 months ago

When Vortex breaks SMAPI, the common fix is to run the SMAPI installer to reset it to a properly working version. That escape hatch will be useful for the Nexus App too, since the approach it takes to support other games will cause issues for Stardew Valley players (per previous discussions on Discord).

So besides being non-standard, renaming the executable will break this escape hatch since the game won't use the fixed StardewModdingAPI.exe.

erri120 commented 5 months ago

That escape hatch will be useful for the Nexus App too, since the approach it takes to support other games will cause issues for Stardew Valley players (per previous discussions on Discord).

What approach do you refer to?

Pathoschild commented 5 months ago

@erri120 I'm referring to this Discord discussion on April 11th. Quoting my initial message:

I think trying to manage all files is partly why Vortex causes so many issues in Stardew Valley's modding tech support. It might be too late to consider anyway, but I guess I'll at least pitch the idea.

Modding Stardew Valley is straightforward: each mod has folder(s) identified by a manifest.json file, and everything inside that folder belongs to that mod. We add/remove a mod by adding/removing its folder(s) in Mods, and update a mod by overwriting its files with the new versions. Even SMAPI mostly works that way with its smapi-internal folder (with a few minimal exceptions like the executable). There's no need to keep track of which files within each mod folder got changed/added/deleted (possibly outside the app), since Stardew Valley mods don't replace game files and such like Bethesda games.

Trying to keep track of the base vanilla state seems like a recipe for state mismatches leading to the wrong files or wrong versions being added/deleted, like Vortex does. So not sure if it's something you'd consider, but it might cause a lot less support friction if (for specific games like Stardew Valley) the app could just add/remove/update mod folders directly without trying to manage the entire game folder state. I think that's how Stardrop (the mod manager recommended over Vortex for Stardew Valley) works for example.

The other issue is that Vortex and the Nexus App both add the SMAPI files directly without running the installer, which means the cleanup and auto-fixes normally applied by the installer don't get applied. In that case running the SMAPI installer directly often fixes issues.

erri120 commented 5 months ago

Right, we do manage the entire game folder. However, we do it very differently than Vortex does it, so issues that Vortex has, are not transferred to the App.

The other issue is that Vortex and the Nexus App both add the SMAPI files directly without running the installer, which means the cleanup and auto-fixes normally applied by the installer don't get applied. In that case running the SMAPI installer directly often fixes issues.

As discussed previously on the Discord as well, the SMAPI installer does all the things it does to support manual modding and previous installations. We don't need the installer because none of its additional functionality applies to the App. Without the additional functionality, the SMAPI installer extracts files from the install.dat archive into the game folder (and sets the terminal color on Unix systems, but that's besides the point).

erri120 commented 5 months ago

As far as the "escape hatch" goes, we shouldn't need it. The App is a mod manager, if users are looking for something else, then they can mod the game manually 😄.

Pathoschild commented 5 months ago

As far as the "escape hatch" goes, we shouldn't need it. The App is a mod manager, if users are looking for something else, then they can mod the game manually 😄.

Yes, but I think that's the problem. The App's design is built around the assumptions that (a) the game folder is completely clean when the App is installed; (b) the game folder never changes outside the App; and (c) nothing will ever go wrong with how the App manages files. I think all of those assumptions will sometimes break in practice.

Vortex players who run into issues can just run the SMAPI installer or reinstall mods manually to fix them, and then continue using Vortex normally.

However the App's current approach breaks those escape hatches, because it won't use the fixed StardewModdingAPI.exe and doesn't allow changing files outside the app. So community tech support may end up just telling players to either:

That would likely hurt the App's adoption and reputation in the community, which is already a bit anti-Vortex in the support channels due to previously discussed Vortex issues.

halgari commented 5 months ago

However the App's current approach breaks those escape hatches, because it won't use the fixed StardewModdingAPI.exe and doesn't allow changing files outside the app.

I'm interested in hearing why this would happen. What would break the .exe that would require re-installing the executable. And couldn't users just re-install SMAPI through the app to fix it? And remember we have full file versioning here, so if it does break we can roll back the changes that broke it, and/or alert the user to the changes.

halgari commented 5 months ago

What might help here is if we note what common modding practices will cause this process to break, and then from there we can figure out what the correct solution to the problem is.

For example:

erri120 commented 5 months ago

(a) the game folder is completely clean when the App is installed; (b) the game folder never changes outside the App; and (c) nothing will ever go wrong with how the App manages files

Al12rs commented 5 months ago

I think the argument here is that our versioning system might have a bug or flaw. That is realistically going to happen, especially in alpha, and we will have to do our best to fix any bugs and issues as users report them.

The idea is of course to get to a system without bugs where users can rollback any breaking change, but if there are issues, then yeah those users should be directed to us so we can be aware of the problems, patch them and help the affected users out.

halgari commented 5 months ago

It's also important to categorize bugs here. Yes all code has bugs, but there's a fundamental difference between a logic error, such as not noticing that one mod requires another, and algorithmic errors that are protected by some fairly simple algorithms. In this case we look at the date of a file, and if the file has changed we re-hash the file to see if it changed. That then drives everything else in the system. So the way this fails is if someone modifies a file, re-sets the modified date, and maintains the same file size in the process. That doesn't mean the process is unhackable or flawless, but someone has to really try to break the app to get that part of it to break.

Then the next wall of protection is the default behavior. The app defaults to assuming that something changed, even when it may not have. So in that case we're doubly protected. This is in great contrast to things like hardlinks and symlinks that Vortex uses or even VFS systems that MO2 uses that when they fail they silently fail. When hashing system fails in the app, we notice too many changes, unless the user is actively trying to break the system.

halgari commented 5 months ago

And I will point out, that no change tracking is a completle non-starter. You say that modders don't modify their files. But they do. They modify configs, saves are overwritten or deleted at some point, and logs are created. At any point Steam can come along and stomp on all the files, breaking things. So it's not a immutable vs mutable system. It is much more immutable than most games, but there is mutation happening here. So if we can handle this game in the same way we handle less clean games, it's a win-win.

Pathoschild commented 5 months ago

I'm interested in hearing why this would happen. What would break the .exe that would require re-installing the executable. And couldn't users just re-install SMAPI through the app to fix it? And remember we have full file versioning here, so if it does break we can roll back the changes that broke it, and/or alert the user to the changes.

That file versioning is one of the things I'm worried about, per my comment above.

You say that modders don't modify their files. But they do. They modify configs, saves are overwritten or deleted at some point, and logs are created. At any point Steam can come along and stomp on all the files, breaking things.

Agreed. That's one of the points I've been trying to make in this and previous discussions, but the response has been that the App requires the game folder to be clean and that there's no need to have escape hatches since the App's state will never be wrong.

It seems like you're set on this approach though, and it's outside the scope of this specific issue. So hopefully I'm just being pessimistic and it'll actually work great in the end, particularly when you figure out change ingesting.

Al12rs commented 5 months ago

I honestly agree that the versioning and change tracking is overkill for stardew. It has the potential of bringing value to users though. So that in the future if they somehow break their setup, they can just revert to a previously working version by pressing Undo a couple of times, or something along those lines.

This could end up being a better user experience for users than having to run the smapi installer.

I'm quite certain that we will have our bugs and that our system will have to go through a few user testing iterations before it will actually be solid and reliable.

The thing is that we will have to use this system for more complex games where data consistency will be even more important and critical. If we have to get the system working for those games, then it should be able to handle even a relatively simpler modding pattern like Stardew.

AlanDavison commented 5 months ago

I do feel the cons quite heavily outweigh the pros for this. Not just in terms of count, but in terms of severity and the support burden they have the opportunity to add to the support volunteers on the Stardew Discord server. They're already quite heavily overloaded as is.

This could end up being a better user experience for users than having to run the smapi installer.

As for not running the SMAPI installer, what would happen if the installer structure changed somewhat? If the bundled mods change? ErrorHandler was made redundant because of inclusion of the error handling into Stardew itself, and removed from the installer. How would the program handle changes like this without running the installer? It just feels to be making things so much more unnecessarily complicated for no gain.

I honestly agree that the versioning and change tracking is overkill for stardew. It has the potential of bringing value to users though. So that in the future if they somehow break their setup, they can just revert to a previously working version by pressing Undo a couple of times, or something along those lines.

And just to weigh in on the file versioning aspect without completely veering off-topic (is there an issue to keep discussion of this focused?), I do think it would be significantly better overall if versioning could be made optional on the game module level as opposed to being something every game's module does purely because of its overkill nature for some games.

The main issue (as raised in the Discord server) is that mods are absolutely free to store save-related data wherever they like. It may be horrendously bad practice to keep a central database of things related to individual saves inside the mod's own directory, but it can be done. In a situation like that, a user could easily rollback their loadout a little and make their save completely unusable. Perhaps recoverable, perhaps not depending on how much save-related data was stored in the mod's own directory and rolled back, throwing it out of sync with the saves.

Frankly, I would call forcing file versioning an unreasonable risk given the possibility of situations like that to occur. Yes, it would absolutely be the fault of the mod author storing save-related data in the wrong place. But at the end of the day, the user shouldn't pay the price for that.

halgari commented 5 months ago

@DecidedlyHuman you raise two issues here I'd like to dig into a bit.

1) Not running the SMAPI installer. True, if the installer becomes more complex we run the risk of having to duplicate this functionality 1:1. But that being said, what has actually changed recently that requires major changes to the way the installer operates? That is to say, where additional files need to be put in new locations or files that are included in SMAPI are no longer to be copied. Notice there's key point here that it's not a question of how does a installer change from version to version, but how do the operations inside the installer change? Where it's no longer "EXE goes into the game folder, config goes into this folder" and becomes "EXE with this prefix goes here, config goes somewhere else". The installers in the app are rules based for the most part and so small changes to the files and their structure doesn't matter.

2) I really don't understand the isseu being raised about saves in the mod folders. The idea with all of this is that users will install mods, play the game, and in general not change the loadout mid-playthrough. Sure users are free to make changes mid-playthrough but any game will likely break if this isn't done properly. If a user wants a completely different setup they are free to fork the loadout, go back to a unmodded loadout, create something completely new, and then swap freely between both loadouts.

There seems to be some miscommunication here I can't put my finger on. @DecidedlyHuman and @Pathoschild are raising valid points, but these points are somewhat orthogonal to the way the app works. The design of the app doesn't make issues with non-standard save locations or updating games, or non-standard installers any worse. It may make the situations slightly better as users can always say "aw crap, let me undo that" and go back to a working loadout.

Frankly, I would call forcing file versioning an unreasonable risk given the possibility of situations like that to occur.

What's the risk? Users can always go back to they way they had it the last time the game worked, or last week, or last monthm or whatever. Unlimited undo is the whole point of this methodology, in that sense there is no risk.

halgari commented 5 months ago

Maybe the issue here is that people assume saves exist outside of the file versioning system? If that's the case I should clear that up. The game's saves are tracked just like the settings, the mods and the game itself. All versioned and all tracked with undo features. So if a mod stores save data in some strange location, no big deal, it's tracked and can be rolled back just like any other sort of data.

Rogue-Toast commented 5 months ago

The idea with all of this is that users will install mods, play the game, and in general not change the loadout mid-playthrough. Sure users are free to make changes mid-playthrough but any game will likely break if this isn't done properly. If a user wants a completely different setup they are free to fork the loadout, go back to a unmodded loadout, create something completely new, and then swap freely between both loadouts.

This might be the case with other games, but with Stardew, modded players add and remove mods all the time, even daily as new mods come out or as mod bugs surface that cause game-breaking issues.

halgari commented 5 months ago

This is good to know, in the case that a user updates a mod, is it assumed that the mods will always be backwards compatable with saves? Is that enforced by SMAPI in some way?

erri120 commented 5 months ago

The archive you get when downloading SMAPI contains the installer executable, as well as three install.dat files for the three supported operating systems: Windows, Linux, and macOS. These three install.dat files are all archives containing the actual SMAPI files that will end up in the game folder.

Assuming the user has a clean vanilla game with no prior SMAPI installations, the original SMAPI installer does the following:

Here's what our SMAPI installer does:

Replacing the game executable and not changing the terminal color (yet) are the only differences between the two installers for a vanilla game. Besides changing the terminal color, the only thing both installers do, in this scenario, is copying files from one place to another.

@DecidedlyHuman

As for not running the SMAPI installer, what would happen if the installer structure changed somewhat? If the bundled mods change? ErrorHandler was made redundant because of inclusion of the error handling into Stardew itself, and removed from the installer. How would the program handle changes like this without running the installer?

Unless install.dat gets removed from the archive you get when downloading SMAPI, we don't have to update our installer. As explained, both the original and our custom installers essentially just copy files from that archive into the game folder. Since we install SMAPI into a single mod in the App, all included SMAPI mods are bundled with it, so upgrading SMAPI won't leave behind the now redundant ErrorHandler SMAPI mod.

Our installer is, in the programming sense, pure. You give it the SMAPI archive as input, and the output is a mapping of files from one place to another. When upgrading SMAPI, you run the installer again on the new version and get a new set of file mappings. We don't mod updates integrated into the App yet, but once we do, the new file mappings will essentially just replace the old file mappings for a clean upgrade.

It just feels to be making things so much more unnecessarily complicated for no gain.

In a sense, it's the original SMAPI installer that's unnecessarily complicated for a vanilla state because you'd achieve the same result by just extracting an archive and copying the deps.json file. Of course, the original SMAPI installer does the things it does to support previous SMAPI installations and manual modding, which is great for users that need it. However, the App doesn't need this additional functionality because users mod the game non-destructively using the App. Users will be able to easily install and uninstall mods, rollback to a previous version of their modlist (we call it Loadout), edit configs from within the App, and get helpful diagnostic messages if we detect that something's not right (eg: missing dependencies).

The goal of Nexus Mods has always been to "make modding easy". The App follows that and allows users to easily install SMAPI with the App as they do with any other mod. They don't have to manually download the archive, run the installer, edit Steam program arguments, or do any other manual work. It just works™️, and if it doesn't, we'll definitely know from our unit tests or vocal users 😄

Pathoschild commented 5 months ago

Since we install SMAPI into a single mod in the App, all included SMAPI mods are bundled with it, so upgrading SMAPI won't leave behind the now redundant ErrorHandler SMAPI mod. [...] When upgrading SMAPI, you run the installer again on the new version and get a new set of file mappings. [...] the new file mappings will essentially just replace the old file mappings for a clean upgrade.

Just to make sure: does that process distinguish between an obsolete file (which should be deleted) and a generated file (which should be kept), since both will be missing from the new file mappings?

This is good to know, in the case that a user updates a mod, is it assumed that the mods will always be backwards compatable with saves? Is that enforced by SMAPI in some way?

Since mods are commonly added/updated to an existing save, they're expected to handle that; any mod which somehow fails in that scenario will probably get bug reports on its mod page until it's fixed. SMAPI and the game are also designed to facilitate that; for example, the game will automatically spawn an NPC that was added to the data asset mid-save.

Mods can also be removed anytime, and there's error-handling to ensure that doesn't cause any issues. For example, custom items from a removed mod will just turn into Error Items in-game, which the player can keep or trash as they see fit. Re-adding the mod later will return those Error Items to normal.

SMAPI also has validation to detect mods that may break that assumption. For example, any mod which changes the save serializer will trigger a scary warning in the SMAPI console indicating that it may permanently break saves when it's removed. Players are often sensitive to those warnings, and will either uninstall the mod or report it on the mod page; that tends to push mod authors away from doing things like that.

erri120 commented 5 months ago

@Pathoschild

Just to make sure: does that process distinguish between an obsolete file (which should be deleted) and a generated file (which should be kept), since both will be missing from the new file mappings?

We haven't implemented mod updates yet, but we're probably going to do something like this:

mod.Files = newFiles;
mod.Version = newVersion;

This behavior won't be exclusive to SMAPI, every mod will likely be updated this way.

erri120 commented 5 months ago

This issue was originally about discussing how we rename StardewModdingAPI.exe to StardewValley.exe but we've ended up talking about everything else. It might be worth enabling GitHub Discussions @halgari @LukeNexusMods.

Al12rs commented 5 months ago

We really haven't discussed mod updates yet. I hear the concern for maintaining some things from the previous version, e.g. generated configuration files. There are some interesting ways we could handle this, but erri is right and this isn't the time or venue. We will start discussing mod updates likely a bit later on