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

Loadout Items, Library Items, and Downloads #1336

Closed
erri120 opened this issue May 13, 2024 · 14 comments
Closed

Loadout Items, Library Items, and Downloads #1336

erri120 opened this issue May 13, 2024 · 14 comments
Assignees
Labels
Epic: Loadouts This is related to Loadouts Epic: Tech-debt Technical debt, this needs solving in the long-term Tech: code-architecture-design Involves design or redesign of the code architecture Tech: data-store This is related to the Data Store.

Comments

@erri120
Copy link
Member

erri120 commented May 13, 2024

Ever since the beginning of the project, a Loadout contained "Mods":

/// <summary>
/// Collection of mods.
/// </summary>
public required EntityDictionary<ModId, Mod> Mods { get; init; }

Which the new DB and PR #1302, this changed to "Mods" specifying the Loadout they are in:

public LoadoutId LoadoutId
{
get => LoadoutId.From(Mod.Loadout.Get(this));
set => Mod.Loadout.Add(this, value.Value);
}

/// <summary>
/// Gets all mods in this loadout.
/// </summary>
public Entities<EntityIds, Mod.Model> Mods => GetReverse<Mod.Model>(Mod.Loadout);

We now have a reversed relationship between Loadouts and "Mods". Instead of getting a Loadout and then having access to its "Mods", we can now just get every Entity that is a Mod and has a LoadoutId property that matches the value of the Loadout we're trying to look up.

This issue is about removing the Mod model, and replacing it with a generic LoadoutItem model.

We've been trying to shoehorn everything into our Mod model. This is best illustrated by looking at our ModCategory enum that describes what the Mod is:

public enum ModCategory
{
GameFiles,
Mod,
Saves,
Overrides,
/// <summary>
/// Files maintained by the game's framework, not intended for direct user modification.
/// </summary>
Metadata,
}

Translating this into English, we're saying that a Mod can be "Game Files", it can be "Saves", it can be a "Mod"...

The abstraction that we have, of Loadouts containing Mods, is woefully inadequate. The effects of this can already be seeing by reading #1195 and #1146. We don't even have two Mods installed and already our system is not doing the things we want it to do.

Other mod managers also have an issue with shoehorning everything into a "Mod". In Vortex (@insomnious), a Nexus Mods Collection is represented as a Mod, in Mod Organizer 2 (@Al12rs) a "Mod" is literally just a subdirectory in the mods directory.

Continuing to use "Mods" to represent the content of a Loadout will just lead to more and more issues, that'll require weird hacks, and that'll incur a lot of tech debt.

However, which the new DB, we can go down a different path. Loadouts won't be containing "Mods" anymore. Instead, we'll have Loadout Items that reference a Loadout. A Loadout Item can be anything: Game Files, Configs, SMAPI Mods, Collections. Since Entities in the new DB are just collections of attributes that we gave a type to make it work in C#, each individual system can just query for the specific attributes it needs.

The apply process wouldn't go through all "Mods" to get the file trees, it would just query all Entities that reference the current Loadout that has a "File Bucket" attribute or similar.

While this is primarily a backend change, doing this allows the UI to be much richer. We could show all Loadout Items like this:

2024-05-13_11-13-31

Here we have "Game Files" because a Loadout contains those, we have SMAPI, and then we have an "Item Group" called "Raised Garden Beds". This item group contains three SMAPI Mods as well as a "Config" item that we created when the user launched the game, and it created a config file for the SMAPI Mod. Our new model would allow all of these to be different types of Items that have relationships between one another.

Whether we show such a view is up to the designers, but what this new model allows us to do is make the "Loadout Mods" view easier as well:

2024-05-13_11-16-57

Instead of using stuff like the ModCategory enum to filter out "Mods" we don't want to show in this view, we'll use the Loaodut Items and special attributes to figure out what we want to show.

Collections would also be easier to implement with this, since it'll just be another Item that gets referenced by all the Items it contains.

Overall, going from "Mods" to Loadout Items will make our data model much easier and simpler, as well as allow us to build better UIs. We won't be able to escape the term "Mod" when making a "Mod Manager" but we won't be forced to use it in our backend.

