League of Extraordinary FoundryVTT DevelopersMana

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.

Calego•960d 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•960d ago

My test was just set to run it at ready hook.

Calego•960d 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•960d 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•960d ago

Calego•960d ago

you could set it to re-check on a timeout but that's kind of awful

Unknown User•960d ago

Calego•960d ago

UX wise, init or ready barely matters, the user still has to go back to setup manually

Mana•960d 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•960d 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•960d 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•960d 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•960d ago

Alright.

Unknown User•960d ago

Mana•960d ago

Hmm.. possibly.

Unknown User•960d ago

Mana•960d ago

Honestly what I have so far would make adding testing all active modules fairly trivial...

Calego•960d 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•960d ago

For developing stuff, it should only be ~3 at most, unless you develop with tons of modules enabled.

Calego•960d ago

<.< i feel attacked 😛

Mana•960d 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•960d ago

👍

Mana•960d ago

Or select box.

Calego•960d ago

thanks @manaflower !

LTL

Leo The League Lion•960d ago

@calego gave LeaguePoints™ to @manaflower (#88 • 13)

Unknown User•960d ago

Mana•960d 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•960d ago

Would a button in the notification for shutting down the game be appropriate here?

Mana•960d 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•960d 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•960d 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•960d ago

https://github.com/League-of-Foundry-Developers/foundryvtt-devMode/pull/21

GitHub

JSON change notifications by mkahvi · Pull Request #21 · League-of-...

Adds two options to be notified of either system JSON (system.json & template.json) changes or module JSON (module.json) changes. The introduced code should be fairly easy to extend or fix ...

Mana•960d 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•960d ago

Awesome. Thanks @manaflower I'll give that a review soon ™️

LTL

Leo The League Lion•960d ago

@calego gave LeaguePoints™ to @manaflower (#83 • 14)

Calego•954d 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•954d ago

Committed the changes. I have no idea how these things work. Click the re-request review? I have no clue.

Calego•954d ago

i gotchu rerequesting review is a good idea yeah merged and released!

Calego•954d ago