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

Source generator read models #1626

Merged
merged 70 commits into from
Jun 26, 2024
Merged

Source generator read models #1626

merged 70 commits into from
Jun 26, 2024

Conversation

halgari
Copy link
Collaborator

@halgari halgari commented Jun 13, 2024

Moves the codebase over to the new source generators. Removes a lot of code that's now been moved over to auto-generated files. Looks like a lot of changes but it's mostly renames/remapping of APIs.

halgari added 29 commits June 4, 2024 13:40
# Conflicts:
#	src/Abstractions/NexusMods.Abstractions.Installers/IArchiveInstaller.cs
# Conflicts:
#	src/Abstractions/NexusMods.Abstractions.Loadouts.Synchronizers/ALoadoutSynchronizer.cs
#	src/NexusMods.App.UI/Pages/LoadoutGrid/LoadoutGridViewModel.cs
#	src/NexusMods.App.UI/Pages/ModLibrary/FileOrigins/FileOriginEntry/FileOriginEntryViewModel.cs
#	src/NexusMods.App.UI/Pages/ModLibrary/FileOrigins/FileOriginsPage.cs
#	src/NexusMods.App.UI/Pages/ModLibrary/FileOrigins/FileOriginsPageViewModel.cs
#	src/NexusMods.App.UI/Pages/ModLibrary/FileOrigins/IFileOriginsPageViewModel.cs
#	src/NexusMods.App.UI/Pages/TextEdit/TextEditorPageViewModel.cs
@halgari halgari requested review from erri120 and Al12rs June 25, 2024 03:50
@halgari
Copy link
Collaborator Author

halgari commented Jun 25, 2024

Most of the feedback is handled, I think we'll need to do a pass on the downloader/analysis metadata sometime this sprint to fix that part of the code. It works for now, I can download files, install them, uninstall, login, etc.

Copy link
Member

@erri120 erri120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downloading via NXM while having the downloads page open results in an exception:

00:00:49.480 [ERROR] x.Version Binding received an Exception!|System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.Collections.Generic.KeyNotFoundException: Entity EId:200000000003B88 does not have attribute NexusMods.Networking.Downloaders.Tasks.DownloaderState/Version
   at NexusMods.MnemonicDB.Abstractions.Attributes.ScalarAttribute`2.ThrowKeyNotfoundException(IHasEntityIdAndDb entity)
   at NexusMods.MnemonicDB.Abstractions.Attributes.ScalarAttribute`2.Get(IHasEntityIdAndDb entity)
   at NexusMods.Networking.Downloaders.Tasks.State.DownloaderState.ReadOnly.get_Version() in /ssd-pool/projects/NexusMods/app/code-review/src/Networking/NexusMods.Networking.Downloaders/obj/Debug/net8.0/NexusMods.MnemonicDB.SourceGenerator/NexusMods.MnemonicDB.SourceGenerator.ModelGenerator/NexusMods_Networking_Downloaders_Tasks_State_DownloaderState.Generated.cs:line 349
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
   at ReactiveUI.Reflection.TryGetValueForPropertyChain[TValue](TValue& changeValue, Object current, IEnumerable`1 expressionChain) in /_/src/ReactiveUI/Expression/Reflection.cs:line 239
   at ReactiveUI.ObservedChangedMixin.TryGetValue[TSender,TValue](IObservedChange`2 item, TValue& changeValue) in /_/src/ReactiveUI/Mixins/ObservedChangedMixin.cs:line 101
   at ReactiveUI.ObservedChangedMixin.GetValueOrDefault[TSender,TValue](IObservedChange`2 item) in /_/src/ReactiveUI/Mixins/ObservedChangedMixin.cs:line 59
   at ReactiveUI.ReactiveNotifyPropertyChangedMixin.<>c.<NestedObservedChanges>b__5_0(IObservedChange`2 x) in /_/src/ReactiveUI/Mixins/ReactiveNotifyPropertyChangedMixin.cs:line 175
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector._.OnNext(TSource value)

