League of Extraordinary FoundryVTT Developers•4y ago

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•4y 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.

ManaOP•4y ago

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

Calego•4y 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

ManaOP•4y 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•4y ago

Message Not Public

Calego•4y ago

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

Unknown User•4y ago

Message Not Public

Calego•4y ago

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

ManaOP•4y 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•4y 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

ManaOP•4y 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•4y 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)

ManaOP•4y ago

Alright.

Unknown User•4y ago

Message Not Public

ManaOP•4y ago

Hmm.. possibly.

Unknown User•4y ago

Message Not Public

ManaOP•4y ago

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

Calego•4y 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

ManaOP•4y ago

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

Calego•4y ago

<.< i feel attacked 😛

ManaOP•4y 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•4y ago

👍

ManaOP•4y ago

Or select box.

Calego•4y ago

thanks @manaflower !

Leo The League Lion•4y ago

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

Unknown User•4y ago

Message Not Public

ManaOP•4y 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•4y ago

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

ManaOP•4y 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•4y 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

ManaOP•4y 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.

ManaOP•4y 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 ...

ManaOP•4y 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•4y ago

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

Leo The League Lion•4y ago

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

Calego•4y ago

@manaflower I uh.. completely forgot about this. But I just pushed a review your way. A few small tweaks and this is golden.

ManaOP•4y ago

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

Calego•4y ago

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

Calego•4y ago

used new GH auto changelog in the release too, very shiny

ManaOP•4y ago

Neat.

Calego•4y ago

somehow the workflow for release failed... wondering if this is a github bug, gonna investigate later

Gaming

Programming

devMode json changed warning