As a small postfix to this post, a "Mod" is just a list of instructions on how to modify something that we humans gave a name to. Outside of modding games, mods exists for cars and other real things as well.

@erri120 erri120 added this to MVP May 13, 2024
@Al12rs
Copy link
Contributor

Al12rs commented May 13, 2024

Something to note is that the App actually mostly only cares about the so called FileBuckets, which is what it will actually need to install the files.
The FileBuckets are the performance critical parts of the code that we really need to be able to reason around efficiently architecture wise.

How these FileBuckets are shown in the UI and are mapped to what users traditionally think of mods is mostly a user experience issue of making things clear and easy.

The proposal above aims to move our architecture from a simple FileBucket system (which we currently improperly call mods), to something that is closer to how we might want the UI to show things.

I think that the FileBucket architecture being efficient is more important than proper mapping of UI concepts into our architecture, but this isn't either or, we can have both. Just I wouldn't want to compromise efficiency.

Apart from UI and Game specific features, Mod Updates are the main feature of the App that would likely be influenced for the better by a more accurate mapping of loadout entities into architecture.

@erri120
Copy link
Member Author

erri120 commented May 15, 2024

@halgari we should use this issue to plan out the core types we'll be using, as well as how the new source generator would be used here.

I'm thinking that a Loadout Item should only consist of a name, a reference to a Loadout, and an optional reference to a "parent"/group which itself is just another Loadout Item:

interface ILoadoutItem
{
  EntityId ItemId;
  LoadoutId LoadoutId;
  Optional<EntityId> ParentItemId;
}

We should also have a marker attribute like HasFiles and Files would contain a reference to a Loadout Item with this marker attribute. In code, we'd want a separate interface:

// better name welcomed
interface ILoadoutItemWithFiles : ILoadoutItem
{
  // optionally can have methods like GetFiles() or ToFileTree(), but this shouldn't really have any properties.
}

Additional metadata like the version should be in separate interfaces as well. I'd almost say we want separate entities for metadata entirely, similar to how we'd do "layers".

Just some ideas, and as you hopefully notice, not a single mention of "mod".

@erri120 erri120 added Epic: Loadouts This is related to Loadouts Tech: data-store This is related to the Data Store. Tech: code-architecture-design Involves design or redesign of the code architecture Epic: Tech-debt Technical debt, this needs solving in the long-term labels Jun 4, 2024
This was referenced Jun 24, 2024
@erri120 erri120 self-assigned this Jul 1, 2024
@erri120
Copy link
Member Author

erri120 commented Jul 1, 2024

Notes after chatting with @captainsandypants and @Al12rs. Will write a separate comment about the "My Mods" page.

Downloads

First off, the data for our downloads should be better organized and simplified. The new designs for the UI heavily make use of the Nexus Mods mod pages.

Files that we "download", be that archives, executables, or other, would get metadata attached to them, matching the data available at the "origin". All of this data is also not local to a Loadout, but global:

