ruipin / fvtt-lib-wrapper Goto Github PK

Steps to reproduce:

In foundry settings - Permission configuration page give Player or Trusted player group the Modify configuration settings permission.
Login as (trusted) player.
Open libWrapper settings.

Observed:

User can't modify libWrapper settings.Priorities and Conflicts tabs are empty. "Statistics collection is disabled" message is displayed.

Expected:

User, who is given access to libWrapper settings, should be able to change them.

While negligible in most cases, libWrapper will add some overhead to wrapper calls compared to traditional wrappers.

In some cases, e.g. testWall, methods get called thousands of times in a loop, meaning that said performance hit actually becomes noticeable.

For these cases, it could make sense to add a wrapper type which does static dispatch. This type would avoid having to set up the libWrapper context, and forgo most of the libWrapper safeties, keeping only basic conflict checking and module prioritisation. Everything necessary for this wrapper should be resolved at wrapping time, similar to how the shim does it.

The module should be updated to use Manifest+

https://foundryvtt.wiki/en/development/manifest-plus

This includes having "library": true, to get libWrapper to load before non-library code in 0.8.x.

Due to Foundry swallowing exceptions that occur inside hooks, libWrapper hooks Hooks._call to catch any unhandled exceptions.

However, this leads to very ugly call stacks any time a hook is triggered, which leads some people to mistakenly believe that an issue is caused by libWrapper, when libWrapper was not involved.

Ideally, we should improve this so that the hooks callstack isn't polluted by libWrapper. Multiple possibilities exist, e.g.:

1. Do not decorate the Hooks._call wrapper.
2. Override the Hooks._call wrapper, instead of wrapping it, reducing the number of libWrapper entries in the callstack, and avoid rethrowing.
3. Move this hook to a different file not called errors.js, e.g. hooks-listener.js.

Right now, libWrapper only has global toggles to disable all notifications related to detected conflicts.

Sometimes, conflicts are expected and known by a module dev or the user, and they do not want to see the notifications for them. However, disabling the notifications runs the risk of the user not being told of new conflicts after e.g. updating various modules.

• The backend needs to be updated to support ignoring specific conflicts.

• There should still be a toggle in the "Conflicts" pane to show all conflicts detected, even those that were ignored. This would allow to debug issues where ignores are too broad.

• There should be an API that allows module devs to choose conflicts that the user should not be notified of.

• There should be a new config pane that allows the user to choose specific notifications they do not want to see again.

Hi, I get the following errors after logging in FoundryVTT in my world:

FoundryVTT: 0.7.10
DnD5e: 1.
Libwrapper: 1.6.0

VM755:8 ReferenceError: ace is not defined
    at editor.js:5
    at Array.forEach (<anonymous>)
    at editor.js:5
    at Function._call (eval at <anonymous> (libWrapper-errors.js:249), <anonymous>:4:14)
    at Function.callAll (foundry.js:2456)
    at Game.initialize (foundry.js:6457)
    at z.🎁call_wrapped [as call_wrapped] (libWrapper-wrapper.js:448)
    at Game.🎁libWrapperInit (libWrapper-api.js:521)
    at Game.🎁Game.prototype.initialize#0 (libWrapper-wrapper.js:142)
    at window.addEventListener.once (foundry.js:7449)
_call @ VM755:8
quick-insert.ts:68 Uncaught (in promise) ReferenceError: KeybindLib is not defined
    at quick-insert.ts:68
    at Function._call (eval at <anonymous> (libWrapper-errors.js:249), <anonymous>:4:14)
    at Function.callAll (foundry.js:2456)
    at Game.initialize (foundry.js:6457)
    at z.🎁call_wrapped [as call_wrapped] (libWrapper-wrapper.js:448)
    at Game.🎁libWrapperInit (libWrapper-api.js:521)
    at Game.🎁Game.prototype.initialize#0 (libWrapper-wrapper.js:142)
    at window.addEventListener.once (foundry.js:7449)

