pf2e
pf2e copied to clipboard
foundryvtt
Supply all roll callback parameters on attack with consumeAmmo
The consumeAmmo logic wraps an existing caller supplied attack roll callback. Unlike a RollParameter callback, which has only the Roll as an argument, an attack roll callback is a CheckRollCallback, which has four parameters. The consumeAmmo wrapper would only pass along the first of these.
Since the wrapper doesn't care about any of the arguments, use the rest argument syntax to pass along whatever argument(s) the callback was called with. This way the code can work with any callback.
The callback also can be async, so await it, otherwise it won't finish before the code in CheckPF2e.roll() that awaits the callback continues.
The callback supplied to rolls was already awaited for weapons that don't consume ammo it still is. It's always been part of the user facing API in CheckPF2e.roll() and from that every exposed check roll function.
https://github.com/foundryvtt/pf2e/blob/c96b0f77017ea816e1d6d8ca4623a735d6a679ed/src/module/system/check/check.ts#L322
The callback was first awaited back in 2021, 39bcea2a8aacf40071d9507d064fd15ec11e7c33
This is very useful, since editing the message, e.g. to add extra content to the flavor text, is something done by macros and modules in callbacks and that requires an async document update.
This is actually true. We do await a user provided callback, its just that weapons with ammo specifically lose that functionality when it becomes wrapped.
What is specifically the goal you're trying to accomplish here? I agree with this change, but it'll be easier to pitch with a reason.
The callback being different breaks the Spellstrike macro in workbench for weapons that consume ammo. This is how I noticed it.
It uses the callback to get access to the chat message from the strike roll it does. It modifies the message to add flags that indicate the spell/strike choice used in the roll, so that the hero point reroll, also part of the macro, can work correctly. And it adds to the flavor text to show the strike action is part of spellstrike and the spell used.
The message is only supplied to the callback, not returned from the strike call, and the consume ammo callback wrapper strips it out.
Since it modifies the chat message, it uses ChatMessage#setFlag() and other async functions. It would be best if it was awaited, so that the message can be altered before the action continues to use it. Otherwise there is stutter and the chat log doesn't scroll properly, when a message is altered after being rendered.
We will need a new way for all messages other than the callback to make modifications as the performance leaves something to be desired (double updates!), but I'll have to do some research into what different modules are doing with it.