League-of-Foundry-Developers / fvtt-module-furnace

34 stars 12 forks source link

Module prevents use of actor and speaker variables in macros #100

Closed schlosrat closed 3 years ago

schlosrat commented 3 years ago

I've got a macro I use (shown below) which works fine without this module loaded, but which generated syntax errors if I load this module.

const flavor = "Breather";
// Change the value between "" to change flavor on chat message

const speaker = ChatMessage.getSpeaker();
// let actor = null;
let macroActor = null;
if (speaker.token) macroActor = game.actors.tokens[speaker.token];
if (!macroActor) macroActor = game.actors.get(speaker.actor);
if (macroActor) {
   let rollData = {};
   rollData.level = macroActor.data.data.level;
   rollData.cons = macroActor.data.data.abilities.cons.total;

   const chatMessage = {flavor, speaker};

   let roll = new Roll("1d6 + @level + @cons", rollData).roll();
   roll.toMessage(chatMessage);
}

Without The Furnace loaded it works as I'd expect, but with The Furnace loaded I get this in my game console

SyntaxError: Identifier 'speaker' has already been declared (Macros.js:224)
    at new Function (<anonymous>)
    at Macro.callScriptMacroFunction [as callScriptFunction] (Macros.js:176)
    at Macro.renderMacro [as renderContent] (Macros.js:197)
    at Macro.executeMacro (Macros.js:221)
    at Hotbar._onClickMacro (foundry.js:20566)
    at HTMLLIElement.dispatch (jquery.min.js:2)
    at HTMLLIElement.v.handle (jquery.min.js:2)
kakaroto commented 3 years ago

The speaker and actor variables already exist in both cases, with or without the module, but without the module, you can re-declare it because it uses eval to evaluate the macro's script. With Furnace, the macro gets called as a function, which allows it to be async (note: the same thing would happen in 0.8.x), so your macro would work if you don't use the const before speaker, as you would just overwrite the variable's value instead of trying to re-declare it. In your case though, you wouldn't need to do ChatMessage.getSpeaker() as Furnace already does it and the speaker variable is already set to that (core Foundry does the same, so you should be able to access the speaker variable in your macro with or without the module). See https://github.com/League-of-Foundry-Developers/fvtt-module-furnace/blob/master/Macros/Macros.js#L151