mclemente / fvtt-advanced-macros

Use handlebars templating, recursive macro calls and call macros with arguments or directly from chat.
MIT License
15 stars 9 forks source link

[PF2e] args no longer defined #41

Closed MrVauxs closed 2 years ago

MrVauxs commented 2 years ago

In previous versions of the module / Foundry, Advanced Macros automatically provided the args property, by default being an empty array.

Now it is no longer the case, breaking a lot of macros.

Foundry 10.288 Pathfinder 2e v4.2.6 Advanced Macros 1.17

mclemente commented 2 years ago

Could you share some macro examples so I can test them?

MrVauxs commented 2 years ago

PF2e Animation Macros is a module that was affected by this, now pretty much all of it's macros being unusable by the user (the module still works due to having Automated Animations and other sources pass the args, but without them, the macros break where they once just had an empty args array).

I say that instead of providing a macro, as they are all dependent on other modules. Still, will try to provide one, if I find one.

MrVauxs commented 2 years ago

A simple console.log(args) no longer works and errors with args is not defined.

mclemente commented 2 years ago

PF2e 4.2.5 introduced it's own monkey-patch to _executeScript and that's what broke them https://github.com/foundryvtt/pf2e/blob/master/src/module/macro.ts. I can't fix this by turning this module into a library so it could wrap the Macro class before they inherit it since they're overriding the method, like I did when a similar issue happened with PF1. I also can't wrap MacroPF2e since the devs haven't made it global through globalThis or CONFIG.PF2E like I do with Macro. You're better off opening this issue on the PF2e repo requesting them to either change it or make it global, or consider dropping the Advanced Macros requirement and rewrite your macros to work under PF2e's Macros.

stwlam commented 2 years ago

A couple points here:

mclemente commented 2 years ago
  • The system isn't monkey-patching Macro

I meant to say "override", sorry.

  • Like any document class, it is exposed in CONFIG (CONFIG.Macro.documentClass in this case).

Thanks, I needed that insight. I'm used to calling the class directly instead of using CONFIG (e.g. Macro.prototype instead of CONFIG.Macro.documentClass.prototype), so I was lost when I couldn't find it on globalThis or on the system's CONFIG.

MrVauxs commented 2 years ago

For some reason the issue remains when using the following code, and only if it's inside of a module, if you copy paste this into a macro, it works.

async function runJB2Apf2eMacro(
    macroName,
    args,
    compendiumName = "pf2e-jb2a-macros.Macros"
) {
    const useLocal = game.settings.get("pf2e-jb2a-macros", "useLocalMacros");
    const pack = game.packs.get(compendiumName);
    if (pack) {
        const macro_data = useLocal ? await game.macros.getName(macroName).toObject() : (await pack.getDocuments()).find((i) => i.name === macroName)?.toObject();

        if (macro_data) {
            const temp_macro = new Macro(macro_data);
            temp_macro.ownership.default = CONST.DOCUMENT_PERMISSION_LEVELS.OWNER;
            debug(`Running ${macroName} macro`, { macro_data, temp_macro, args });
            temp_macro.execute(args);
        } else {
            ui.notifications.error("Macro " + macroName + " not found");
        }
    } else {
        ui.notifications.error("Compendium " + compendiumName + " not found");
    }
};

You can use it like this runJB2Apf2eMacro("Scorching Ray", { args: "123" }) and it will play the macro from PF2e Animation Macros compendium.

MrVauxs commented 2 years ago

The same exact error (just different source) happens when Sequencer tries to run a macro with AM enabled. image

mclemente commented 2 years ago

Ok, it should work for both cases now.