Active module list:
about-time--v0.8.4;
ActiveAuras--v0.2.11;
animated-spell-effects--v0.6.5;
blood-n-guts--v0.8.7;
calendar-weather--v3.1.2;
coloredeffects--v0.0.7;
combat-utility-belt--v1.4.0;
CommunityLighting--v0.3.3;
compendium-folders--v2.2.44;
conditional-visibility--v0.1.4;
condition-automation--v3.0.0;
blind-roll-skills--v0.7.0;
custom-hotbar--v2.1.0;
custom-nameplates--v1.0.2;
Custom-Token-Animations--v0.1.20;
darkraven-games-soundscapes-free--v1.0.1;
deprecated-modules--v1.1.3;
dice-so-nice--v3.3.4;
dice-calculator--v0.6.8;
dnd5e-helpers--v2.0.3;
drag-ruler--v1.7.2;
Dynamic-Effects-SRD--v4.1.13;
dae--v0.2.62;
dynamic-illumination--v0.9.0;
EasyTable--v1.3.2;
easy-target--v2.11;
forien-copy-environment--v1.1.1;
forien-scene-navigator--v0.1.6;
foundry_community_macros--v0.40;
foundry_community_tables--v0.13;
free-loot-tavern-compendium--v1.9.1;
fxmaster--v1.0.3;
gm-notes--v0.4;
gm-screen--v2.4.3;
healthEstimate--v2.4.0;
launchmacro--v1.3.3;
illandril-chat-enhancements--v1.1.5;
illandril-hotbar-uses--v2.3.3;
illandril-token-hud-scale--v1.3.3;
image-hover--v1.1.7;
itemacro--vv1.5.1;
jb2a_patreon--v0.2.0;
colorsettings--v2.5.9;
_chatcommands--v1.3.3;
lib-df-hotkeys--v2.3.4;
lib-wrapper--v1.6.1.0;
LockView--v1.4.10;
long-rest-hd-healing--v2.2.0;
macroeditor--v1.0.7;
maestro--v0.7.4;
magicitems--v2.0.8;
_mathjs--v7.5.1-fvtt2;
max-crit--v0.1.7;
michaelghelfi--v1.2;
midi-qol--v0.3.103;
monsterblock--v2.4.7;
multilevel-tokens--v1.4.2;
Next-Up--v0.0.59;
perfect-vision--v1.9.4;
permission_viewer--v0.8.5;
pings--v1.2.11;
quick-insert--v2.4.7;
raise-my-hand--v1.1.0;
scene-transitions--v0.1.3;
settings-extender--v1.1.5;
SharedVision--v1.0.4;
alt5e--v1.5.2;
socketlib--v1.0.6;
_sortablejs--v1.10.2-fvtt1;
SoundBoard--v1.5.1;
tension-pool--v0.0.30;
Tetra-Cube-Importer--v1.1.0;
furnace--vv2.6.0;
tidy-ui_game-settings--v0.1.23;
times-up--v0.1.5;
token-action-hud--v1.0.22;
token-attacher--v4.0.2;
tokenmagic--v0.5.0;
rolladvantage-token-stamp-2-foundry--v1.1.2;
token-vision-animation-world--v1.0.0;
torch--v1.1.4;
trigger-happy--v0.8.2;
turnAlert--v1.5.1;
VoiceActor--v0.0.9;

The module "Better Rolls" has an interesting use case. It is, by default, a full override. As such, it uses "OVERRIDE". However, if the caller passes a specific parameter, it wishes to call the original method.

Because OVERRIDE by default doesn't provide the wrapped chain object, they are forced to use private APIs to access the _wrapped method instead. In addition to being unsupported, this will soon become impossible due to the work for #16.

In the original API design, OVERRIDE didn't provide the wrapped chain object on purpose. This was to avoid people using OVERRIDE just because they wanted to be the last module called, even when they weren't truly overriding a method. However, I failed to realise that there is a very rare use case where such a module might wish to call the original method.

An optional solution that does not compromise the API's design goals and maintains backward compatibility is to take an optional chain parameter that, if true, provides the wrapped chain object regardless of the wrapper type:

libWrapper.register("module-a", "Foo.prototype.bar", "OVERRIDE", {chain: true});

Optionally, the third parameter could be replaced by the dictionary (only when not a string, for backwards compatibility), e.g.:

libWrapper.register("module-a", "Foo.prototype.bar", {type: "OVERRIDE", chain: true});

In the shim code, functions are called like this:

obj[fn_name] = function() { return fn.call(this, original, ...arguments); };

In the module code they're called like this:

result = fn.call(obj, next_fn, ...args);

except if they're OVERRIDE type, in which case they're called like this:

return fn.apply(obj, args);

This breaks the behavior of shimmed override functions (e.g. in my module, ZoomPanOptions) when the module exists. The next_fn argument is not passed.

Issue

Currently, libWrapper explicitly prevents modules from chaining more than once:

libWrapper.register('my-fvtt-module', 'SightLayer.prototype.updateToken', function (wrapped, ...args) {
    wrapped(...args); // Works
    wrapped(...args); // Throws 'libWrapper: This wrapper function has already been called, and must not be called twice.'
}, 'MIXED');

This was implemented to avoid the extra complexities that come with managing the call chain as well as some libWrapper internal limitations (which have since been resolved, and thus are no longer the limiting factor).

Possible solutions

Option 1

It would be very simple to implement a situation where modules higher in the call chain do not see the multiple chaining, i.e. if we have 3 modules A, B, and C wrapping the same method in this order, and B chains twice:

A -> B -> C1 -> original
       -> C2 -> original

The biggest advantage of this option is that it is simply emulating the way that pure Javascript wrappers already work. This means there is no surprise for the user of the library, and things "just work".

This scales well enough if two modules chain twice. For example, if both A and B were to chain twice:

A -> B1 -> C11 -> original
        -> C12 -> original
  -> B2 -> C21 -> original
        -> C22 -> original

However, this could easily break module A or C. A might need to see every call to the original method, while C might not expect to see the same call twice. As such, it could end up causing some difficult-to-debug compatibility issues.

Option 2

It could be argued that the following would be more correct behaviour:

A1 -> B -> C1 -> original
        -> A2 -> C2 -> original

This would ensure A sees all calls to the original method, including those caused by multiple chaining.
However, I suspect this actually makes it easier to break module A, as it breaches the principle of least surprise due to going against the way pure Javascript wrappers work.

This option also gets more complex if two modules chain twice. For example, if both A and B were to chain twice:

A1 -> B1 -> C111 -> original
         -> A2 -> C121 -> original
               -> C122 -> original
   -> B2 -> C211 -> original
         -> A3 -> C221 -> original
               -> C222 -> original

Option 3

Another possibility is to create a new type of override (e.g. MULTICHAIN) which must be unique (similar to OVERRIDE), and always comes between MIXED and OVERRIDE, i.e. it is always the penultimate call in the chain.

This override type would be allowed to chain the call twice, since there will no longer be modules between it and the final call in the chain, as such removing the need to manage the correct call chain.

The only significant disadvantage of this option is that, due to the uniqueness requirement, it prevents two modules from chaining the same wrapper more than once.