On startup, RestoreWindowState also seem to fail:

00:00:04.223 [ERROR] Exception while loading window state|System.InvalidOperationException: Sequence contains no elements
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at NexusMods.App.UI.Windows.WindowManager.RestoreWindowState(IWorkspaceWindow window) in /ssd-pool/projects/NexusMods/app/code-review/src/NexusMods.App.UI/Windows/WindowManager.cs:line 115

@halgari
Copy link
Collaborator Author

halgari commented Jun 25, 2024

I fixed the second, but I can't seem to replicate the first issue. Managed SDV, installed mods from Nexus and disk, clicked through the windows, etc. Was this due to some old state?

@halgari halgari requested a review from erri120 June 25, 2024 12:32
@erri120
Copy link
Member

erri120 commented Jun 25, 2024

I fixed the second, but I can't seem to replicate the first issue. Managed SDV, installed mods from Nexus and disk, clicked through the windows, etc. Was this due to some old state?

With fresh state:

  1. Login
  2. Add Stardew Valley
  3. Open Downloads Workspace
  4. Download SMAPI via NXM -> Exception

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only went through some parts of the code since @erri120 already reviewed and mostly did testing of the app.

Problems:

  • Enabling/Disabling a mod does not enable the Apply button
  • Downloading a mod causes a bunch of exceptions as @erri120 described
  • Installing a larger mod from Download completely freezes the UI for multiple seconds (tested with SMIM)
  • Completed Downloads section does not show in the UI, completed downloads don't appear in it
  • Download progress doesn't seem to work
  • Adding mod to loadout multiple times causes exception vm.Value Binding received an Exception!|System.Collections.Generic.KeyNotFoundException: Entity EId:200000000003BFF does not have attribute NexusMods.Abstractions.Loadouts.Mods.Mod/Version in C:\Dev\Repos\NexusMods.App-code-review-tree\src\NexusMods.App.UI\Pages\LoadoutGrid\Columns\ModVersion\ModVersionViewModel.cs:line 15

@halgari
Copy link
Collaborator Author

halgari commented Jun 26, 2024

I had some major computer issues today, and am way behind on this, I've fixed/verified the bugs mentioned by @Al12rs by overwriting my changes to the Download Service and re-porting those to the source generators, with all the code merges they had become rather broken. The tests pass locally so here's hoping they pass in C# as well, either way, I'm going to bed now, lol

@@ -60,6 +60,7 @@ public ModEnabledViewModel(IConnection conn)
{
var mod = Mod.Load(db, id);
txInner.Add(mod.Id, Mod.Enabled, !mod.Enabled);
txInner.Add(mod.Id, Mod.Revision, mod.Revision + 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this use mod.Revise(txInner)?

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can still reproduce:

  • Adding mod to loadout multiple times causes exception vm.Value Binding received an Exception!|System.Collections.Generic.KeyNotFoundException: Entity EId:200000000003BFF does not have attribute NexusMods.Abstractions.Loadouts.Mods.Mod/Version in C:\Dev\Repos\NexusMods.App-code-review-tree\src\NexusMods.App.UI\Pages\LoadoutGrid\Columns\ModVersion\ModVersionViewModel.cs:line 15
  • Completed Downloads section does not show in the UI, completed downloads don't appear in it

I think for the second one completed downloads are there, but they disappears if you close and reopen the app?

@halgari
Copy link
Collaborator Author

halgari commented Jun 26, 2024

@Al12rs I fixed the completed items not showing up after a restart, can you confirm that you are still seeing the .Version issue after deleting the local state of the app? I can't get this error to happen, even with downloading multiple files, adding multiple copies, etc.

@Al12rs
Copy link
Contributor

Al12rs commented Jun 26, 2024

I tested again and can no longer reproduce 👍

@halgari halgari merged commit 727c0ee into main Jun 26, 2024
11 of 12 checks passed
@halgari halgari deleted the source-generator-read-models branch June 26, 2024 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants