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

Add register settings for "early" settings #290

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Hoppelite
Copy link

This was causing this file to break when checking for developer-mode below. It seems that unregistered settings are being unset.
image

@MrPrimate
Copy link
Owner

MrPrimate commented Jan 9, 2023

These are already registered else where, it might be that a module conflict is causing them to be delayed till later, as they are on an odd hook, I will try moving them to earlier in the init hook, rather than looping additionally here.

(I don't see these errors on any of my test platforms).

@Hoppelite
Copy link
Author

These are already registered else where, it might be that a module conflict is causing them to be delayed till later, as they are on an odd hook, I will try moving them to earlier in the init hook, rather than looping additionally here.

Ahh, I see. The issue we are finding is when it tries to get the developer-mode setting in this file (https://github.com/MrPrimate/ddb-importer/pull/290/files#diff-ed4e0f64386c49a547d8ef4d7c97650d67544ccd30db2d8dcd8bdf44d4df609aR193), the setting has not been registered yet. It is possible that a newer version of Foundry finds the lack of setting registry an exceptional case, or the hook call order has changed.

It might not be suitable to rely on the hooks running in a particular order for this as it causes a race condition (in a sense).
Maybe we could move

if (game.settings.get(SETTINGS.MODULE_ID, "developer-mode")) {
  CONFIG.DDBI.DEV.enabled = true;
}

To the early settings registry function instead?
Though now I look a little deeper, I notice this setting is expected to be set by another hook. We could possibly make it a custom hook which is called when developer mode is set, one way or another. Then the other hooks that want to use the setting can just register a callback, effectively using the hook like an event.

@MrPrimate
Copy link
Owner

Does this still occur for you in v3.3.0 or v3.3.1 of the module?

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.

2 participants