Conclusion

After going through these options, I am tempted to go with Option 1 since it is the most simplistic, and works most similar to pure Javascript wrappers. Option 3 is also tempting, but I think the strict limitation of only one module chaining twice is hard to justify purely on compatibility grounds.

Confirm you have read the above
Appears to be a problem with how libWrapper is handling the lib wrapper.ignore_conflicts method. I am using that method to ignore a potential conflict, but I am still getting a warning message regarding that very conflict.

Describe the bug
The libRuler module overrides Ruler.prototype.moveToken with this call to libWrapper:
libWrapper.register("libruler", "Ruler.prototype.moveToken", libRulerMoveToken, "OVERRIDE");

This is an override because libRuler basically breaks up the moveToken function into parts that are easier for modules to wrap or override.

I am working on code for the Drag Ruler module that wraps this function:
libWrapper.register("drag-ruler", "Ruler.prototype.moveToken", dragRulerMoveToken, "MIXED");

It is a MIXED wrap because Drag Ruler intercepts certain key commands, like spacebar, and does not call moveToken in that circumstance. Otherwise, it will call the wrap function for moveToken.

I am getting an occasional warning from libWrapper about the potential conflict because "drag-ruler did not chain the wrapper." So I put this ignore code in Drag Ruler:
libWrapper.ignore_conflicts("drag-ruler", "libruler", "Ruler.prototype.moveToken");

After doing so, the console reports the following:

libWrapper: Wrapped 'Ruler.prototype.moveToken'. libWrapper-wrapper.js:347:2
libWrapper: Registered a wrapper for 'Ruler.prototype.moveToken' by module 'libruler' with type OVERRIDE. libWrapper-api.js:516:3
libWrapper: Registered a wrapper for 'Ruler.prototype.moveToken' by module 'drag-ruler' with type MIXED. libWrapper-api.js:516:3
libWrapper: Ignoring conflicts involving module 'drag-ruler' and [module 'libruler'] for targets [Ruler.prototype.moveToken]. libWrapper-api.js:631:3

But the console still reports the following warning (which also shows up as a notification in the UI):

libWrapper: Potential conflict detected between module 'drag-ruler' and module 'libruler'.
Module 'drag-ruler' did not chain the wrapper for 'Ruler.prototype.moveToken'. libWrapper-notifications.js:67:10

To Reproduce
I can reproduce this in my system by logging in and then dragging the token and trying to create a waypoint. It would be difficult to reproduce from libWrapper alone, because it is dependent on work-in-progress Drag Ruler code. But if it helps, I could show you where that code is in my git repository. Or happy to help test solutions.

Expected behavior
I would expect that the conflict warning no longer appear once libWrapper.ignore_conflicts is called.

Technical Details (please complete the following information):

• LibWrapper Version 1.9.0
• FoundryVTT Version 0.8.8
• Browser & Version Firefox 91.0.1

Receiving this error whenever I try to import an item. Screen shot for ease.

VM6936:7 Foundry VTT | Error thrown in hooked function
_call @ VM6936:7
callAll @ foundry.js:153
callback @ foundry.js:8866
(anonymous) @ foundry.js:8805
_handleUpdateDocuments @ foundry.js:8805
_updateDocuments @ foundry.js:8693
async function (async)
_updateDocuments @ foundry.js:8684
update @ backend.mjs:154
async function (async)
update @ backend.mjs:151
updateDocuments @ document.mjs:365
🎁call_wrapped @ libWrapper-wrapper.js:448
updateDocuments @ ItemContainer.js:77
🎁call_wrapper @ libWrapper-wrapper.js:557
🎁CONFIG.Item.documentClass.updateDocuments#0 @ libWrapper-wrapper.js:144
update @ document.mjs:447
🎁call_wrapped @ libWrapper-wrapper.js:448
_update @ ItemContainer.js:150
🎁call_wrapper @ libWrapper-wrapper.js:557
🎁CONFIG.Item.documentClass.prototype.update#0 @ libWrapper-wrapper.js:144
importFromJSON @ foundry.js:9646
(anonymous) @ foundry.js:9671
Promise.then (async)
callback @ foundry.js:9671
submit @ foundry.js:20351
_onClickButton @ foundry.js:20314
dispatch @ jquery.min.js:2
v.handle @ jquery.min.js:2
VM6936:8 TypeError: Cannot read property 'id' of null
at delayedSort (init.js:114)
at init.js:150
at Function._call (eval at (libWrapper-errors.js:249), :4:14)
at Function.callAll (foundry.js:153)
at ClientDatabaseBackend.callback (foundry.js:8866)
at foundry.js:8805
at Array.map ()
at ClientDatabaseBackend._handleUpdateDocuments (foundry.js:8805)
at ClientDatabaseBackend._updateDocuments (foundry.js:8693)
at async Function.updateDocuments (document.mjs:365)
_call @ VM6936:8
callAll @ foundry.js:153
callback @ foundry.js:8866
(anonymous) @ foundry.js:8805
_handleUpdateDocuments @ foundry.js:8805
_updateDocuments @ foundry.js:8693
async function (async)
_updateDocuments @ foundry.js:8684
update @ backend.mjs:154
async function (async)
update @ backend.mjs:151
updateDocuments @ document.mjs:365
🎁call_wrapped @ libWrapper-wrapper.js:448
updateDocuments @ ItemContainer.js:77
🎁call_wrapper @ libWrapper-wrapper.js:557
🎁CONFIG.Item.documentClass.updateDocuments#0 @ libWrapper-wrapper.js:144
update @ document.mjs:447
🎁call_wrapped @ libWrapper-wrapper.js:448
_update @ ItemContainer.js:150
🎁call_wrapper @ libWrapper-wrapper.js:557
🎁CONFIG.Item.documentClass.prototype.update#0 @ libWrapper-wrapper.js:144
importFromJSON @ foundry.js:9646
(anonymous) @ foundry.js:9671
Promise.then (async)
callback @ foundry.js:9671
submit @ foundry.js:20351
_onClickButton @ foundry.js:20314
dispatch @ jquery.min.js:2
v.handle @ jquery.min.js:2
foundry.js:9735 The Document#entity property has been deprecated in favor of Document#documentName. Support will be removed in 0.9.0

