From a55a59e5a5abc2351fe946b0428371ebe85661ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Wed, 21 Feb 2024 15:01:06 +0200 Subject: [PATCH 1/3] Remove code that's never executed The if-branch checks a variable that has been set to false just two lines ago. I'm not sure if this is a bug that should be handled some other way, but at least this part of the code has remained the same for four years, so probably it's not very critical bug at least. --- src/pages/Profiles.vue | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pages/Profiles.vue b/src/pages/Profiles.vue index 74e500ba1..62f126dc0 100644 --- a/src/pages/Profiles.vue +++ b/src/pages/Profiles.vue @@ -375,9 +375,6 @@ export default class Profiles extends Vue { closeNewProfileModal() { this.addingProfile = false; this.renamingProfile = false; - if (this.addingProfile) { - document.dispatchEvent(new CustomEvent("created-profile", {detail: ''})); - } } removeProfile() { From 08a6b0268dc434178bd8bfa20336149ca6223ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Wed, 21 Feb 2024 17:37:00 +0200 Subject: [PATCH 2/3] Store active profile in Vuex Do changes to active profile via Vuex methods. Reading values from Vuex are covered in separate commits. This commit covers all cases where the active profile is changed, except for: - CacheUtil.clean(): I considered this not required at this time as the method only iterates over profiles and sets back the original profile in the end. This can be revisited if we want to decommission the current implementation in Profile.ts completely - Test cases I'm not particularly happy with the fact that setActiveProfile mutator accepts profile name as a string instead of a profile object. This too can be revisited once the refactoring has progressed a bit. Now there were some cases where the active profile was changed without updating the persistent settings, and having separate actions or one action with a more complex payload seemed like overkill. --- src/pages/Profiles.vue | 28 ++++++++++++---------- src/store/modules/ProfileModule.ts | 38 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/pages/Profiles.vue b/src/pages/Profiles.vue index 62f126dc0..b77b31753 100644 --- a/src/pages/Profiles.vue +++ b/src/pages/Profiles.vue @@ -278,6 +278,8 @@ export default class Profiles extends Vue { private profileList: string[] = ['Default']; + // The profile currently selected from the profileList, which may or + // may not be the "active profile" stored in Profile.ts and Vuex. private selectedProfile: string = ''; private addingProfile: boolean = false; @@ -338,12 +340,15 @@ export default class Profiles extends Vue { await this.updateProfileList(); } - selectProfile(profile: string) { - new Profile(profile); + // User selected a profile from the list of existing profile. + // This does not mean the "Select profile" button was pressed yet. + async selectProfile(profile: string) { this.selectedProfile = profile; - settings.setProfile(profile); + await this.$store.dispatch('profile/updateActiveProfile', profile); } + // Open modal for entering a name for a new profile. Triggered + // either through user action or profile importing via file or code. newProfile(type: string, nameOverride: string | undefined) { this.newProfileName = nameOverride || ''; this.addingProfile = true; @@ -355,18 +360,21 @@ export default class Profiles extends Vue { }); } + // User confirmed creation of a new profile with a name that didn't exist before. + // The profile can be either empty or populated via importing. createProfile(profile: string) { const safeName = this.makeProfileNameSafe(profile); if (safeName === '') { return; } - new Profile(safeName); + this.$store.commit('profile/setActiveProfile', safeName); this.profileList.push(safeName); this.selectedProfile = Profile.getActiveProfile().getProfileName(); this.addingProfile = false; document.dispatchEvent(new CustomEvent("created-profile", {detail: safeName})); } + // User confirmed updating an existing profile via importing. updateProfile() { this.addingProfile = false; document.dispatchEvent(new CustomEvent("created-profile", {detail: this.selectedProfile})); @@ -401,12 +409,9 @@ export default class Profiles extends Vue { } } } - new Profile('Default'); + await this.$store.dispatch('profile/updateActiveProfile', 'Default'); this.selectedProfile = Profile.getActiveProfile().getProfileName(); - const settings = await ManagerSettings.getSingleton(this.activeGame); - await settings.setProfile(Profile.getActiveProfile().getProfileName()); - this.closeRemoveProfileModal(); } @@ -534,7 +539,7 @@ export default class Profiles extends Vue { await FileUtils.emptyDirectory(path.join(Profile.getDirectory(), profileName)); await fs.rmdir(path.join(Profile.getDirectory(), profileName)); } - new Profile(profileName); + this.$store.commit('profile/setActiveProfile', profileName); } if (parsed.getMods().length > 0) { this.importingProfile = true; @@ -566,7 +571,7 @@ export default class Profiles extends Vue { } } if (this.importUpdateSelection === 'UPDATE') { - new Profile(event.detail); + this.$store.commit('profile/setActiveProfile', event.detail); try { await FileUtils.emptyDirectory(path.join(Profile.getDirectory(), event.detail)); } catch (e) { @@ -656,8 +661,7 @@ export default class Profiles extends Vue { settings = await ManagerSettings.getSingleton(this.activeGame); await settings.load(); - this.selectedProfile = settings.getContext().gameSpecific.lastSelectedProfile; - new Profile(this.selectedProfile); + this.selectedProfile = await this.$store.dispatch('profile/loadLastSelectedProfile'); // Set default paths if (settings.getContext().gameSpecific.gameDirectory === null) { diff --git a/src/store/modules/ProfileModule.ts b/src/store/modules/ProfileModule.ts index 215e0c771..90e61d466 100644 --- a/src/store/modules/ProfileModule.ts +++ b/src/store/modules/ProfileModule.ts @@ -17,6 +17,7 @@ import ProfileModList from '../../r2mm/mods/ProfileModList'; import SearchUtils from '../../utils/SearchUtils'; interface State { + activeProfile: Profile | null; modList: ManifestV2[]; order?: SortNaming; direction?: SortDirection; @@ -31,6 +32,7 @@ export default { namespaced: true, state: (): State => ({ + activeProfile: null, modList: [], order: undefined, direction: undefined, @@ -39,6 +41,22 @@ export default { }), getters: >{ + activeProfile(state) { + if (state.activeProfile !== null) { + return state.activeProfile; + } + + console.warn("Called profile/activeProfile but profile is not set. Falling back to Profile provider."); + const profile = Profile.getActiveProfile(); + + if (profile !== undefined) { + return profile; + } + + console.warn("Called Profile.getActiveProfile but profile is not set. Falling back to Default profile."); + return new Profile('Default'); + }, + modsWithUpdates(state, _getters, rootState): ThunderstoreCombo[] { return ThunderstoreDownloaderProvider.instance.getLatestOfAllToUpdate( state.modList, @@ -78,6 +96,15 @@ export default { }, mutations: { + // Use updateActiveProfile action to ensure the persistent + // settings are updated. + setActiveProfile(state: State, profileName: string) { + // Stores the active profile in Profile.ts. + const profile = new Profile(profileName); + + state.activeProfile = profile; + }, + // Avoid calling this directly, prefer updateModList action to // ensure TSMM specific code gets called. setModList(state: State, list: ManifestV2[]) { @@ -175,6 +202,12 @@ export default { } }, + async loadLastSelectedProfile({commit, rootGetters}): Promise { + const profileName = rootGetters['settings'].getContext().gameSpecific.lastSelectedProfile; + commit('setActiveProfile', profileName); + return profileName; + }, + async loadOrderingSettings({commit, rootGetters}) { const settings: ManagerSettings = rootGetters['settings']; commit('setOrder', settings.getInstalledSortBy()); @@ -182,6 +215,11 @@ export default { commit('setDisabledPosition', settings.getInstalledDisablePosition()); }, + async updateActiveProfile({commit, rootGetters}, profileName: string) { + commit('setActiveProfile', profileName); + rootGetters['settings'].setProfile(profileName); + }, + async updateDirection({commit, rootGetters}, value: SortDirection) { commit('setDirection', value); rootGetters['settings'].setInstalledSortDirection(value); From 4f64064ea9f59f1dcdc0e808b024560187b97136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Thu, 22 Feb 2024 13:49:18 +0200 Subject: [PATCH 3/3] Change the relation between selectedProfile and active profile - Profiles.vue stored selectedProfile ("selected in the UI from the list of available profiles") internally - Sometimes when the selectedProfile changed, activeProfile (stored in Profile.ts and Vuex) was updated, and sometimes not. Sometimes the last selected profile setting in persistent storage was updated and sometimes not - This was changed so that updating the selectedProfile always updates activeProfile and the persistent setting. I assume this is more correct behaviour, if not for any other reason, then to make the behaviour consistent and predictable - One exception to this is the temporary folder when renaming a profile is not saved to persitent settings - One issue with the approach is that the synchronous setter method now calls an async dispatch operation. I tested all the flows I could think of, and they all worked fine - Fixed an issue where the selected profile was no longer highlighted in the profile list after it was renamed --- src/pages/Profiles.vue | 39 +++++++++++++----------------- src/store/modules/ProfileModule.ts | 4 +++ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/pages/Profiles.vue b/src/pages/Profiles.vue index b77b31753..2adadf3d9 100644 --- a/src/pages/Profiles.vue +++ b/src/pages/Profiles.vue @@ -44,7 +44,7 @@