Improve performance of Manager.setAllModsEnabled #1191
Conversation
anttimaki
commented
Jan 30, 2024
anttimaki
commented
- Skip processing the mods that already are in the desired state
- Update VueX store only once at the end, not after each mod, since this will trigger a costly rerendering (which will be addressed separately
There was a problem hiding this comment.
Should probably wrap the logic to a try/catch to avoid state drift on a thrown error? Even though a lot of the code uses the paradigm where errors are returned instead of thrown, nothing actually guarantees this to be the case throughout all the code paths (this is also the main reason why I don't like this paradigm in JS; you need to be prepared to handle exceptions anyway and I'm not aware of any linters that could guarantee all potential exceptions are getting handled.)
anttimaki
force-pushed
the
enable-all-mods-performance
branch
from
bcbde74 to
56defd2
Compare
src/pages/Manager.vue
Outdated
| const name = `Error ${enabled ? "enabling" : "disabling"} mods`; | ||
| const message = (e as Error).message; | ||
| this.showError(new R2Error(name, message)); |
There was a problem hiding this comment.
There's a R2Error.fromThrownValue class method (or something like that) available which you can use for this kind of stuff btw
There was a problem hiding this comment.
Changed to use fromThrownValue.
- Skip processing the mods that already are in the desired state - Update VueX store only once at the end, not after each mod, since this will trigger a costly rerendering (which will be addressed separately - Wrap the logic in try-catch to avoid state drift on unexpected errors
anttimaki
force-pushed
the
enable-all-mods-performance
branch
from
56defd2 to
9a18907
Compare
