Skip to content

Added: Replace Library Items Atomically in Place & Other Library Refactorings #3199

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented May 15, 2025

This PR adds a few things:

  • The ability to 'replace' instances of a LibraryItem with another LibraryItem across all loadouts.
    • Including filter for read only collections.
  • A small cleanup of API usage; removing duplicate operations.
    • e.g. Removing library linked loadout items is now in LibraryService, rather than duplicated in multiple classes.
  • Fix: Installing an item uses latest DB state rather than one from the item.
    • Fixes a potential issue where a user:
      • Opens Library View
      • Adds a collection from the browser
      • Installs a Library Item to that collection
    • And the operation fails because collection didn't exist at the time the library item was read from the DB.
  • Some other library APIs and refactorings, such as LoadoutsWithLibraryItem API.

Replace operation does not preserve user ingested files such as configs.

Some of the new APIs for the LibraryService also query or interact with loadouts.
These can live in either a Loadout service, a Library service (currently), or another service.
For now I just added them to existing service. Thoughts?


The ability to properly 'replace' here is dependent on the ability for installers to not leave junk should they suddenly decide that a mod may be incompatible mid way through.

Therefore, I've gone through the effort of reviewing every single installer to ensure that skipping/termination happens before anything is added to the current transaction:

Installer No Junk Comment
AdvancedManualInstallerUI [X] Adds to tx after user made choices, if cancelled, tx is not touched.
AdvancedManualInstaller [X]
FomodXmlInstaller [X] Bails out before adding to tx
GenericPatternMatchInstaller [X] Succeeds if any file was deployed. Any file means tx was used at least once. No early terminya~tion after tx.
BG3SEInstaller [X] Bails out early if DLL Hijacking stub DLL is missing.
BLSEInstaller [X] Bails if no BLSE launcher present.
BannerlordModInstaller [X] Bails out if no module info, else proceeds till end.
AppearancePresetInstaller [x] Only creates items and uses tx if an ".preset" is found. Success/failure depends if any item was found.
FolderlessModInstaller [x] Matches .archive and docs files. Only uses tx if any found, and finding indicates success.
RedModInstaller [x] Bails out if no info.json found, redModInfoFile == null can terminate but that throws, failing all, so we're good.
SimpleOverlayModInstaller [x] Exits if no roots, else exits only if 1 file.
GenericInstaller [x] All checks done beforehand.
SMAPIInstaller [x] Bails out early. May encounter errors later, but this does not return unsupported.
FallbackCollectionDownloadInstaller [x] Always succeeds.

There are cases where installers throw exceptions if something 'exceptional' happens; but that terminates the whole operation rather than delegating to next, which is considered ok in this scenario.

@Sewer56 Sewer56 marked this pull request as ready for review May 15, 2025 13:43
Comment on lines +76 to 194

/// <summary>
/// Unlinks a number of items added by <see cref="InstallItem"/> function call from a loadout.
/// </summary>
/// <param name="itemIds">The <see cref="LibraryLinkedLoadoutItem"/>s to remove</param>
/// <param name="tx">Existing transaction to use</param>
void RemoveLibraryItemsFromLoadout(IEnumerable<LibraryLinkedLoadoutItemId> itemIds, ITransaction tx);

/// <summary>
/// Removes library items (originally installed via <see cref="InstallItem"/>) from all
/// loadouts using an existing transaction
/// </summary>
/// <param name="libraryItems">The library items to remove from the loadouts</param>
/// <param name="tx">Existing transaction to use</param>
void RemoveLibraryItemsFromAllLoadouts(IEnumerable<LibraryItem.ReadOnly> libraryItems, ITransaction tx);

/// <summary>
/// Removes library items (originally installed via <see cref="InstallItem"/>) from all
/// loadouts with automatic transaction
/// </summary>
/// <param name="libraryItems">The library items to remove from the loadouts</param>
Task RemoveLibraryItemsFromAllLoadouts(IEnumerable<LibraryItem.ReadOnly> libraryItems);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="oldItem">The library item to be replaced</param>
/// <param name="newItem">The replacement library item</param>
/// <param name="options">Options regarding how to replace this library item.</param>
/// <param name="tx">The transaction to use</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemInAllLoadouts(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem, ReplaceLibraryItemOptions options, ITransaction tx);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="replacements">The replacements to perform</param>
/// <param name="options">Options regarding how to replace these library items.</param>
/// <param name="tx">The transaction to add this replace operation to.</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemsInAllLoadouts(IEnumerable<(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem)> replacements, ReplaceLibraryItemsOptions options, ITransaction tx);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="replacements">The replacements to perform</param>
/// <param name="options">Options regarding how to replace this library item.</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemsInAllLoadouts(IEnumerable<(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem)> replacements, ReplaceLibraryItemsOptions options);
}

/// <summary>
/// Represents the result of a <see cref="ILibraryService.ReplaceLibraryItemInAllLoadouts"/> operation.
/// </summary>
public enum LibraryItemReplacementResult
{
/// <summary>
/// The operation was successful.
/// </summary>
Success,

/// <summary>
/// The operation failed (unknown reason).
/// </summary>
FailedUnknownReason,
}

/// <summary>
/// Options for the <see cref="ILibraryService.ReplaceLibraryItemInAllLoadouts(NexusMods.Abstractions.Library.Models.LibraryItem.ReadOnly,NexusMods.Abstractions.Library.Models.LibraryItem.ReadOnly,ReplaceLibraryItemOptions,NexusMods.MnemonicDB.Abstractions.ITransaction)"/>
/// API.
/// </summary>
public struct ReplaceLibraryItemOptions
{
/// <summary>
/// Ignores items in ReadOnly collections such as collections from Nexus Mods.
/// </summary>
public bool IgnoreReadOnlyCollections { get; set; }
}

/// <summary>
/// Options for the 'ReplaceLibraryItemsInAllLoadouts' API.
/// </summary>
public struct ReplaceLibraryItemsOptions
{
/// <summary>
/// Ignores items in ReadOnly collections such as collections from Nexus Mods.
/// </summary>
public bool IgnoreReadOnlyCollections { get; set; }

/// <summary>
/// Gets the <see cref="ReplaceLibraryItemOptions"/> for this <see cref="ReplaceLibraryItemsOptions"/>
/// </summary>
public ReplaceLibraryItemOptions ToReplaceLibraryItemOptions() => new() { IgnoreReadOnlyCollections = IgnoreReadOnlyCollections };
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need all of these methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

The overloads with a tx allow you to do the operations as part of a larger transaction without committing.
The overloads without a create a transaction there and then, and do the changes on the spot.

RemoveLibraryItemsFromLoadout I added because the code for that was duplicated in multiple places throughout the app; so I lifted it out and deduplicated.

@Al12rs
Copy link
Contributor

Al12rs commented May 19, 2025

@Sewer56 Build is failing

@Sewer56
Copy link
Member Author

Sewer56 commented May 19, 2025

@Sewer56 Build is failing

I guess something must have changed on main since, hmmmmm.

I'm currently debugging on deck, so will see later. This is probably a trivial fix though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants