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.

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

Develop #1286

Merged
merged 68 commits into from
Apr 6, 2024
Merged

Develop #1286

merged 68 commits into from
Apr 6, 2024

Conversation

ebkr
Copy link
Owner

@ebkr ebkr commented Apr 6, 2024

No description provided.

anttimaki and others added 30 commits February 22, 2024 14:41
There's no need to pass dependants list as a prop since it can be as
easily be determined internally.

Dependency list prop was dropped altogether since it's not used.
The implementation imitates what's currently being done by
LocalModList.performDisable().

This helps remove business logic and driect references to providers
from components, hopefully supporting reuse and easier refactoring.
The modal now handles disabling of mods entirely internally.
LocalModCard now disables mods without dependants internally, and
delegates mods with dependants to DisableModModal, effectively cutting
out the middleman LocalModList.
This significantly speeds up the process.
Now that the same method is no longer used to disable mods too, it can
be cleaned up quite nicely.

The whole implementation will end up being replaced with one that's
located in the Vuex store, like the disabling counterpart, but it's
beyond the scope of this PR.
Make DisableModModal more self-sustained
Use Vuex when disabling all mods from settings page
- Main benefit is that emitted errors no longer need to bubble all the
  way to the App.vue to get shown. This should prevent situations where
  refactoring components distrupts the emit chain and leaves the error
  unshown
- Refactors the modal to a single standalone component
- Fixes the odd naming where R2Error's "name" was stored in variable
  called "message", and "message" was stored in "stack" variable
- Unifies some logged messages from "name\n-> message" format to
  "[name]: message" format
- Fix the VotV game definition to target the correct top-level game binary.
- Fix invalid UE4SS-settings.ini runtime file linkage. Prior to this change this file
  was incorrectly moved into the root-most game dir, instead of alongside the other UE4SS
  and shimloader files within 'GAME/Binaries/Win64'.
"local" in the variable name refers to locally installed mods, i.e.
the mods currently included in the profile. It therefore makes sense
for the mod list to be part of the ProfileModule.
The old name was a bit confusing since it could be read as a method
that resets the active game stored in GameManager's internal state.
Set the game when user proceeds (or is automatically redirected) from
game selection screen. Update the settings whenever game is changed
properly via action. To ensure the settings are properly loaded, they
should be accessed via getter rather than directly from the state. This
is not ideal but we're constrained by the fact that actions are the
only part of Vuex that supports async operations and provides access to
both local and root state/getters/mutations/actions.

The change means that the GameManager is no longer the single source of
truth in the matter. This is a trade-off with eventually being able to
do more business logic in Vuex store instead of the UI components.
SearchAndSort now accesses only Vuex, which handles updating the local
state and the persistent settings storage.
- If profile/loadOrderingSettings is called in SearchAndSort, its
  sibling component LocalModList has already tried to access related
  settings
- Move resetting mod list when changing between profiles from Profiles
  component to Manager, since that seems to work too and feels like a
  more correct place for it
- Move modFilters/reset call from Manager's created to beforeCreate.
  The location doesn't really matter since the components that use the
  related settings aren't mounted initially, but for consistency's
  sake it's called in the same place as the rest
Move localModList from main Vuex store to ProfileModule
Use Vuex to manage error modal
Duplicate active game and settings instance in Vuex
Fix invalid VotV game definition and invalid UE4SS-settings link behavior
Commit b17d878 moved this bit of code from Profiles.vue to Manager.vue
since it makes for the latter to handle the state manipulation that
affects its own contents.

However, Vue doesn't await beforeCreate(), which opened up a race
condition where it's technically possible for the updating of the mod
list with an empty array to complete last, causing the view to render
an empty state.

While placing this bit of code in Profiles.vue is not ideal, at least
it works since the method is awaited there.
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.
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.
- 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
Allows pretty printing file sizes for human digestion.
If the zipped payload of the profile exceeds the limit, the process is
cancelled before the B64 encoding and upload steps, and an error modal
is shown to the user.

The size is limited firstly because there shouldn't be need for larger
profiles, and exceeding the limit is a symptom of something being
wrong, e.g. unintended contents in the BepInEx config folder.

Second reason is that too large profiles will crash on the B64 encoding
step on TSMM, which uses different file operation implementation than
r2mm.
One identified source for errors when exporting a profile as a code is
when the zip file is already open in another app. This doesn't affect
export as a file operations, since the zip used there is timestamped.
I assume the zip in code export is not to avoid bloating the data
directory with multiple archives of the same profile over time.
Move resetting mod list back to Profiles.vue
Limit the size of the profiles exported as a code to 20MB
anttimaki and others added 28 commits March 6, 2024 09:33
The implementation imitates what's currently being done by
LocalModList.performEnable(). Execption is that the new implementation
supports similar "progress indicator" as the disabling method does,
even though it's not currently used since mods are never enabled via
modal where the progress could be shown, even if multiple mods are
enabled at once.

This helps remove business logic and direct references to providers
from components, hopefully supporting reuse and easier refactoring.
- I'm not sure if this is the proper way to test components that rely
  on state stored in Vuex, but it's the first of many ways I tried that
  worked
- Tests need to await $nextTick() so the async created() method is done
  before the test cases continue
This cleans up the Manager component and speeds up feature since the
old implementation updated the mod list after each mod was enabled,
whereas the new one does it only once at the end.
Add MS Store version of Bomb Rush Cyberfunk
Make UninstallModModal more self-sustained
Make AssociatedModsModal more self-sustained
Make LocalModCard more self-sustained
Delegate enabling all mods to Vuex from settings view
Change the Vuex setter for active game to also update the value into
GameManager to ensure better sync. The setter now returns settings for
the newly activated game for convenience.
Originally the code first updated the list of available mods from
Thunderstore API and then loaded the local mod list from disk. In
commit dee959f yours truly (probably by mistake) changed this so that
both operations are executed independently on 5 minute intervals. this
probably leads to situations where the local mod list is loaded before
the online mod list is updated, which makes no sense, so revert to do
these operations in sequence.

(IDK why we load the mod list from the disk here in the first place but
that's outside the scope of this commit.)

Since refreshThunderstoreModList now also loads the mod list from disk,
this doesn't need to be separately handled in Manager/Settings view.

Loading the profile's mod list from disk was moved to Vuex action.
refreshThunderstoreModList could also be refactored into Vuex but since
it's used elsewhere too, it's not done within this commit.
Utilise Vuex-based methods in App.vue and related UtilityMixin
Use Vuex to access active game and settings in Profiles.vue
Updated versions and CHANGELOG
@ebkr ebkr self-assigned this Apr 6, 2024
@ebkr ebkr merged commit 21ce0b2 into master Apr 6, 2024
8 checks passed
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.

6 participants