-
Notifications
You must be signed in to change notification settings - Fork 257
Use map lookups for LocalModList #2054
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,42 +2,64 @@ import ManifestV2 from '../../model/ManifestV2'; | |
|
|
||
| export default class Dependants { | ||
|
|
||
| public static getDependantList(manifestMod: ManifestV2, modList: ManifestV2[], dependantSet?: Set<ManifestV2>): Set<ManifestV2> { | ||
| private static buildDependantIndex(modList: ManifestV2[]): Map<string, ManifestV2[]> { | ||
| const index = new Map<string, ManifestV2[]>(); | ||
| for (const mod of modList) { | ||
| for (const dep of mod.getDependencies()) { | ||
| const depName = dep.substring(0, dep.lastIndexOf('-')); | ||
| if (!index.has(depName)) { | ||
| index.set(depName, []); | ||
| } | ||
| index.get(depName)!.push(mod); | ||
| } | ||
| } | ||
| return index; | ||
| } | ||
|
|
||
| private static buildModMap(modList: ManifestV2[]): Map<string, ManifestV2> { | ||
| const map = new Map<string, ManifestV2>(); | ||
| for (const mod of modList) { | ||
| map.set(mod.getName(), mod); | ||
| } | ||
| return map; | ||
| } | ||
|
|
||
| public static getDependantList( | ||
| manifestMod: ManifestV2, | ||
| modList: ManifestV2[], | ||
| dependantSet?: Set<ManifestV2>, | ||
| dependantIndex?: Map<string, ManifestV2[]> | ||
| ): Set<ManifestV2> { | ||
| const dependants = dependantSet || new Set<ManifestV2>(); | ||
| const index = dependantIndex || this.buildDependantIndex(modList); | ||
|
|
||
| // For all mods | ||
| modList.forEach(mod => { | ||
| // Get mod dependencies | ||
| mod.getDependencies().forEach(dependency => { | ||
| // If mod name equals mod trying to get dependants | ||
| if (dependency.startsWith(manifestMod.getName() + "-")) { | ||
| if (!dependants.has(mod)) { | ||
| dependants.add(mod); | ||
| // Get dependants of new mod | ||
| this.getDependantList(mod, modList, dependants).forEach(recursive => { | ||
| dependants.add(recursive); | ||
| }) | ||
| } | ||
| } | ||
| }) | ||
| }) | ||
| const directDependants = index.get(manifestMod.getName()) || []; | ||
| for (const mod of directDependants) { | ||
| if (!dependants.has(mod)) { | ||
| dependants.add(mod); | ||
| this.getDependantList(mod, modList, dependants, index); | ||
| } | ||
| } | ||
| return dependants; | ||
| } | ||
|
|
||
| public static getDependencyList(manifestMod: ManifestV2, modList: ManifestV2[], dependencySet?: Set<ManifestV2>): Set<ManifestV2> { | ||
| public static getDependencyList( | ||
| manifestMod: ManifestV2, | ||
| modList: ManifestV2[], | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should be replacing modList entirely with modMap otherwise we're having double the memory consumption |
||
| dependencySet?: Set<ManifestV2>, | ||
| modMap?: Map<string, ManifestV2> | ||
| ): Set<ManifestV2> { | ||
| const dependencies = dependencySet || new Set<ManifestV2>(); | ||
| manifestMod.getDependencies().forEach(dependant => { | ||
| modList.forEach(mod => { | ||
| if (dependant.startsWith(mod.getName() + "-")) { | ||
| if (!dependencies.has(mod)) { | ||
| dependencies.add(mod); | ||
| this.getDependencyList(mod, modList, dependencies).forEach(recursive => { | ||
| dependencies.add(recursive); | ||
| }) | ||
| } | ||
| } | ||
| }) | ||
| }) | ||
| const map = modMap || this.buildModMap(modList); | ||
|
|
||
| for (const depString of manifestMod.getDependencies()) { | ||
| const depName = depString.substring(0, depString.lastIndexOf('-')); | ||
| const mod = map.get(depName); | ||
| if (mod !== undefined && !dependencies.has(mod)) { | ||
| dependencies.add(mod); | ||
| this.getDependencyList(mod, modList, dependencies, map); | ||
| } | ||
| } | ||
| return dependencies; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,14 @@ export default { | |
| return state.modList; | ||
| }, | ||
|
|
||
| modMap(state): Map<string, ManifestV2> { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not rebuilding the map every time this getter is called? |
||
| const map = new Map<string, ManifestV2>(); | ||
| for (const mod of state.modList) { | ||
| map.set(mod.getName(), mod); | ||
| } | ||
| return map; | ||
| }, | ||
|
|
||
| // Swap the ManifestV2s to ThunderstoreMods as the latter knows the version number | ||
| // of the latest version, which we need when showing how mods will be updated. | ||
| modsWithUpdates(state, _getters, _rootState, rootGetters): ThunderstoreMod[] { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same outcome but you have changed the logic here. I'd probably say to revert so that it's a bit easier to read and doesn't rely on knowing the format as much.