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

Initial Loadout Items #1750

Merged
merged 9 commits into from
Jul 17, 2024
Merged

Conversation

erri120
Copy link
Member

@erri120 erri120 commented Jul 11, 2024

Part of #1336.

I believe that this super simple design will get us very far. The main concern was "mods" and how we represent them. With the advent of collections, we'd also need to figure out how to represent those as well.

The simplest solution is the following (editor):

erDiagram
    LoadoutItem {
        string Name
        bool IsDisabled
        Loadout Loadout
        OptionalLoadoutItemGroup Parent
    }

    LoadoutItemWithTargetPath {
        GamePath TargetPath
    }

    LoadoutItemWithTargetPath ||--|| LoadoutItem : includes

    LoadoutFile {
        Hash Hash
        Size Size
    }

    DeletedFile
    GeneratedFile

    LoadoutFile ||--|| LoadoutItemWithTargetPath : includes
    DeletedFile ||--|| LoadoutItemWithTargetPath : includes
    GeneratedFile ||--|| LoadoutItemWithTargetPath : includes

    LoadoutItemGroup {
        LoadoutItem[] Children
    }

    LibraryLinkedLoadoutItem {
        LibraryItem LibraryItem
    }

    LibraryItem
    LibraryLinkedLoadoutItem ||--|| LoadoutItem : includes
    LibraryLinkedLoadoutItem ||--|| LibraryItem : references

    LoadoutItemGroup ||--|| LoadoutItem : includes

    LoadoutItemGroup ||--|{ LoadoutItem : references
Loading

With this design, we can easily represent mods, collections, and other stuff using groups. Besides having groups, a LoadoutItem has a Source. I'm not 100% whether we want to have this be required or not, but what this allows us to do, is create a link between the Loadout and the Library added in my recent PRs.

The UI for the library is going to show library items that have been added manually or downloaded via Nexus Mods. For the Loadout, we're essentially going to do the same: get all loadout items where the library item is one of the desired types.

This massively simplifies how we make the relationship between the loadout and library in the UI clear to the user, and it allows us to implement many UI designs much more easily.

@erri120 erri120 requested a review from a team July 11, 2024 09:38
@erri120 erri120 self-assigned this Jul 11, 2024
@halgari halgari marked this pull request as ready for review July 11, 2024 12:36
@halgari halgari marked this pull request as draft July 11, 2024 12:37
@halgari
Copy link
Collaborator

halgari commented Jul 11, 2024

Like this, but a few comments (didn't mean to accidentally convert it from a draft earlier):

Not all LoadoutFiles will have a Hash/Size, reified deletes are the current example, but generated files would be another. I think we should split apart LoadoutFile into a model with To and then have something else like "ExtractedFile" or "LibraryFile" or something that combines the To with Hash and Size.

@halgari
Copy link
Collaborator

halgari commented Jul 11, 2024

I'm not a fan of the current design's (code in main) ModCategory property, that can be : GameFiles, Mods, Overrides, etc, But it does get used in several places, mostly as a way to figure out what the type of a given group is, so that we can add files to it later on. We could implement this via other models, but we should probably add GameFiles and Overrides to this design

@erri120
Copy link
Member Author

erri120 commented Jul 11, 2024

I'm not a fan of the current design's (code in main) ModCategory property, that can be : GameFiles, Mods, Overrides, etc, But it does get used in several places, mostly as a way to figure out what the type of a given group is, so that we can add files to it later on. We could implement this via other models, but we should probably add GameFiles and Overrides to this design

This is where the LibraryItem comes in. Implementations would check the LibraryItem or just the exact type of the LoadoutItem to know what "category" it is. Categories are replaced by explicit types.

@erri120
Copy link
Member Author

erri120 commented Jul 11, 2024

@Al12rs and I couldn't come up with better names for LoadoutItemWithTargetPath and LoadoutItemWithLibraryItem, so suggestions are welcomed.

@erri120 erri120 changed the title Initial Library Items Initial Loadout Items Jul 15, 2024
@erri120 erri120 mentioned this pull request Jul 15, 2024
70 tasks
@erri120 erri120 marked this pull request as ready for review July 16, 2024 09:14
@erri120 erri120 requested review from halgari and Al12rs and removed request for a team July 16, 2024 09:14
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.

Ok from me, but I'd like to hear from @halgari when he comes back today for a few things above

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.

Tests are failing, it seems parts of the app still expect DeletedFile to have Hash and size since they were including File before.

@erri120 erri120 requested a review from Al12rs July 17, 2024 09:53
@erri120
Copy link
Member Author

erri120 commented Jul 17, 2024

@Al12rs everything should pass. I've temporarily disabled the DisabledFile model since it was conflicting with the existing model.

@erri120 erri120 merged commit 52dacdd into Nexus-Mods:main Jul 17, 2024
12 checks passed
@erri120 erri120 deleted the feat/1336-loadout-items branch July 17, 2024 10:54
@Sewer56 Sewer56 mentioned this pull request Jul 31, 2024
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