Skip to content
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

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

Merged
merged 5 commits into from
Jan 19, 2024

Conversation

sonic2kk
Copy link
Owner

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
Copy link
Owner Author

sonic2kk commented Jan 16, 2024

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

sonic2kk commented Jan 16, 2024

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
Copy link
Owner Author

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

@sonic2kk
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant