ruipin / fvtt-lib-wrapper

Library for Foundry VTT which provides module developers with a simple way to modify core Foundry VTT code, while reducing the likelihood of conflict with other modules.
GNU Lesser General Public License v3.0
35 stars 15 forks source link

Recommended way to use libWrapper with systems #40

Closed mjeffw closed 3 years ago

mjeffw commented 3 years ago

What is the recommended way to incorporate libWrapper into a system? I'd like to use it, and also have it available for any modules that also have a dependency on libWrapper.

I've seen that installing some modules, Foundry will prompt the user to install any dependent modules. Is this automation, or must be supported by code or configuration in the system? How to do this?

ruipin commented 3 years ago

Dependencies is unfortunately something that Foundry doesn't really do very well. You can set libWrapper as a dependency of your system, and Foundry will try to ensure it gets installed as well. However, there is no hard guarantee it will be enabled correctly, so you still need to check for it manually and have some sort of fallback.

My suggestions would be:

  1. Don't use libWrapper on a system when you can avoid it

    It might be worth trying to avoid libWrapper at all. At least as of 0.8.x, systems should always load before modules (except "library": true modules). As long as you register your wrappers/overrides as soon as possible (e.g. immediately on system execution, on the init hook, etc), you will be the first to have a go. Be careful about using async etc before patching everything you need to patch, as this could mean modules get to run before your patches.

    Assuming this is done, from the point of view of the modules loading afterward, those wrappers/overrides are the Foundry code, not even libWrapper would know they have been touched. This is what basically all systems do right now, and it works relatively well, mostly because systems always come first and there's only one system active at a time.

  2. If you can't avoid using libWrapper, make it a soft dependency

    If for whatever reason you need to dynamically patch methods, or need to be able to have modules go after the system for some reason, then the best way to proceed is probably to use the shim and recommend libWrapper, rather than require it.

    The only real use-cases I can think of is when you need to have a wrapper on specific methods, and want it to run even when modules override said method using libWrapper. Alternatively, when you need to wrap things during gameplay, and not just when your system loads. In such cases, libWrapper would be the best solution indeed.

    I would also probably only use libWrapper when the libWrapper functionality is actually relevant for the patch you are doing. I.e., I wouldn't use it when overriding Foundry methods, only for "WRAPPER" or "MIXED" wrappers, where modules might need to come after you.

    Anyway, keeping it to a soft dependency means that libWrapper can be disabled when triaging issues, and if libWrapper breaks for whatever reason (and I haven't gotten around to fixing it yet) you can still run your system fine.

  3. If you make it a hard dependency, explicitly fail if libWrapper doesn't exist

    If you want a hard dependency, you'll need to check for libWrapper being active and fail otherwise. This is because Foundry's dependency system isn't perfect, and there are plenty of cases where it might allow an user to load into a world regardless of the dependency being installed/active.

Let me know if you have any further questions. I'll see if I can summarize some of this and add it to the documentation somewhere, since it seems important enough.

mjeffw commented 3 years ago

| Don't use libWrapper on a system when you can avoid it

Confessing: I'm a developer, but my languages of choice don't typically include Javascript. To patch a Foundry class, how would you go about it?

In my usecase I need to patch TokenHUD#getData to modify the list of statusEffects. Is it possible to register a subclass of TokenHUD with the override? or "monkey patch" the getData method? Also where is the best place to do this -- In an "init" Hook? or a "ready" Hook?

Any guidance you have is hugely appreciated!

ruipin commented 3 years ago

You can do either. Often if just patching a single method, you'd monkey patch the existing class. If completely rewriting a lot of the behaviour, you'd probably want to replace the class with your own version.

Anyway, to monkey-patch a method without libWrapper, you'd usually do something like:

Hooks.once('init', () => {
  const original_TokenHud_getData = TokenHUD.prototype.getData;
  TokenHUD.prototype.getData = function(...args) {
     // do stuff
     let result = original_TokenHud_getData.apply(this, args);
     // do stuff
     return result;
  }
});

The FoundryVTT Discord #module-development channel and the League of Extraordinary Foundry VTT Developers Discord are the best places to ask this kind of more generic questions, if you have more or need more detail.

ruipin commented 3 years ago

I have now added a section about systems to the documentation, see SYSTEMS.md.

I will now close this issue.

mjeffw commented 3 years ago

Thanks, you gave me good information that led to a successful new feature!