devMode json changed warning

I checked the module repo and I honestly can't figure out where I would plug in my code. I'd be guessing I'd be adding entirely new file like how the anchor and disable template cache do, but the existing folders make no sense for what this would be about.
41 Replies
Calego
Calego3y ago
How often are you running this fetch? your code, for reference:
fetch(`systems/${game.system.id}/template.json`, { method: 'GET', cache: 'no-cache', redirect: 'manual'})
.then(function (response) {
if (response.ok) response.json().then(data => {
const diff = diffObject(game.system.template, data);
if (!isObjectEmpty(diff)) {
ui.notifications?.warn("template.json has changed", {permanent:true});
console.warn("template.json has changed:", diff);
}
})
});
fetch(`systems/${game.system.id}/template.json`, { method: 'GET', cache: 'no-cache', redirect: 'manual'})
.then(function (response) {
if (response.ok) response.json().then(data => {
const diff = diffObject(game.system.template, data);
if (!isObjectEmpty(diff)) {
ui.notifications?.warn("template.json has changed", {permanent:true});
console.warn("template.json has changed:", diff);
}
})
});
This is something which would be independant, part of one of the classes, potentially a new class altogether to fully contain this functionality. It'll need this fetch as a method, and some trigger, as well as a way to disable/enable the functionality via a setting.
Mana
Mana3y ago
My test was just set to run it at ready hook.
Calego
Calego3y ago
If it can run on a hook, a new files in hooks is good you're thinking "on reload, check if the json is different and you should go back to setup?" I like that
Mana
Mana3y ago
Yeah. I don't think it makes much sense otherwise since I don't think you can reload JS without doing evil sorcery.
Unknown User
Unknown User3y ago
Message Not Public
Sign In & Join Server To View
Calego
Calego3y ago
you could set it to re-check on a timeout but that's kind of awful
Unknown User
Unknown User3y ago
Message Not Public
Sign In & Join Server To View
Calego
Calego3y ago
UX wise, init or ready barely matters, the user still has to go back to setup manually
Mana
Mana3y ago
I like ready myself to avoid it from interfering with the initial load too much. Same reason I've pushed my template preloading there in all my other projects.
Calego
Calego3y ago
👍 Here's all you should need to get this set up in dev mode: A new file in hooks which looks like the dev-mode-anchor one, but will end up a lot simpler/smaller. A new setting to enable this And a tweak to foundryvtt-devMode.mjs to tie it all together
Mana
Mana3y ago
Problem tho. This works flawlessly when developing a system, but if you're module developer. How would I determine which module.json to test? It looks like devMode doesn't allow determining which itself. Only decent option I can think of is not to actually do it in ready hook and just expose two functions, one for doing it for a module (module.json), the other for system (system.json & template.json). Or introduce new setting where you just type down module ID
Calego
Calego3y ago
IMO it's fine if iteration 1 of this is only geared towards systems Setting would be "warn me if my system json changes" (default off)
Mana
Mana3y ago
Alright.
Unknown User
Unknown User3y ago
Message Not Public
Sign In & Join Server To View
Mana
Mana3y ago
Hmm.. possibly.
Unknown User
Unknown User3y ago
Message Not Public
Sign In & Join Server To View
Mana
Mana3y ago
Honestly what I have so far would make adding testing all active modules fairly trivial...
Calego
Calego3y ago
Feels like a lot of fetches to me but sure yeah I guess it won't hurt anything, esp since it happens asynchronously in the hook
Mana
Mana3y ago
For developing stuff, it should only be ~3 at most, unless you develop with tons of modules enabled.
Calego
Calego3y ago
<.< i feel attacked 😛
Mana
Mana3y ago
Also, the JSON files are tiny, and likely you're doing this locally anyway (not guaranteed). I'll add two booleans, one for testing system, the other for testing modules.
Calego
Calego3y ago
👍
Mana
Mana3y ago
Or select box.
Calego
Calego3y ago
thanks @manaflower !
Leo The League Lion
@calego gave vote LeaguePoints™ to @manaflower (#88 • 13)
Unknown User
Unknown User3y ago
Message Not Public
Sign In & Join Server To View
Mana
Mana3y ago
Well, there's a bump in the road for non template.json checking. Foundry apparently reformats module.json description, so if you happen to have say <br/> in there, it will always trigger as if it had been modified. Same applies to system.json I imagine. I'll just do the template.json comparison and leave the extended code in with some comments about why it's not enabled.
Daniel Thorp
Daniel Thorp3y ago
Would a button in the notification for shutting down the game be appropriate here?
Mana
Mana3y ago
It's not going to pop up a dialog, just normal notification. Dialog would be far too intrusive as it's not something you HAVE to restart for (depending what you're doing). Except for Foundry reformatting the description, this is so far working for all other data. I guess I could just delete description comparison...
Calego
Calego3y ago
There's probably a method out there that does this sanitation, maybe you can find that and run it before diffing? Part of moduleData document maybe
Mana
Mana3y ago
Considering the problem seems to only occur with description, I just deleted it from diffing. I don't think anyone particularly cares for the description of their module/system changing.
Mana
Mana3y ago
GitHub
JSON change notifications by mkahvi · Pull Request #21 · League-of-...
Adds two options to be notified of either system JSON (system.json &amp; template.json) changes or module JSON (module.json) changes. The introduced code should be fairly easy to extend or fix ...
Mana
Mana3y ago
Not entirely sure about the import in the main .mjs file, but it should be fine. I guess the fetchData should've been named differently, too. Ugh. Loaded some 50 modules for testing that, so.. it should be fairly functional.
Calego
Calego3y ago
Awesome. Thanks @manaflower I'll give that a review soon ™️
Leo The League Lion
@calego gave vote LeaguePoints™ to @manaflower (#83 • 14)
Calego
Calego3y ago
@manaflower I uh.. completely forgot about this. But I just pushed a review your way. A few small tweaks and this is golden.
Mana
Mana3y ago
Committed the changes. I have no idea how these things work. Click the re-request review? I have no clue.
Calego
Calego3y ago
i gotchu rerequesting review is a good idea yeah merged and released!
Calego
Calego3y ago
used new GH auto changelog in the release too, very shiny
No description
Mana
Mana3y ago
Neat.
Calego
Calego3y ago
somehow the workflow for release failed... wondering if this is a github bug, gonna investigate later