(NOTE: LocalFile includes DownloadedFile not the other way around, I couldn't make the diagram look decent the other way around)

erDiagram
    DownloadedFile {
        string FileName
        ulong Size
        Hash Hash
        DateTime DateDownloaded
    }
    NexusModsFile {
        string Name
        string Version
        int FileId
        NexusModsPage Page
    }
    NexusModsPage {
        string Name
        Image Screenshot
        int ModId
    }
    LocalFile {
        string Path
    }

    DownloadedFile ||--|| LocalFile : includes
    DownloadedFile ||--|| NexusModsFile : includes
    NexusModsFile }|--|| NexusModsPage : references

    Archive {
        DownloadedFile Origin
    }
    ArchiveFile {
        stirng FileName
        ulong Size
        Hash Hash
        Archive Parent
    }
    NestedArchive

    DownloadedFile ||--|| Archive : references
    Archive ||--|{ ArchiveFile : references
    Archive ||--|| NestedArchive : includes
    ArchiveFile ||--|| NestedArchive : includes
Loading

If a user adds a file from disk, we'd create a new LocalFile entity. If the user downloads via Nexus Mods, we create a NexusModsFile entity, and check link it to an existing NexusModsPage or create a new one.

If the downloaded file is an archive, we run it through the extractor and create new Archive and ArchiveFile entities. I put NestedArchive in there and made it include both Archive and ArchiveFile but I'm not 100% sure about that.

This model can also be expanded with a NexusModsCollection model that includes DownloadedFile as well, since a collection is also downloaded as an archive:

erDiagram
    NexusModsCollection {
        NexusModsFile[] Files
    }

    NexusModsCollection ||--|| DownloadedFile : includes
    NexusModsCollection ||--|{ NexusModsFile : references
Loading

The new "Mods Library" page designed by @captainsandypants will display DownloadedFile entities. How it's displayed is beside the point, we just need to make sure the data is easily obtainable.

@Al12rs
Copy link
Contributor

Al12rs commented Jul 1, 2024

I'm still not happy with some of the names, like DownloadedFile that can represents a LocalFile if I understood the architecture correctly.

I'm thinking about some potential alternatives like UserArchive or based on function InstallableFile

@erri120
Copy link
Member Author

erri120 commented Jul 1, 2024

@Al12rs what about LibraryFile?

@Al12rs
Copy link
Contributor

Al12rs commented Jul 1, 2024

@Al12rs what about LibraryFile?

I like that more than everything else so far, I think that could work.
I would actually propose LibraryItem for symmetry with LoadoutItem, what do you think?
I guess I'm proposing making a hierarchy here, similar to what we are going to do with Loaodut items

We can rename the FileOriginsPage to LibraryPage since we have a clearer idea what the Library term actually means now.

@erri120
Copy link
Member Author

erri120 commented Jul 1, 2024

@Al12rs what about LibraryFile?

I like that more than everything else so far, I think that could work.
I would actually propose LibraryItem for symmetry with LoadoutItem, what do you think?
I guess I'm proposing making a hierarchy here, similar to what we are going to do with Loaodut items

We can rename the FileOriginsPage to LibraryPage since we have a clearer idea what the Library term actually means now.

LibraryItem also came to me a minute after writing LibraryFile. This might also cover things that aren't necessarily a single file.

@erri120
Copy link
Member Author

erri120 commented Jul 1, 2024

Updated relationships would look like this:

erDiagram
    LibraryItem {
        string Name
    }

    LibraryFile {
        ulong Size
        Hash Hash
        string FileName
    }

    NexusModsLibraryFile {
        NexusModsFileMetadata FileMetadata
    }

    NexusModsFileMetadata {
        string Name
        string Version
        int FileId
        NexusModsModPageMetadata Page
    }
    NexusModsModPageMetadata {
        string Name
        Image Screenshot
        int ModId
    }

    LocalFile {
        string Path
    }

    LibraryFile ||--|| LibraryItem : includes
    NexusModsLibraryFile ||--|| LibraryFile : includes
    LocalFile ||--|| LibraryFile : includes
    NexusModsLibraryFile ||--|| NexusModsFileMetadata : references
    NexusModsFileMetadata }|--|| NexusModsModPageMetadata : references

    LibraryArchive {
        LibraryArchiveFileEntry[] Children
    }
    LibraryArchiveFileEntry {
        stirng FileName
        ulong Size
        Hash Hash
        LibraryArchive Parent
    }
    NestedArchive

    LibraryArchive ||--|| LibraryFile : includes
    LibraryArchiveFileEntry }|--|| LibraryArchive : references
    NestedArchive ||--|| LibraryArchive : includes
    NestedArchive ||--|| LibraryArchiveFileEntry : includes
Loading

@Al12rs
Copy link
Contributor

Al12rs commented Jul 1, 2024

Cool! Nice work! Yeah I think this is pretty understandable.
I guess the NestedArchive would have a parent property? Or was it a deliberate decision not to have parent property on it?

@erri120
Copy link
Member Author

erri120 commented Jul 1, 2024

