-
Notifications
You must be signed in to change notification settings - Fork 186
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
Use IndexedDB to store Thunderstore package list #1261
Conversation
30d4de1
to
85ea2aa
Compare
43bcbcb
to
8e0d998
Compare
85ea2aa
to
b745c05
Compare
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.
Didn't review the refactoring code, just the dexie store so far
8e0d998
to
9f67eb2
Compare
v3.2 brought in some new goodies, although I'm not sure if we can fully utilise them due to old Vue version.
This IndexedDB wrapper is used for storing the responses received when querying Thunderstore API for package listings for a specific game. The refactoring shouldn't be visible to users (except for a minor delay when storing the response) for now. In short term this allows us to drop the duplicate in-memory list of ThunderstoreMod objects. In medium term this aims to make it easier to do further refactorings to deal with the poor performance of handling the very large mod lists in communities like Lethal Company.
The process of accessing the list of mods available via Thunderstore API for a specific game: 1. Components (SplashMixin on the foreground and UtilityMixin on the background) trigger the API request, currently via ConnectionProvider 2. Components pass the response to ThunderstorePackages which writes the results to IndexedDB. ThunderstorePackages module might become obsolete in the future 3. Components trigger Vuex update where TsModsModule reads the mod list from IndexedDB and stores it in memory 4. Component access the list via Vuex To keep the commit size down ModBridge and ThunderstorePackages' internal methods still use the duplicate PACKAGES and PACKAGES_MAP objects, but those will be cleaned out in the subsequent commits.
The methods isn't really used much and this allows us to drop the dependency on the soon obsolete ModBridge.
Implement cached "ManifestV2 to ThunderstoreMod" conversion in TsModsModule in a way that's compatible with what ModBridgre did previously. This allows us to drop ModBridge, and the duplicate in-memory list of PACKAGES.
b745c05
to
788cd1d
Compare
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.
Didn't spot anything obviously wrong with this, good to thoroughly test of course.
I don't see this PR impacting it but locally importing a mod that does not exist in the Thunderstore API is probably a good idea to check still works after these refactorings.
const cacheKey = `${mod.getName()}-${mod.getVersionNumber()}`; | ||
|
||
if (state.cache.get(cacheKey) === undefined) { | ||
const tsMod = state.mods.find((m) => m.getFullName() === mod.getName()); |
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.
Should we not be able to query this from the IndexedDB now? Maybe you added that in a later PR already & I forgot
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.
Accessing IndexedDB is async and the sync Vuex getter is called by e.g. sync Vue computed properties. But I can look into it (not blocking this PR). At the very least the prewarming of cache could be done that way, although that would still require keeping the whole list in memory for other accessing code paths.
Draft PR for requesting early feedback.Cleaned up the PR, and good to go from my perspective. Note that #1266 was prepended to this PR stack.Refreshing the contents of the IndexedDB on the SplashScreen takes about 12 secs for Lethal Company. I'd say ~66% of that is throwing out the old values, so we might consider some alternative way of dealing with mod removals? Otherwise the changes didn't seem to affect performance much one way or another (except when the IndexedDB is refreshed again by the background update). EDIT: actually there's notable but not catastrophic slowdown when rendering local mod list for the first time, currently investigating this.