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.

Sign up for GitHub

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

Improve performance when uninstalling multiple mods at once #1155

Merged
merged 6 commits into from
Jan 22, 2024
Merged

Improve performance when uninstalling multiple mods at once #1155

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cf950e7
Add LocalModList.updateModListAfterChange to reduce code duplication
anttimaki Jan 11, 2024
f716d11
Postpone updating local mod list to improve uninstall performance
anttimaki Jan 11, 2024
4020602
Show the mod being currently uninstalled in the LocalModList modal
anttimaki Jan 11, 2024
0a5b1eb
Disable uninstall button while mods are being uninstalled
anttimaki Jan 11, 2024
b64c89c
Reduce the likelihood of state management bugs
MythicManiac Jan 22, 2024
cb39207
Merge other recent LocalModList changes
MythicManiac Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 34 additions & 39 deletions src/components/views/LocalModList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,15 @@
@click="disableMod(selectedManifestMod)">
Disable
</button>
<button v-if="dependencyListDisplayType === 'uninstall'" class="button is-info"
<button v-if="dependencyListDisplayType === 'uninstall'"
class="button is-info"
:disabled="modBeingUninstalled !== null"
@click="uninstallMod(selectedManifestMod)">
Uninstall
</button>
<span v-if="modBeingUninstalled" class="tag is-warning margin-top--1rem margin-left">
Uninstalling {{ modBeingUninstalled }}
</span>
<button v-if="dependencyListDisplayType === 'view'" class="button is-info"
@click="selectedManifestMod = null">
Done
Expand Down Expand Up @@ -264,6 +269,7 @@ import SearchUtils from '../../utils/SearchUtils';
private showingDependencyList: boolean = false;
private selectedManifestMod: ManifestV2 | null = null;
private dependencyListDisplayType: string = 'view';
private modBeingUninstalled: string | null = null;

// Filtering
private sortDisabledPosition: SortLocalDisabledMods = this.settings.getInstalledDisablePosition();
Expand Down Expand Up @@ -330,19 +336,25 @@ import SearchUtils from '../../utils/SearchUtils';
return ModBridge.getCachedThunderstoreModFromMod(mod);
}

async updateModListAfterChange(updatedList: ManifestV2[]) {
await this.$store.dispatch("updateModList", updatedList);

const err = await ConflictManagementProvider.instance.resolveConflicts(updatedList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}

this.filterModList();
}

async moveUp(vueMod: any) {
const mod: ManifestV2 = new ManifestV2().fromReactive(vueMod);
const updatedList = await ProfileModList.shiftModEntryUp(mod, this.contextProfile!);
if (updatedList instanceof R2Error) {
this.$emit('error', updatedList);
return;
}
await this.$store.dispatch("updateModList",updatedList);
const err = await ConflictManagementProvider.instance.resolveConflicts(updatedList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}
this.filterModList();
await this.updateModListAfterChange(updatedList);
}

async moveDown(vueMod: any) {
Expand All @@ -352,12 +364,7 @@ import SearchUtils from '../../utils/SearchUtils';
this.$emit('error', updatedList);
return;
}
await this.$store.dispatch("updateModList", updatedList);
const err = await ConflictManagementProvider.instance.resolveConflicts(updatedList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}
this.filterModList();
await this.updateModListAfterChange(updatedList);
}

isLatest(mod: ManifestV2): boolean {
Expand All @@ -380,7 +387,7 @@ import SearchUtils from '../../utils/SearchUtils';
return Dependants.getDependencyList(mod, this.modifiableModList);
}

async performUninstallMod(mod: ManifestV2): Promise<R2Error | void> {
async performUninstallMod(mod: ManifestV2, updateModList=true): Promise<R2Error | void> {
const uninstallError: R2Error | null = await ProfileInstallerProvider.instance.uninstallMod(mod, this.contextProfile!);
if (uninstallError instanceof R2Error) {
// Uninstall failed
Expand All @@ -395,10 +402,8 @@ import SearchUtils from '../../utils/SearchUtils';
this.$emit('error', modList);
return modList;
}
await this.$store.dispatch("updateModList",modList);
const err = await ConflictManagementProvider.instance.resolveConflicts(modList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
if (updateModList) {
await this.updateModListAfterChange(modList);
}
}

Expand Down Expand Up @@ -438,47 +443,43 @@ import SearchUtils from '../../utils/SearchUtils';
this.$emit('error', updatedList);
return updatedList;
}
await this.$store.dispatch("updateModList", updatedList);
const err = await ConflictManagementProvider.instance.resolveConflicts(updatedList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}
this.filterModList();
await this.updateModListAfterChange(updatedList);
}

async uninstallMod(vueMod: any) {
let mod: ManifestV2 = new ManifestV2().fromReactive(vueMod);
try {
for (const dependant of Dependants.getDependantList(mod, this.modifiableModList)) {
const result = await this.performUninstallMod(dependant);
this.modBeingUninstalled = dependant.getName();
const result = await this.performUninstallMod(dependant, false);
if (result instanceof R2Error) {
this.$emit('error', result);
this.modBeingUninstalled = null;
return;
}
}
const result = await this.performUninstallMod(mod);
this.modBeingUninstalled = mod.getName();
const result = await this.performUninstallMod(mod, false);
if (result instanceof R2Error) {
this.$emit('error', result);
this.modBeingUninstalled = null;
return;
}
} catch (e) {
// Failed to uninstall mod.
const err: Error = e as Error;
this.$emit('error', err);
this.modBeingUninstalled = null;
LoggerProvider.instance.Log(LogSeverity.ACTION_STOPPED, `${err.name}\n-> ${err.message}`);
}
this.selectedManifestMod = null;
this.modBeingUninstalled = null;
const result: ManifestV2[] | R2Error = await ProfileModList.getModList(this.contextProfile!);
if (result instanceof R2Error) {
this.$emit('error', result);
return;
}
await this.$store.dispatch("updateModList", result);
const err = await ConflictManagementProvider.instance.resolveConflicts(result, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}
this.filterModList();
await this.updateModListAfterChange(result);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like there are a handful of code paths here which could in theory apply changes without refreshing the mod list, would it make sense to always refresh it before returning?

I'll push that change on top of this before merging since it seems a bit less prone to state management bugs that way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in b64c89c

}

showDependencyList(vueMod: any, displayType: string) {
Expand All @@ -492,7 +493,6 @@ import SearchUtils from '../../utils/SearchUtils';
const mod: ManifestV2 = new ManifestV2().fromReactive(vueMod);
if (this.getDependantList(mod).size === 0) {
this.performUninstallMod(mod);
this.filterModList();
} else {
this.showDependencyList(mod, DependencyListDisplayType.UNINSTALL);
}
Expand Down Expand Up @@ -552,12 +552,7 @@ import SearchUtils from '../../utils/SearchUtils';
this.$emit('error', updatedList);
return updatedList;
}
await this.$store.dispatch("updateModList",updatedList);
const err = await ConflictManagementProvider.instance.resolveConflicts(updatedList, this.contextProfile!);
if (err instanceof R2Error) {
this.$emit('error', err);
}
this.filterModList();
await this.updateModListAfterChange(updatedList);
}

updateMod(vueMod: any) {
Expand Down
3 changes: 3 additions & 0 deletions src/css/custom.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ code {
&--buttons {
margin-top: 0.5em;
}
&--1rem {
margin-top: 1rem;
}
}

.pre {
Expand Down