mouse0270 / module-credits

Lists the authors of projects on the Manage Modules Window. If a url is provided in the module.json file, it will make the version tag link to the module url.
MIT License
7 stars 6 forks source link

Module dependencies not recognized on enabling/disabling modules? #107

Closed coffiarts closed 1 year ago

coffiarts commented 1 year ago

Is it only me? After enabling MM+ in Foundry 11, none of the module dependencies (be it required or recommended) seem to be recognized on enabling or disabling modules.

To be more precise:

Disabling MM+ brings back all those features at once. I have no errors at all in my console apparently related to MM+

mouse0270 commented 1 year ago

Are you talking about this Dialog? image

coffiarts commented 1 year ago

Yes, exactly. Meanwhile I've discovered that it seems to work at least for some modules, but I simply can't get what makes the difference.

For example...

But it apparently DOES work here: https://github.com/otigon/automated-jb2a-animations/blob/main/module.json

Or here (just the snippet instead of a link):

"relationships": { "requires": [ { "id": "dsa5-core", "type": "module", "compatibility": { "minimum": "5.0.0" } } ], "systems": [ { "id": "dsa5", "type": "system", "compatibility": { "minimum": "5.0.0" } } ] },

I can't understand what makes those module manifests behave differently in MM+

mouse0270 commented 1 year ago

Can you do me a favor, can you disable everything but MM+. Then reload and just click on Hot Pan & Zoom! I think this is an issue with recommends.

For example, if I disable Always Centred and Socketlib and then try enabling Hot Pan, I do get the dialog, but if Socketlib is already enabled but Always Centred is not, then I do not get the dialog.

So its a bug, but I want to make sure we have the same bug.

coffiarts commented 1 year ago

Thanks for your instant support! Really appreciate that :) And yes, it's a good point to disable everything first (might have thought of it myself). Find below my exact steps. I assume that they confirm your idea, but judge by yourself:

1.) Disable everything image

2.) Reload, then activate Hot Pan & Zoom only The prompt appears correctly, stating all the dependencies (including recommended): image

3.) Reload, then activate MM+, confirming all the suggestions: image image image

4.) Reload, then checking Hot Pan & Zoom details Only socketlib dependency is listed: image

5.) Activate Hot Pan & Zoom This time nothing happens! image

6.) DEactivate Hot Pan & Zoom Weirdly, now at least the prompt for socketlib is showing up! But still not for the recommended one. image

Hope this helps. Really glad that you're looking into this. AFAIK, the "recommended" flag has been recently introduced in Foundry 11.

mouse0270 commented 1 year ago

Yeah, they choose recommends over optional, its a dispute between me and the devs. I disagree that recommends and optional are the same thing.

It does appear that my issue is what I think it is. So I will work on patching it. I already had a patch I forgot to update a week ago so maybe I can get it out today/tomorrow

coffiarts commented 1 year ago

Just a naive idea - given that it helps: Could we (aaaaaalll the mod authors out there) simply switch back to "optional" - at least as a workaround? Or is "optional" completely unsupported in v11?

coffiarts commented 1 year ago

And with your input I am seeing clearer... guess it's lines like this one that have to be changed, right? https://github.com/mouse0270/module-credits/blob/74600d50a49d733fa87d2bd75058d209c6a75337/scripts/init.mjs#L108 I hope it's as simple as just replacing "optional" by "recommended"? But I'll leave you to it, so that you can sort it out in peace and without any haste. Just curious ;-)

mouse0270 commented 1 year ago

So basically, MM+ supports optional, and its supposed to support recommends, but I clearly missed something. haha.

My idea is this Recommends Are modules that the author wants you to run alongside and that you should download and enable. Optional Are modules that the author supports but wouldn't really recommend downloading and installing.

For example MM+ recommends that you install and use lib-wrapper and socketlib because these two modules improve the functionality of MM+. However, it has an optional module of libThemer which just adds support but I wouldn't want someone to download and install libThemer just because they installed MM+

coffiarts commented 1 year ago

Yeah, sounds like a plausible distinction. Though I guess that 99% of the dependencies out there will match the "recommended" definition, while"optional" would be reserved for a very few things that authors just have to support to make some people happy, though it won't add much value to their mods (never would have thought of that strict "optional" case myself). In such a 1-%-optional scenario, the distinction might cause some confusion, as many people would mix up "optional" with "recommended", because they just don't care for the difference.

mouse0270 commented 1 year ago

Honestly, its mostly the fact that I dislike the term "recommends" As it implies that I want you to install and run this module too. Where optional does not and simply implies that my module supports it.

So my supporting of recommends and optional is mostly because I disagree with the devs reasoning for "recommends" instead of optional.

mouse0270 commented 1 year ago

Should be fixed in version 2.2.1.2 https://github.com/mouse0270/module-credits/releases/tag/2.2.1.2

If not let me know.

coffiarts commented 1 year ago

Works absolutely fine now - everything as expected! Thanks for fixing that so rapidly.

Just a side note - it seems to me that the previous version was wrongly labelled as v2.2.2.1 instead of v2.2.1.1, that's why I wasn't able to run an auto-update in Foundry's setup.

To install the latest version, I had to do a complete uninstall and reinstall.

Before update: image

After uninstall and fresh install: image

Just pointing that out because you should be aware that users who simply click the "update" or "update all" will probably never be aware of the update (unless you change the new version to something higher than 2.2.2.1).

mouse0270 commented 1 year ago

Darn I was hoping fixing that on the admin portal would work... I might have to push a new update that is version 2.2.3 or something. Thanks for pointing this out to me.

coffiarts commented 1 year ago

Well, it was worth a try. I wouldn't be able myself to tell in advance how that "version syncing" really works in Foundry. So it seems (from this example) that mod versions are locally stored and compared to the latest versions in the admin portal.