Cool! Yeah I think this is pretty understandable. I guess the NestedArchive would have a parent property? Or was it a deliberate decision not to have parent property on it?

I'm not sure what NestedArchive is supposed to be yet. It "includes" ArchiveFile which has a Parent that is an Archive but it's also an Archive itself. The idea for this, is to allow consumers to not care whether we have a nested archive or not, they just get an Archive and stuff works.

@halgari can we do double includes like this, or use a different approach?

@Al12rs
Copy link
Contributor

Al12rs commented Jul 1, 2024

Ah, I had missed the relationship with ArchiveFile, I see what you mean.

I don't think NestedArchive entity needs to include ArchiveFile and be the same entity, it could just hold a reference to it or something like that.

@halgari
Copy link
Collaborator

halgari commented Jul 1, 2024

We can have a double includes in the data model, but it's not exposed in a clear way via the source generators. What it would involve is writing our own "TryGetAsXX" and then doing a "reinterpret cast" into the other type. This is pretty much the way the code works internally, and it's about 10 lines of code.

@erri120
Copy link
Member Author

erri120 commented Jul 2, 2024

Notes from meeting with @Al12rs about download states.

We've realized that the actual data we want to put into the DB, in regard to downloads, is very limited:

erDiagram
    LibraryFile
    NexusModsLibraryFile {
        NexusModsFileMetadata FileMetadata
    }
    NexusModsLibraryFile ||--|| LibraryFile : includes
    NexusModsFileMetadata {
        string Name
        string Version
        int FileId
        NexusModsModPage Page
    }
    NexusModsModPageMetadata {
        string Name
        int ModId
    }
    NexusModsLibraryFile ||--|| NexusModsFileMetadata : references
    NexusModsFileMetadata }|--|| NexusModsModPageMetadata : references

    PersistedState {
        PersistedStatus Status
        OptionalSize TotalSize
    }
    NXMDownloadState {
        NexusModsFileMetadata FileMetadata
    }
    NXMDownloadState ||--|| PersistedState : includes
    NXMDownloadState ||--|| NexusModsFileMetadata : references
Loading

The downloads are currently storing data in the DB and in a "sidecar" file. We've come up with a much better and cleaner design for downloads that puts all the live data into these sidecar files. This data is going to be represented using a IDownloadActivity.

ADownloadTask, IFileOriginRegistry, IDownloadService and a ton of over types will be renamed, updated, and some will be removed entirely.

The new approach has the NXM handler create a NXMDownloadActivity that contains the data for the NXM download. This would get passed to a new ILibraryService that accepts the IDownloadActivity and calls StartAsync on it. The IDownloadActivity doesn't do the downloading itself, it just passes itself to its IDownloader instance, e.g. NXMDownloader. The downloader is responsible for actually downloading an updating the progression the download activity.

Once the download is finished, the library service will check if the file is an archive, if that's the case, it will create an extraction activity and pass it to the extractor. When the extractor returns, we create an analyze activity to get the hashes and sizes.

The library service calls IDownloadActivity.CreateLibraryFile on the original activity it received. This can also be a factory, but what this method is doing is taking in the name, hash, and size, to create a LibraryFile entity. For NXM downloads, this will be a NexusModsLibraryFile.

Finally, the library service will create Archive and ArchiveFiles and reference the LibraryFile.

@erri120 erri120 changed the title Move from Loadout Mods to Loadout Items Pp Jul 2, 2024
@erri120 erri120 changed the title Pp Loadout Items, Library Items, and Downloads Jul 2, 2024
@erri120 erri120 moved this to In Progress in MVP Jul 2, 2024
This was referenced Jul 2, 2024
This was referenced Jul 11, 2024
@erri120
Copy link
Member Author

erri120 commented Jul 17, 2024

Superseded by #1763.

@erri120 erri120 closed this as completed Jul 17, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in MVP Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic: Loadouts This is related to Loadouts Epic: Tech-debt Technical debt, this needs solving in the long-term Tech: code-architecture-design Involves design or redesign of the code architecture Tech: data-store This is related to the Data Store.
Projects
Archived in project
Development

No branches or pull requests

3 participants