search

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
672 stars 42 forks source link

Clicking apply to apply stardew valley mods, mod app gives error and play button remains grayed out #1628

Open ConnorMartin138 opened 2 weeks ago

ConnorMartin138 commented 2 weeks ago

Bug Report

Summary

When clicking apply to apply any Stardew Valley mods, in this instance just SMAPI, the console gives the error message given below, and the play button remains grayed out.

Steps to reproduce

Completely uninstalled and reinstalled Stardew and Mod App, reinstalled Stardew and Mod App from fresh downloads. Opened Mod App, all mods still in library, set only SMAPI to active, click apply.

What is the expected behaviour?

Should apply mods and allow play button to work.

Other information

Running linux. I completely removed stardew valley and mod app and removed all the config or save locations I could find for both and when I reinstalled the game and mod app again it remains the same.

Console output:

00:01:55.763 [ERROR] Encountered an exception published to an object with an unobserved ThrownExceptions property|System.NullReferenceException: Object reference not set to an instance of an object.
   at NexusMods.Abstractions.Loadouts.Synchronizers.ALoadoutSynchronizer.FindChangedFiles(DiskStateTree diskState, Model loadout, FileTree prevFileTree, DiskStateTree prevDiskState) in /_/src/Abstractions/NexusMods.Abstractions.Loadouts.Synchronizers/ALoadoutSynchronizer.cs:line 647
   at NexusMods.Abstractions.Loadouts.Synchronizers.ALoadoutSynchronizer.Ingest(Model loadout) in /_/src/Abstractions/NexusMods.Abstractions.Loadouts.Synchronizers/ALoadoutSynchronizer.cs:line 582
   at NexusMods.DataModel.ApplyService.Apply(Model loadout) in /_/src/NexusMods.DataModel/ApplyService.cs:line 66
   at NexusMods.App.UI.LeftMenu.Items.ApplyControlViewModel.<Apply>b__39_0() in /_/src/NexusMods.App.UI/LeftMenu/Items/ApplyControlViewModel.cs:line 158
   at NexusMods.App.UI.LeftMenu.Items.ApplyControlViewModel.Apply() in /_/src/NexusMods.App.UI/LeftMenu/Items/ApplyControlViewModel.cs:line 155
   at NexusMods.App.UI.LeftMenu.Items.ApplyControlViewModel.<>c__DisplayClass38_0.<<-ctor>b__0>d.MoveNext() in /_/src/NexusMods.App.UI/LeftMenu/Items/ApplyControlViewModel.cs:line 57

Log Files: nexusmods.app.main.current.log nexusmods.app.slim.current.log

erri120 commented 2 weeks ago

0.5.1:

https://github.com/Nexus-Mods/NexusMods.App/blob/98fd59841040da98eed91198653045bb890e7642/src/Abstractions/NexusMods.Abstractions.Loadouts.Synchronizers/ALoadoutSynchronizer.cs#L647

main:

https://github.com/Nexus-Mods/NexusMods.App/blob/595937917de44c135294b3ab39d6f50905d7f4b4/src/Abstractions/NexusMods.Abstractions.Loadouts.Synchronizers/ALoadoutSynchronizer.cs#L647

We're using ! to circumvent the null-check. No idea why we're doing this, but we shouldn't do that as seen here. @Sewer56 I think you wrote this.

Al12rs commented 2 weeks ago

This problem is very likely this:

Sewer56 commented 2 weeks ago

We're using ! to circumvent the null-check. No idea why we're doing this, but we shouldn't do that as seen here. @Sewer56 I think you wrote this.

The person to last work on that code would have been @halgari .

I've never really worked on that part of the Synchronizer. I did add that suppression back ground January when working on Abstractions, but that was on the old pre-MnemonicDB ingest code. The data types we're working with since have changed, as the method was slightly altered in the MMDB migration.

Anyway, I think the problem is likely deeper rooted though. Note that our IDE today is telling us that the suppression is redundant. So something else changed under the hood. Likely another part of the code is making an assumption that is not holding true.

In any case, I think @Al12rs 's on the money, with it being related to #1473

Al12rs commented 2 weeks ago

The assumption in question is that we can find the matching Loadout File entry in the current loadout of something that was in the Old DiskState. Before, the passed loadout for this operation was the LastAppliedRevision loaodut revision, not the current loadout revision.

Basically, if we remove anything (disable, delete etc) in the loadout, and apply, the current loadout will be missing those things, which code assumes to be present.

There is a fundamental issue with how we deal with DiskStates and LastAppliedRevisions here.