search

sonic2kk / steamtinkerlaunch

Linux wrapper tool for use with the Steam client for custom launch options and 3rd party programs
GNU General Public License v3.0
2.05k stars 70 forks source link

ModOrganizer 2: Prevent MO2MODE from being 'none', only checkMO2 if MO2MODE is 'GUI' #1017

Closed sonic2kk closed 5 months ago

sonic2kk commented 6 months ago

Resolves an issue observed in the log for #1012. May help #1016, very little information was provided to dianogse if it was the same problem.

Overview

It has been reported that a couple of games somehow get into a state where MO2MODE is none (or $NON). This shouldn't happen, and so far has only been actually confirmed to happen with one game for one user and only on SteamOS (#1012, happened with Harvestella). A fresh install fixes this issue. In the log for #1012, I observed MO2MODE was none.

This would explain two things:

  1. ModOrganizer 2 is launched in checkMO2 if MO2MODE != disabled. Of course, none != disabled, so it tries to launch whenever a game config gets in this state, where MO2MODE = none.
  2. A clean install fixes this because it re-creates the config files, thus re-creating the config file properly.

Nowhere in the code is MO2MODE set to none, so this must be some config file defaulting that incorrectly sets the value. However, none of my config files have this value, including a couple of games never launched with SteamTinkerLaunch until just now. So I'm not sure how this actually happens. I suspect it could be somehow isolated to Steam Deck usage, but I don't know what would be causing it there.

Implementation

As a fix, I have made the followiing changes:

  1. Add a migration in migrateCfgOption that forces MO2MODE = disabled if MO2MODE is none. This should fix config files that got in a bad state somehow with this invalid value.
  2. Change checkMO2 to only launch on $MO2MODE = gui. Previously there were plans for a silent mode, but as I don't use MO2 and don't work on that, and no one else has shown any interest, for now we will only launch on gui. This makes sense
  3. Exit early in checkMO2 if we have MO2MODE = none. This should never happen now because of migrateCfgOption, but it is an extra bit of insurance. We also have some logging for this.
  4. When a user tries to launch MO2 in silent mode, since this is not implemented, default to gui. This was not an issue before since we only prevented launch on MO2MODE = disabled, but now we are skipping that.
  5. Exit early if MO2MODE = disabled, not all that important to functionality but results in cleaner code.

This all needs extensively tested and some keen review from myself, but this should hopefully resolve the problems. If more information is provided in #1016 we can see if it resolves that issue too.

I suspect, somehow, this is related to custom commands in the case of #1012. But I have no idea, really. At the very least this should prevent it from happening.

sonic2kk commented 6 months ago

Tested a game that uses ModOrganizer 2 on my system (The Elder Scrolls IV: Oblivion Game of the Year, AppID 22330). Works as expected. MO2 starts up properly when enabled.

sonic2kk commented 6 months ago

Tested a Proton game that does not use ModOrganizer 2 (PowerWash Simulator, AppID 1290000). Works as expected, no MO2 prompt.

I inspected the calls to checkMO2 and they look the same as before but with extra logging, so the flow should be normal.

sonic2kk commented 6 months ago

Tested forcing MO2MODE to none in the game config file for a Proton game (Travellers Rest). It was migrated to disabled correctly on startup. When launching the game via STL, checkMO2 is correctly skipped because MO2MODE is disabled.

sonic2kk commented 6 months ago

I considered an option to migrate if MO2MODE was not gui or disabled, but we don't know how this will change in future. This would be brittle, and even though currently gui and disabled are the only valid UI values, there is no need for this change now.

This PR updates checkMO2 to only launch on gui (and this is an easy change if we add silent or any other future mode), and if the MO2MODE values get in a bad state that isn't none, MO2 won't try to launch / install now. Also, should that happen, it can be resolved on the GUI, and we have better logging now.

sonic2kk commented 6 months ago

There seems to be no regression in behaviour, this should work as expected. I have asked another user to investigate to see if this problem is "widespread", and the OP of #1016 to provide some more details and optionally to test if this PR resolves their issue.

This is ready to merge but I am waiting for more feedback from various places before merging.

sonic2kk commented 6 months ago

There is potential in future to restructure checkMO2 to be less nested, but that is for another PR.

sonic2kk commented 5 months ago

Issue is confirmed to be reproducible on and exclusive to SteamOS, not sure why though. Maybe related to #935, but surely it would've come up before now.

I will merge as-is after bumping the version, although this is more of a hack than a real fix. I would rather not leave this PR open though and have this issue remain widespread.