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

Jump to bottom

WIP: Callback stack refactoring and installation after download #1564

Open
wants to merge 1 commit into
base: download-vuex-poc
Choose a base branch
from
Open

WIP: Callback stack refactoring and installation after download #1564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 34 additions & 20 deletions src/components/views/DownloadModModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@
:value='activeDownloadProgress'
:className="['is-dark']"
/>
<div v-if="activeInstallProgress > 0">
<h3 class='title'>Installing {{activeDownloadModName}}</h3>
<p>{{Math.floor(activeInstallProgress)}}% complete</p>
<Progress
:max='100'
:value='activeInstallProgress'
:className="['is-dark']"
/>
</div>
</div>
</div>
<button class="modal-close is-large" aria-label="close" @click="closeModal();"></button>
Expand Down Expand Up @@ -145,6 +154,10 @@ let assignId = 0;
return this.$store.getters['modDownload/activeDownloadModName'];
}

get activeInstallProgress(): number {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Vuex implementation may return undefined as well, so using number as the return type here is a bug.

return this.$store.getters['modDownload/activeInstallProgress'];
}

get activeGame(): Game {
return this.$store.state.activeGame;
}
Expand Down Expand Up @@ -175,8 +188,8 @@ let assignId = 0;
failed: false,
};
DownloadModModal.allVersions.push([currentAssignId, progressObject]);
setTimeout(() => {
ThunderstoreDownloaderProvider.instance.download(profile.asImmutableProfile(), tsMod, tsVersion, ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
setTimeout(async () => {
const downloadedMods = await ThunderstoreDownloaderProvider.instance.download(profile.asImmutableProfile(), tsMod, tsVersion, ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
const assignIndex = DownloadModModal.allVersions.findIndex(([number, val]) => number === currentAssignId);
if (status === StatusEnum.FAILURE) {
if (err !== null) {
Expand All @@ -196,28 +209,29 @@ let assignId = 0;
}
DownloadModModal.allVersions[assignIndex] = [currentAssignId, obj];
}
}, async (downloadedMods: ThunderstoreCombo[]) => {
ProfileModList.requestLock(async () => {
for (const combo of downloadedMods) {
try {
await DownloadModModal.installModAfterDownload(profile, combo.getMod(), combo.getVersion());
} catch (e) {
return reject(
R2Error.fromThrownValue(e, `Failed to install mod [${combo.getMod().getFullName()}]`)
);
}
});
await ProfileModList.requestLock(async () => {
for (const combo of downloadedMods) {
try {
await DownloadModModal.installModAfterDownload(profile, combo.getMod(), combo.getVersion());
} catch (e) {
return reject(
R2Error.fromThrownValue(e, `Failed to install mod [${combo.getMod().getFullName()}]`)
);
}
const modList = await ProfileModList.getModList(profile.asImmutableProfile());
if (!(modList instanceof R2Error)) {
const err = await ConflictManagementProvider.instance.resolveConflicts(modList, profile);
if (err instanceof R2Error) {
return reject(err);
}
}
const modList = await ProfileModList.getModList(profile.asImmutableProfile());
if (!(modList instanceof R2Error)) {
const err = await ConflictManagementProvider.instance.resolveConflicts(modList, profile);
if (err instanceof R2Error) {
return reject(err);
}
return resolve();
});
}
return resolve();
});
}, 1);


});
}

Expand Down
14 changes: 7 additions & 7 deletions src/providers/ror2/downloading/ThunderstoreDownloaderProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ export default abstract class ThunderstoreDownloaderProvider {
* @param modVersion The version of the mod to download.
* @param ignoreCache Download mod even if it already exists in the cache.
* @param callback Callback to show the current state of the downloads.
* @param completedCallback Callback to perform final actions against. Only called if {@param callback} has not returned a failed status.
*/
public abstract download(profile: ImmutableProfile, mod: ThunderstoreMod, modVersion: ThunderstoreVersion,
ignoreCache: boolean,
callback: (progress: number, modName: string, status: number, err: R2Error | null) => void,
completedCallback: (modList: ThunderstoreCombo[]) => void): void;
public abstract download(
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • We could consider renaming this to "downloadWithDependencies" oslt
  • Depending on the callsites we might take in the main mod as a ThunderstoreCombo (I think we currently keep splitting and combining the mod & version unnecessarily in different places)

profile: ImmutableProfile, mod: ThunderstoreMod, modVersion: ThunderstoreVersion,
ignoreCache: boolean,
callback: (progress: number, modName: string, status: number, err: R2Error | null) => void
): Promise<ThunderstoreCombo[]>;

/**
* A top-level method to download exact versions of exported mods.
Expand Down Expand Up @@ -87,11 +87,11 @@ export default abstract class ThunderstoreDownloaderProvider {
* Iterate the {@class ThunderstoreCombo} array to perform the download for each mod.
* Progress to the next one recursively once the callback received has been successful.
*
* @param entries IterableIterator of entries for {@class ThunderstoreCombo} mods to download.
* @param entries The {@class ThunderstoreCombo} mods to download.
* @param ignoreCache Should mod be downloaded even if it already exists in the cache?
* @param callback See {@method download}
*/
public abstract queueDownloadDependencies(entries: IterableIterator<[number, ThunderstoreCombo]>, ignoreCache: boolean, callback: (progress: number, modName: string, status: number, err: R2Error | null) => void): void
public abstract queueDownloadDependencies(entries: ThunderstoreCombo[], ignoreCache: boolean, callback: (progress: number, modName: string, status: number, err: R2Error | null) => void): void

/**
* Generate the total count of mods to be downloaded. Cached mods are not included in this count unless download cache is disabled.
Expand Down
103 changes: 62 additions & 41 deletions src/r2mm/downloading/BetterThunderstoreDownloader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default class BetterThunderstoreDownloader extends ThunderstoreDownloader
let downloadCount = 0;
const downloadableDependencySize = this.calculateInitialDownloadSize(dependencies);

await this.queueDownloadDependencies(dependencies.entries(), ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
await this.queueDownloadDependencies(dependencies, ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
if (status === StatusEnum.FAILURE) {
callback(0, modName, status, err);
} else if (status === StatusEnum.PENDING) {
Expand All @@ -82,10 +82,12 @@ export default class BetterThunderstoreDownloader extends ThunderstoreDownloader
});
}

public async download(profile: ImmutableProfile, mod: ThunderstoreMod, modVersion: ThunderstoreVersion,
ignoreCache: boolean,
callback: (progress: number, modName: string, status: number, err: R2Error | null) => void,
completedCallback: (modList: ThunderstoreCombo[]) => void) {

public async download(
Copy link
Collaborator

Choose a reason for hiding this comment

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

These comments are based on just looking at the code so there's quite a change I'm missing something, but the approach taken here doesn't seem optimal to me. I would rather:

  1. Gather everything we're going to download (preferably into one array, but there might be a reason to handle the "main download" separately from the dependencies). Pretty much as the code currently does, although there might be some changes to clean up / simplify the implementation
  2. Instead of using queueDownloadDependencies (even a refactored one) use _downloadCombo and _saveDownloadResponse helpers like downloadImportedMods. I think the queueing makes the code harder to read.

profile: ImmutableProfile, mod: ThunderstoreMod, modVersion: ThunderstoreVersion,
ignoreCache: boolean,
callback: (progress: number, modName: string, status: number, err: R2Error | null) => void
): Promise<ThunderstoreCombo[]> {
let dependencies: ThunderstoreCombo[] = [];
await this.buildDependencySet(modVersion, dependencies, DependencySetBuilderMode.USE_EXACT_VERSION);
this.sortDependencyOrder(dependencies);
Expand All @@ -94,9 +96,11 @@ export default class BetterThunderstoreDownloader extends ThunderstoreDownloader
combo.setVersion(modVersion);
let downloadCount = 0;

let downloadedModsList: ThunderstoreCombo[] = [combo];

const modList = await ProfileModList.getModList(profile);
if (modList instanceof R2Error) {
return callback(0, mod.getName(), StatusEnum.FAILURE, modList);
throw modList;
}

let downloadableDependencySize = this.calculateInitialDownloadSize(dependencies);
Expand All @@ -122,30 +126,32 @@ export default class BetterThunderstoreDownloader extends ThunderstoreDownloader
callback(this.generateProgressPercentage(progress, downloadCount, downloadableDependencySize + 1), mod.getName(), status, err);
} else if (status === StatusEnum.SUCCESS) {
downloadCount += 1;
// If no dependencies, end here.
if (dependencies.length === 0) {
callback(100, mod.getName(), StatusEnum.PENDING, err);
completedCallback([combo]);
return;
}
}
});

// If dependencies, queue and download.
await this.queueDownloadDependencies(dependencies.entries(), ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
if (status === StatusEnum.FAILURE) {
callback(0, modName, status, err);
} else if (status === StatusEnum.PENDING) {
callback(this.generateProgressPercentage(progress, downloadCount, downloadableDependencySize + 1), modName, status, err);
} else if (status === StatusEnum.SUCCESS) {
callback(this.generateProgressPercentage(progress, downloadCount, downloadableDependencySize + 1), modName, StatusEnum.PENDING, err);
downloadCount += 1;
if (downloadCount >= dependencies.length + 1) {
callback(100, modName, StatusEnum.PENDING, err);
completedCallback([...dependencies, combo]);
}
}
});
// If no dependencies, end here.
if (dependencies.length === 0) {
callback(100, mod.getName(), StatusEnum.SUCCESS, null);
return downloadedModsList;
}

// If dependencies, queue and download.
let downloadedDependencies = await this.queueDownloadDependencies(dependencies, ignoreCache, (progress: number, modName: string, status: number, err: R2Error | null) => {
if (status === StatusEnum.FAILURE) {
callback(0, modName, status, err);
} else if (status === StatusEnum.PENDING) {
callback(this.generateProgressPercentage(progress, downloadCount, downloadableDependencySize + 1), modName, status, err);
} else if (status === StatusEnum.SUCCESS) {
callback(this.generateProgressPercentage(progress, downloadCount, downloadableDependencySize + 1), modName, StatusEnum.PENDING, err);
downloadCount += 1;
if (downloadCount >= dependencies.length + 1) {
callback(100, modName, StatusEnum.PENDING, err);
}
}
})
});

downloadedModsList.push(...downloadedDependencies);
return downloadedModsList;
}

public async downloadImportedMods(
Expand Down Expand Up @@ -197,20 +203,35 @@ export default class BetterThunderstoreDownloader extends ThunderstoreDownloader
return completedProgress + (progress * 1/total);
}

public async queueDownloadDependencies(entries: IterableIterator<[number, ThunderstoreCombo]>, ignoreCache: boolean, callback: (progress: number, modName: string, status: number, err: R2Error | null) => void) {
const entry = entries.next();
if (!entry.done) {
await this.downloadAndSave(entry.value[1] as ThunderstoreCombo, ignoreCache, async (progress: number, status: number, err: R2Error | null) => {
if (status === StatusEnum.FAILURE) {
callback(0, (entry.value[1] as ThunderstoreCombo).getMod().getName(), status, err);
} else if (status === StatusEnum.PENDING) {
callback(progress, (entry.value[1] as ThunderstoreCombo).getMod().getName(), status, err);
} else if (status === StatusEnum.SUCCESS) {
callback(100, (entry.value[1] as ThunderstoreCombo).getMod().getName(), status, err);
await this.queueDownloadDependencies(entries, ignoreCache, callback);
}
public async queueDownloadDependencies(
thunderstoreCombos: ThunderstoreCombo[],
ignoreCache: boolean,
callback: (progress: number, modName: string, status: number, err: R2Error | null) => void
): Promise<ThunderstoreCombo[]> {
const downloadTasks: Promise<void>[] = [];
const mods: ThunderstoreCombo[] = [];

thunderstoreCombos.forEach((mod) => {
const task = new Promise<void>((resolve, reject) => {
this.downloadAndSave(mod, ignoreCache, (progress: number, status: number, err: R2Error | null) => {
if (status === StatusEnum.FAILURE) {
callback(0, mod.getMod().getName(), status, err);
reject(err); // Reject the promise if there's a failure
} else if (status === StatusEnum.PENDING) {
callback(progress, mod.getMod().getName(), status, err);
} else if (status === StatusEnum.SUCCESS) {
callback(100, mod.getMod().getName(), status, err);
mods.push(mod);
resolve();
}
});
});
}

downloadTasks.push(task);
});

await Promise.all(downloadTasks);
return mods;
}

public calculateInitialDownloadSize(list: ThunderstoreCombo[]): number {
Expand Down
Loading
Loading