Ingest required error

[halgari]

Resolves #871

It seems like the problem was mostly due to us storing the previous disk state keyed by LoadoutId instead of on the game version. One we switch to using the game version it all seems to work correctly. I'll do more testing on the CLI and we can create a new PR if this doesn't go far enough for some reason

[codecov-commenter]

Codecov Report

Attention: 37 lines in your changes are missing coverage. Please review.

Comparison is base (8728b65) 0.00% compared to head (f4276d4) 58.14%.
Report is 1 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff            @@
##           main     #974       +/-   ##
=========================================
+ Coverage      0   58.14%   +58.14%     
=========================================
  Files         0      637      +637     
  Lines         0    20564    +20564     
  Branches      0     1572     +1572     
=========================================
+ Hits          0    11957    +11957     
- Misses        0     8246     +8246     
- Partials      0      361      +361     

Flag	Coverage Δ
Linux	57.44% <32.72%> (?)
Windows	57.35% <32.72%> (?)
clean_environment_tests	58.12% <32.72%> (?)
macOS	56.83% <32.72%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
.../NexusMods.Abstractions.DiskState/DiskStateTree.cs	84.31% <100.00%> (ø)
...ons.Loadouts.Synchronizers/ALoadoutSynchronizer.cs	94.08% <100.00%> (ø)
.../NexusMods.DataModel/Loadouts/DiskStateRegistry.cs	96.42% <100.00%> (ø)
...sMods.DataModel.Tests/ALoadoutSynchronizerTests.cs	0.00% <0.00%> (ø)

... and 633 files with indirect coverage changes

[halgari]

Confirmed by the CLI:

• Managed Skyrim into a list called "BaseList1"
• Managed Skyrim into a list called "MainList1"
• Applied both lists (works fine)
• Installed SKSE into "MainList1"
• Applied both lists and verified files were added/removed with each apply

[Al12rs]

I want to test this locally, specifically adding outside changes to the test case above.

[Al12rs]

Tested the PR merge with main using the new Apply button (has the same logic for apply as LaunchButton had).
I tested a number of scenarios with two loadouts with and without outside changes.

Testing results:

• Removing mods and Applying leaves empty folder structures in game folder
• Creating a second loadout ingests the previously deployed loadout as the game files for that loadout.
• Changes are ingested into the loadout currently being applied instead of being ingested into the last applied loadout
• Outside changes are ingested and a new loadout is created but it is never used
  • When applying, outside changes are ingested, but then the original loadout is Applied
    • Outside changes are removed from file system and are not integrated in the loadout shown in the LoadoutGrid
  • Options:
    • Update user loadout with outside changes and apply the new loadout
    • Ingest outside changes into a branch of last applied loadout revision and keep that for users to decide what to do with.
• The loadout revision ID saved with a DiskState is not guaranteed to have a flattened loadout that matches the DiskState (unapplied changes).
• No way to get the Last Applied LoadoutId and the last applied loadout revision without also getting the entire last DiskState.
  • This is needed for Apply button UI logic (show which loadout is being applied and change diff) and knowing in which loadout to do the ingest.

Let me know if you want to handle any of these in this PR. We can discuss some of these in more details after standup

[halgari]

Tested the PR merge with main using the new Apply button (has the same logic for apply as LaunchButton had). I tested a number of scenarios with two loadouts with and without outside changes.

Testing results:

• Removing mods and Applying leaves empty folder structures in game folder

• Creating a second loadout ingests the previously deployed loadout as the game files for that loadout.

• Changes are ingested into the loadout currently being applied instead of being ingested into the last applied loadout

• Outside changes are ingested and a new loadout is create but it is never used

  • When applying, outside changes are ingested, but then the original loadout is Applied

    • Outside changes are removed from file system and are not integrated in the loadout shown in the LoadoutGrid

  • Options:

    • Update user loadout with outside changes and apply the new loadout
    • Ingest outside changes into a branch of last applied loadout revision and keep that for users to decide what to do with.

• The loadout revision ID saved with a DiskState is not guaranteed to have a flattened loadout that matches the DiskState (unapplied changes).

• No way to get the Last Applied LoadoutId and the last applied loadout revision without also getting the entire last DiskState.

  • This is needed for Apply button UI logic (show which loadout is being applied and change diff) and knowing in which loadout to do the ingest.

Let me know if you want to handle any of these in this PR. We can discuss some of these in more details after standup

Let's put these in a separate issue and do a different PR for that

[Sewer56]

I also reviewed the code before falling asleep, just didn't comment on case tired me would say something silly.

In any case, there is one potential caveat I see here. I've heard mentions in the past of 'supporting multiple installations of the same game' at once within the team.

Because we serialize with key of Name|Version|Store, there's.no way to uniquely identify a second copy of the game with thebsame version.

Personally I think the point is moot since a user can only currently have 1 copy of the game per store anyway in the App (since we pull from e.g. Steam info and Steam can only have 1 copy).

I don't think it's worth investing effort in this area, but I figured it was worth pointing out since I've heard of the desire of multi copies per game a few times; and that would require changing ID format.