Currently, world scripts and macros are unable to use libWrapper as they do not have a module ID.

It might be a good idea to come up with a special ID that can be used for those cases. For example, "SCRIPTS" or 0.

In addition, systems can't use libWrapper either. While they usually won't need it, since they run first, might make sense to add support in case someone wants to patch things after init.

Hi,

I'm the author of Scene Packer and currently include your shim but not a hard dependency.

I've been asked whether it's possible to remove the popup dialog that's bundled in the shim. I know it's possible, but I want to know what the differences currently are between the full module and the shim version. I can't see an issue or wiki entry that lists the differences and only one issue from several months ago (#3) that discusses differences.

My current thought is to use the shim that you provide, but remove the popup dialog, therefore using the full module if it's enabled but not prompting people if it isn't. What functionality would be lost with this approach though?

trying to draw something or create a wall fails with no error message while alternative rotation and the latest version of lib-wrapper are on. tested with only these two modules on while on a new campaign

How do I wrap a game system specific class's method. I would like to wrap ActorSheet5e _prepareArmorClassAttribution.
I tried going through CONFIG.Actor.sheetclasses;

CONFIG.Actor.sheetClasses.character["dnd5e.ActorSheet5eCharacter"].cls.prototype._prepareArmorClassAttribution

which points to the correct method, but libwrapper rejects the string

A lot of people seem to confuse libWrapper errors caused by modules (where libWrapper gives a message like libWrapper: Error detected in module 'some-module-id') as an error caused by libWrapper.

This results in a considerable amount of unrelated complaints or invalid bug reports. Explaining repeatedly that what they're complaining about is not a libWrapper bug takes some time, and unfortunately this is not a rare issue, with some more extreme cases of people DMing me on Discord directly somewhat aggressively.

It is necessary to improve the library such that it is clearer which errors are actually caused by libWrapper, and which ones are simply detected by libWrapper.

These misunderstandings seem to be caused mostly by two different issues:

