-
Notifications
You must be signed in to change notification settings - Fork 10
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
onchange patch #33
onchange patch #33
Conversation
Removed attaching an onchange event to core setting. Instead places it in `then` promise.
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.
The existing implementation is meant to be an easily removable temporary fix until Foundry VTT fixes this issue. I guess that is sort of unlikely to happen, so I agree that doing this instead might make sense.
js/find-the-culprit.js
Outdated
@@ -354,7 +354,9 @@ async function deactivationStep(chosenModules = []) { | |||
await game.settings.set(moduleName, "modules", currSettings); | |||
} | |||
|
|||
game.settings.set("core", ModuleManagement.CONFIG_SETTING, original); | |||
game.settings.set("core", ModuleManagement.CONFIG_SETTING, original).then(r => { |
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.
This is inside an asynchronous function, so it makes more sense to await
this line and then put the reload on the next line without using then
. This is more in line with modern JavaScript best practices.
switched to await. |
CHANGELOG.md
Outdated
@@ -1,3 +1,6 @@ | |||
# v1.4.5 | |||
- Moved window reload logic to `then` instead of core settings `onchange` event. Fixes incompatibility between MM+ and Ftc |
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.
Needs to be updated
Sssshhhhhhhh.... It was always correct. lol |
Removed attaching an onchange event to core setting. Instead places it in
then
promise.This patch is designed to fix the conflict between MM+ and FtC, However, reloading after the setting is saved will remove any conflict with any other Module attaching an
onchange
event to that setting.