1. Error messages are too technical. Users see the libWrapper prefix on an error and assign blame immediately, since they do not have the technical knowledge to understand what the error message means.

   Possible improvements here are (in no particular order):

   • Document what kind of support we provide, and point anything else to either module-specific support channels, or the community Foundry support channels (Discord #modules-troubleshooting / Reddit).

     • "About" page inside the libWrapper settings dialog.
     • README.md
     • Github issue template
       • Explain what kind of information we require to triage an issue.

   • Explain everywhere above that we do not provide support for errors caused by modules that promote or endorse piracy (LibWrapper will work fine, but do not expect support here)

   • Improve error messages

     • Use module names (instead of IDs) for any user-facing error messages.
     • Remove libWrapper prefix from any error message not caused by libWrapper.
     • Add This is *not* a libWrapper bug to a prominent place of the console messages for any issue not caused by libWrapper.
     • If the module provides an issue tracker in its Manifest, provide a link to this in the JS Console error message.
     • Provide a link to the package page in foundryvtt.com, if available.
     • If the error is a failure to find a method to wrap, explicitly state It is likely this module has not been updated for <Foundry version X.Y.Z>. as extra user-facing information.
     • For errors not caused by libWrapper:
       • Notification Message: <Module Name>: Error detected in module. <extra user facing information, if any>
       • Exception message (displayed in the JS console):

         <Notification Message>
         This issue should be reported to the corresponding module developer, it is *not* a libWrapper bug.
         
         Find information about this module here: <`url` entry in manifest; or URL to module page on foundryvtt.com> 
         Report this issue here: <URL to issue tracker for module>. [NOTE: only if `bugs` entry in manifest>
         Ask the community for support at:  [NOTE: Only if no bug tracker above]
         - Discord #modules-troubleshooting channel (https://discord.gg/foundryvtt)
         - Reddit (https://www.reddit.com/r/FoundryVTT)
         
         == Technical Details for Developers:
         Detected by libWrapper.
         Package ID= <Module ID>
         Error= <technical error message>
         

     • For errors caused by libWrapper:
       • Notification Message: libWrapper: Internal error detected. <extra user facing information, if any>
       • Exception message (displayed in the JS console):

         <Notification Message>
         
         Find information about libWrapper here: <libWrapper github homepage> 
         Report this issue here: <URL to libWrapper issue tracker>.
         
         == Technical Details for Developers:
         Caused by libWrapper
         Related Module ID= <Related Module ID>
         Error= <technical error message>
         

   • Show an error message dialog on errors

     • This dialog has the same information as the JS console
     • With hypertext links
     • With an expandable and copy+pastable "Technical Details" section
     • Possibility to disable these messages (and get only notifications)
     • Foundry version updates forcefully re-enable these messages
     • If bug reporter is installed, use it (depends on League-of-Foundry-Developers/bug-reporter#103)

2. The error listeners (see error-listeners.js) are present on every single stack trace containing a hook being called, or an Application being rendered. Many people seem to misunderstand this as indicating libWrapper is causing the errors. Possible improvements here are:

   • Rename the file to listeners.js, which is a less "culprit-sounding" name.
   • Do not prefix libWrapper- to the sourceMap name of the error listeners file.
   • Wait until this is implemented by Foundry, which will remove the need for these listeners to show up in the stack frame.

More suggestions are welcome.

After the last update when the module is active the PF2e system does not load.

Currently, the global libWrapper class exposes a lot of underlying code (methods that start with _) that probably shouldn't be available in global scope.

If I have some time, it might be useful to move these methods off to a private scope that is not exposed to the world. Additionally, it might make sense for the release versions to minify any method name starting in _.

i have alot of modules such as custom hotbar/midi qol/ automated animations etc etc that require this module as a dependancy its also happening with multi level tokens. where im having a constant flashing token with an error due to the libwrapper. please fix this asap. i need these running for my games.

"libWrapper: Can't wrap 'Token.prototype._onUpdateTokenActor', target does not exist or could not be found."

Not sure what this really means or how to fix it.

Since v1.0.4, the full libWrapper library officially supports instance-specific as well as inherited-method wrapping.

Unfortunately, adding this to the shim would be quite a bit of work, and as such the shim does not support this as of now.

For example, the following will work with the full library but throw an exception with the shim:

class A {
	x() {
		console.log("inside A.x");
	}
}

class B extends A {
}

libWrapper.register(MODULE_ID, 'A.prototype.x', function(original) {
	console.log("inside A.prototype.x wrapper");
});
// <- Works with both the full library and the shim

libWrapper.register(MODULE_ID, 'B.prototype.x', function(original) {
	console.log("inside B.prototype.x wrapper");
});
// <- If using the shim, throws exception 'Uncaught libWrapper Shim: "B.prototype.x" does not exist or could not be found.'
//    If using the full library, works fine.

globalThis.a = new A();
libWrapper.register(MODULE_ID, 'a.x', function(original) {
	console.log("inside a.x wrapper");
});
// <- If using the shim, throws exception 'Uncaught libWrapper Shim: "a.x" does not exist or could not be found.'
//    If using the full library, works fine.

It is worth brainstorming whether this could be implemented in a simple way that would be appropriate for the shim.

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?

The documentation states:

- If another module already has an `OVERRIDE` wrapper registered to the same method:
        - If the GM has explicitly given your module priority over the existing one, your wrapper will take over.
        - Otherwise, libWrapper will throw a `libWrapper.AlreadyOverriddenError` exception. This exception can be caught by your module in order to fail gracefully and activate fallback code.

However, because failed OVERRIDE wrappers do not add the module to the UI, if the only wrapper by a given module is an OVERRIDE, it will be impossible to prioritize it without first disabling the conflicting module.

This should be fixed, so that a failed OVERRIDE wrapper still adds the module to the UI.

Essentially, instead of generating the list of modules by looking at active wrappers, modules should be added to the list as soon as they attempt to register a wrapper - even if the registration later fails.

As a work-around, modules can register a dummy wrapper to make sure they're added to the modules list, e.g.:

class _dummy {
  function target() {
  }
}
globalThis._dummy_MyModule_wrapper_target = _dummy;

libWrapper.register('my-module', '_dummy_MyModule_wrapper_target.prototype.target', ()=>{});

Thanks to Cris#6864 on Discord for reporting this.

If an existing OVERRIDE wrapper gets taken over by a different module because they have priority, right now modules are not notified.

It might make sense to add an option API to allow modules to set a callback that should be fired when this event happens.

This could use the Hooks system, e.g. a hook libWrapper.OverrideLost

Might make sense to add this also for similar events, e.g. libWrapper.ConflictDetected.

ilcom94 on Discord was trying to wrap the longRest method with libWrapper, and noticed that he was seeing an infinite loop. This did not happen without libWrapper. After some debugging, he tracked this down to an interaction with the module Magic Item, which does not use libWrapper.

With the Magic Items module enabled, the infinite loop can be reproduced using e.g. the following example code:

libWrapper.register("some-module", "CONFIG.Actor.entityClass.prototype.longRest", 
async function(wrapped, args={}) {
    console.log("longRest pre", args);
    // can access/modify args here if you want, e.g. args.dialog or args.chat or args.newDay
    let data = await wrapped(args);
    console.log("longRest post", data);
    return data;
});

Investigation so far

It looks like the Magic Item module wraps instances directly, using the following code:

this.actor.longRest = this.longRest(this.actor.longRest, this);

When libWrapper wraps the prototype's longRest, it creates a Handler object which triggers the libWrapper Wrapper functionality.

Because the actor by default doesn't have an instance-specific longRest object, then this.actor.longRest reads that Handler object. Magic Items then assigns its wrapper to this.actor.longRest, creating an instance-specific member instead of trying to replace the class prototype.

Usually, if someone tries to re-assign a Handler object, libWrapper will detect this, redirect said handler to the original method, and re-wrap the method after the assignment to ensure the libWrapper wrappers have primacy. However, because in this situation the Handler object isn't re-assigned, but instead the instance's member is modified directly, this code never triggers.

When libWrapper finishes the call chain and goes to call the original method, it will trigger the instance-specific method which contains the Magic Items wrapper, which then triggers libWrapper, and this repeats all over again.

Assuming this is the true cause, this issue will only occur in the case where someone manually wraps an inherited class or instance directly, while libWrapper was used to wrap the parent class or prototype.

Some further testing needs to be done to ensure this is the actual issue.

Solution

Some investigation/brain-storming needs to be done to decide how to solve this.

Assuming the above hypothesis is correct, we might not wish to trigger the instance-specific method after our wrappers if we have wrapped the prototype. We also need to consider what to do with regards to the Handler read by Magic Items - if someone were to wrap the prototype directly, Magic Items would be pointed towards the original method directly, skipping libWrapper, which might not be desirable.

Meanwhile, we need to ensure this situation is part of the test suite.

Is your feature request related to a problem, e.g. with a module? Please describe.
Currently there is no way to localize text for other languages. All settings of libWrapper are displayed in English invariant of what the games language is set to. This can be problematic for people that are not fluent in the English language.

Describe the solution you'd like
Implement foundries default solution for translation using the i18n framework. Specify translation and keys in dedicated files, each containing the translation for a specific local.

Describe alternatives you've considered
There really is no alternative. It would simple improve accessibility for the user.

Additional context
I do understand why libWrapper itself does not prioritize translations, however I would still emphasize that the amount of work to localize the project is minimal and actual translations can be done by the community.

In the console view I am seeing the following issues, which seem to be adding a slight delay to the start up:

• The Document#_id property is deprecated in favor of Document#id or Document#data#_id. Support will be removed in 0.9.0
• You are calling PlaceableObject#setFlag which has been deprecated in favor of Document#setFlag. Support will be removed in 0.9.0
• You are retrieving the value of CONFIG.Item.entityClass which has been renamed to CONFIG.Item.documentClass. Support for the old configuration value will be removed in 0.9.0
• PF2e | Failed to execute onBeforePrepareData on rule element [object Object]. TypeError: Cannot read property 'type' of null
• PF2E | Unknown rule element PF2E.RuleElement.FlatModifer

I also exported the console for the session:

Console_Export_Forge_Foundry.log

Some modules, like Mess, attempt to patch existing methods by converting them to string first using toString(), applying a regex, and then replacing the original method.

Unfortunately, this is completely incompatible with any method wrapped by libWrapper, as toString() returns the source code of the libWrapper handler.

If possible, it might be a good idea to override the toString() method of the libWrapper handler function objects, and outright throw an exception, or if possible return the original method's toString() result.

Commit 26f0970 implements checker CallOrderChecker which allows automatic checking of wrapper call order, including detailed validation of argument passing, return values, and this. It also helps make unit tests much more readable.

Before: https://github.com/ruipin/fvtt-lib-wrapper/blob/8cddf9f593e8587739fa3f9269ca7b55a775b07e/tests/test_wrapper.js
After: https://github.com/ruipin/fvtt-lib-wrapper/blob/26f097024a37b2b088514117809a6b3f75dd231b/tests/test_wrapper.js

These changes allowed me to easily find bug #13.

As such, the remaining test cases (e.g. test_lib.js) should be ported to use this checker.

Right now, whenever libWrapper appears in the callstack, it is usually quite cryptic.

For example:

layer.targetObjects is not a function
    at Canvas._onDragLeftDrop (foundry.js:10723)
    at O._call_wrapped (wrapper.js:298)
    at O.call_wrapper (wrapper.js:389)
    at Canvas.canvasOnDragLeftDrop (main.js:127)
    at O.call_wrapper (wrapper.js:421)
    at Canvas.🎁@handler0(Canvas.prototype._onDragLeftDrop) (wrapper.js:133)
    at MouseInteractionManager.callback (foundry.js:36392)
    at MouseInteractionManager._handleDragDrop (foundry.js:36767)
    at MouseInteractionManager._handleMouseUp (foundry.js:36737)
    at e.a.emit (index.js:181)

One can see that the libWrapper code is in a wrapper.js file.

This should be improved - the wrapper.js file should be renamed to libWrapper-wrapper.js. The file lib-wrapper.js should be renamed libWrapper-lib.js. These are the two files that will often show up in call-stacks, and thus we should make sure they contain the full library name.

In addition, the Canvas.🎁@handler0(Canvas.prototype._onDragLeftDrop) function name should probably be shortened, for example to Canvas.🎁Canvas.prototype._onDragLeftDrop#0.

Lastly, it makes sense to also use Function.displayName for the browsers that support it (i.e. Firefox).

Right now, libWrapper does not support wrapping constructors.

Some investigation should be done as to whether this is actually feasible, and if so we should implement this.

Something like patchwork.js might serve as inspiration.

Currently, the test suite uses tape but does not measure code coverage.

We could potentially switch to node-tap which has native coverage support, although it seems it depends on nyc which does not support code coverage for ESModules yet.

Alternatively, we could try using c8 for coverage, while staying on tape.

As it stands, modules are unable to signal a desired prioritization other than "WRAPPER", "MIXED", and "OVERRIDE".

A suggestion that has come my way is to add optional before and after constraints that will be used to decide on default prioritization. These would be lists of module IDs (or a single module ID), which would run by default before/after the given wrapper.

Example:

libWrapper.register("module-a", "Foo.prototype.bar", "MIXED", {before: "module-b", after: ["module-c", "module-d"]});

Optionally, we could have the dictionary be the third parameter (when it is not a string, for backward compatibility), and have the wrapper type be one of the dictionary values:

libWrapper.register("module-a", "Foo.prototype.bar", {type: "MIXED", before: "module-b", after: ["module-c", "module-d"]});

The user could then be warned if these constraints conflict (including when it's because the users themselves changed prioritization).

I can run norc's custom hotbar and any number of other modules that don't need libwrapper just fine but if I try to run custom hotbar and libwrapper together the game gets stuck loading at 100% and the console keeps spitting out thousands of messages about trying to load custom hotbar

Foundry 0.7.9, lib wrapper 1.5.3.0, dnd5e 1.2.4.
Server: docker image on VM running Ubuntu
Browser: Safari Technical Preview, Release 123 (Safari 14.2, WebKit 16612.1.7.10), on macOS 11.3.1

Disclaimer

Yes, I know Foundry doesn't actively support Safari. But please, hear me out... Foundry runs well on Safari Technical Preview, and on macOS, Safari is dramatically faster than Chrome or Firefox. I mean, sometimes double the framerate on an older laptop. And this particular issue, I think, involves the lib wrapper shim, so it breaks a lot of modules.

Issue
When using lib wrapper alone, or using other modules that use the default lib wrapper shim code, I see this error in the console:

SyntaxError: Invalid regular expression: invalid group specifier name

The result is that lib wrapper doesn't load, nor do other modules that use that shim. Nor do modules that depend on libwrapper. So this error has rather large consequences to getting Foundry to work in Safari.

Proposed Solution

I think this error is due to the fact that Safari does not accept negative or positive lookahead regex expressions. See this StackOverflow question or this one for examples.

Specifically, this regex in shim.js may need to be changed in order to work in Safari:
const MODULE_ID = ((import.meta?.url ?? Error().stack)?.match(/(?<=\/)modules\/.+(?=\/)/i)??[])[0]?.split('/')?.find(n => n && game.modules.has(n));

If that can be changed to not use the positive lookbehind, then hopefully lib wrapper will load on Safari, and other modules can adopt the change as well in their shim.

Thanks!

Because generator functions are synctactic sugar around a Generator factory, trying to wrap a generator the naive way will trigger a libWrapper exception:

game.generatorFn = function*(...args) {
  yield args[0];
  yield args[1];
  yield args[2];
};

libWrapper.register('package_id', 'game.generatorFn',  function*(wrapped, ...args) {
  const gen = wrapped(...args);
  yield gen.next().value+1;
  yield gen.next().value+1;
  yield gen.next().value+1;
}, "WRAPPER");

console.log(game.generatorFn(1).next());
// <- The wrapper for 'game.generatorFn' registered by module 'package_id' with type WRAPPER did not chain the call to the next wrapper, which breaks a libWrapper API requirement. This wrapper will be unregistered.

A workaround is to divide the wrapper into two parts, a factory, and a generator:

game.generatorFn = function*(...args) {
  yield args[0];
  yield args[1];
  yield args[2];
};

function* generatorFn_wrapped(wrapped, ...args) {
  yield wrapped.next().value+1;
  yield wrapped.next().value+1;
  yield wrapped.next().value+1;
}

libWrapper.register('package_id', 'game.generatorFn',  function(wrapped, ...args) {
  return generatorFn_wrapped(wrapped(...args), ...args);
}, "WRAPPER");

console.log(game.generatorFn(1,2,3).next().value);
// <- 2

It might be a good idea to have libWrapper do this automatically when it detects the wrapper function is a generator. When libWrapper.register detects that the wrapper function is a generator, it should automatically create a corresponding factory, and register that instead, similar to above.

To detect whether a function is a generator, the following StackOverflow thread is useful:
https://stackoverflow.com/questions/21559365/how-to-identify-an-es6-generator

I am currently trying to implement this library and I am running into following problem when this module isn't active:

TypeError: Cannot read property 'value' of undefined
    at Function.register (shim.js:34)
    at PlayerData.registerMouseWheel (player-data.js:133)
    at patch_ruler (patch-ruler.js:18)
    at difficult-terrain.js:8
    at Function._call (foundry.js:2239)
    at Function.call (foundry.js:2224)
    at Canvas.draw (foundry.js:10911)
    at async Scene.view (foundry.js:29232)
    at async Game.initializeCanvas (foundry.js:5942)
    at async Game.setupGame (foundry.js:5824)

while my code looks like this:

    registerMouseWheel() {
        let self = this;
        libWrapper.register(MODULE_ID, 'canvas.activeLayer._onMouseWheel', function (wrapped, e) {
            if (!self.rulerArray.some(value => value.waypoints.length > 0))
                return wrapped(e);

            if (Date.now() - self.lastRegisteredMouseWheel < self.gameSettings.interval)
                return

            self.lastRegisteredMouseWheel = Date.now();
            if (e.deltaY < 0) {
                self.setTerrainMultiplier(self.gameSettings.increment)
                self.updateRuler(e);
            } else {
                self.setTerrainMultiplier(-self.gameSettings.increment)
                self.updateRuler(e);
            }
        }, 'MIXED');

I have no problem when the lib is activated but I would expect that this would still work, even when no priority is supported.

Full code if needed:
https://github.com/Nordiii/difficultterrain/tree/feature/libwrapper_support

Thanks for your time.

Foundry 0.8.9
libwrapper 1.10.3.0

It might be useful for modules to be able to query the current priorities, or request a list of modules that is higher/lower priority than them.

Unsure how to handle unprioritized modules, though.

Inspired by some modules like Mess which patch methods using a regex, I have been wondering about standardising an API to do this officially supported by libWrapper, and that attempts to handle possible issues such as conflicts.

This API could be e.g.

libWrapper.patch("module-id", "path.to.method", /match/, "replace");

and for ease-of-use, also support

libWrapper.patch("module-id", "path.to.method",
  [/match1/, "replace1"],
  [/match2/, "replace2"],
  ...,
  [/matchN/, "replaceN"]
);

Each pair would behave exactly like a call to String.prototype.replace, so would also support replacement methods, etc.

libWrapper would then be responsible for working out which method to patch (including, if necessary, modifying a prior patch), and automatically setting up a corresponding OVERRIDE.

• If there's already an override, this method would fail, unless the given module has a higher priority, in which case the existing override gets discarded (and a conflict is signalled).
• Someone registering an OVERRIDE that is higher priority than all of the modules that set up a patch will cause the patch to be discarded, similar to the current behaviour when there are two OVERRIDE conflicts.
• If libWrapper cannot resolve the function to source code, it throws an exception.
• If any individual patch "pair" leaves the source code unchanged, libWrapper throws an exception. A third parameter can be used to signal that patches are not required.
  • The exception is if the patch is being applied on top of an already patched method, and the current module is higher priority than the existing module(s). In such a case, the existing patches are dropped, and the new module takes over.
• If the final source code is unchanged from the original (i.e. none of the patch pairs applied successfuly), libWrapper throws an exception.
• If the final source code fails to parse, libWrapper throws an exception.
• Patches (or to be more exact, the corresponding OVERRIDE) cannot be unregistered.
• The 🎁 emoji will be prefixed to the patched function names, to make it obvious in the call stack the method has been modified.

This would mean that two modules patching the same method, as long as their regexes don't overlap, would still be compatible.

Modules would be responsible for catching the exceptions if they require fall-back behaviour. As usual, uncaught exceptions will be raised to the user as errors.

The shim could be given a naive fallback implementation, that just does toString(), replace, and then overwrites the original.

Currently this crashes 0.8.7 and stops many other modules from functioning including DAE that are dependent on this module.

class Foo {
    async bar() {
        return 0;
    }
}

libWrapper.register("some-module", "Foo.prototype.bar", async function (wrapped) {
    await new Promise(resolve => setTimeout(resolve, 1000));
    const a = await wrapped();
    return a + 1;
});

const foo = new Foo();

await foo.bar();

Uncaught libWrapper: This wrapper function is no longer valid, and must not be called. This is most likely caused by an issue in the 'Foo.prototype.bar' wrapper registered by module 'some-module'.

It's probably necessary to add a new suffix to support asynchronous functions, because we can't know whether wrapped is called asynchronously or the function simply returns a promise but wrapped is called synchronously.

libWrapper.register("some-module", "Foo.prototype.bar#async", ...)

FVTT 0.7.9 (node)
DnD5e System 1.2.0
libWrapper 1.0.5.3
Compatibily erro with Midi QOL 0.3.43
I tried to reinstall both modules and without success, I know it depends on the other module and the problem may be in it, if it is the case I will report to the author of mid qol but I thought it opportune to warn.
Thank you

I have an override like this

Object.defineProperty(Token.prototype, 'inCombat', {get: inCombat})

function inCombat() {
    const combat = ui.combat.combat;
    if (!combat) return false;
    const combatants = combat.getCombatantByToken(this.id);
    return combatants.some(c => c !== undefined);
}

but I can't find an example of that kind of overrides in the documentation. Is wrapping this kind of overrides possible in the current version?

Having this module activated with Scene Clicker prevents Actor sheets from opening from the sidebar.

Not sure if this is a Scene Clicker issue or libWrapper but I narrowed down the issue only persists if I have both activated and just started after latest lib wrapper update...

LibWrapper should detect that the user has passed unknown paramets (e.g. unknown options), and warn the user that a given module might not be fully compatible with the current version of the library.

Right now, if we have the following class hierarchy:

• Class A implements method foo, wrapped by method WrapperA using libWrapper.
• Class B extends A implements method foo which calls super.foo() (i.e. A.foo()).

By default:

• A.foo() calls WrapperA() -> A.foo().
• B.foo() calls B.foo() -> WrapperA() -> A.foo().

If we wrap B.foo with method WrapperB using libWrapper, the situation changes:

• A.foo() calls WrapperA() -> A.foo() (no change).
• B.foo() calls WrapperB() -> WrapperA() -> B.foo() -> A.foo().

Notice that WrapperA() and B.foo() swapped position in the B.foo() call order.

This is because libWrapper currently gives priority to any inherited wrappers (i.e. WrapperA()), and only once they're exhausted will it consider calling the wrapped method (i.e. B.foo()). Since before, B.foo() was not controlled by libWrapper, it would not check for inherited wrappers.

This is an inconsistency which in my opinion breaches the principle of least surprise, since this is not how class inheritance nor traditional JS wrappers would work.

Specifically, it means libWrapper can accidentally bubble up wrappers from base classes all the way up to child classes, with which the wrapper might be incompatible.

This prioritization should be removed, such that:

• A.foo() calls WrapperA() -> A.foo() (no change).
• B.foo() calls WrapperB() -> B.foo() -> WrapperA() -> A.foo().

The test cases which detected this issue will need to be updated with the correct call order once this bug is fixed:

Right now, any potential module conflicts detected by libWrapper are shown only in the "Conflicts" tab in the settings menu, as well as in the JS console.

Due to the number of people that don't realise this functionality exists, I believe we should change this so that by default, conflicts are notified to the GM whenever they occur, and the GM can choose to disable this in the settings menu.

Ideally, we'd warn only once per detected conflict. A more advanced implementation of this feature could allow the GM to ignore conflicts between specific modules one-by-one.

Hello,

I have started to see a number of errors pop-up in connection with libwrapper. Below is the Conflicts information. Let me know if additional information will assist. Thank you!

https://screenrec.com/share/WKvHRuFejs