Add "miss" versions for effects

[cirrahn]

This adds a "miss" version for each weapon which hits a target.

Note that:

• The "miss" versions are very rough.
• I've never played LANCER before, so some of these may be nonsensical!

Demo video

---

This PR is also bloated with the following extras:

• Add a volume slider which controls a multiplier for sound volume in each macro (I was going deaf testing these)
• Add "active combatant" as a fallback instead of "currently selected token" (we like "click to release token")
• Add a "macro" (a script in the tooling folder) to run each FX macro in-game, this was used to make the demo above
• Add a system to pack/unpack the macro DB to allow the macros to be edited in a coding environment outside of Foundry
• Add a NodeJS package.json to define commands for pack/unpack scripts
• Fix on-startup error due to empty en.json language file
• Fix the "Displacer" macro which was broken by a bad git merge
• Developer git/IDE QoL files

I should have made these separate PRs, but I made this for an upcoming game and do not have the spare time. Sorry!

[dodgepong]

This is very cool! Though you're right, this is a lot for a single PR 😅 Would make it a lot easier to review if they were separate, but at least the commits appear nicely discrete.

Defaulting to active combatant is a good idea! The ID of the firing actor/token should be available in the reroll data, so arguably it would be cleaner to use that? The trick is passing it into the macro without also using Advanced Macros since I don't believe passing parameters to macros is supported outside of that module yet (I'm given to believe that this could change though). I suppose prepending the macro command like you're doing for isMiss would work, perhaps?

Have you checked how this behaves when targeting multiple enemies with a single attack where some are misses and some are hits? A quick glance at this without testing suggests that if one attack misses, the miss animation will play for every attack loop for each target.

[cirrahn]

Have you checked how this behaves when targeting multiple enemies with a single attack where some are misses and some are hits?

A comically large oversight!

How would you feel about a solution to this, and the other points you raised, which moved the message parsing into the macro itself, but as an API call to the module? Something like:

---

In macro (API format from "community best practices")

// use the prepend trick to inject the triggering message ID
const messageId = "abc..."

const {
  targetsMissed,  // a set of token IDs for which the attack missed
  sourceToken,  // the token doing the action
} = game.modules.get('lancer-weapon-fx').api.getMacroVariables(
  // allow macro to be run without prepending the message ID. Useful?
  typeof messageId === "undefined" ? null : messageId,
  // pass the regular macro call actor through as a fallback
  actor,
)

// ...

for (let target of Array.from(game.user.targets)) {
  // ...
  sequence.effect()
    // ...
    .missed(targetsMissed.has([target.id]))
}

In module

game.modules.get('lancer-weapon-fx').api = class {
  static getMacroVariables(messageId, actor) {
    // parse the message and return info relevant to the macro
    // ... `game.messages.get(messageId)` ...
    // (repurpose current message-parsing function)

    // if no `messageId` passed, fall back on e.g.
    // `canvas.tokens.controlled[0] ?? game.combat?.current?.tokenId`
    // or `actor.token`
    // for the `sourceToken`

    // if no `messageId` passed, fall back on `new Set()` for `targetsMissed`
    // (i.e. assume all hits)
  }
}

---

This would work out cleaner than my current implementation, as all the copy-paste boilerplate in the macros would get refactored out to a single API method. The same API idea could be used for the volume setting too. For example:

In macro

// ...
  .volume(game.modules.get('lancer-weapon-fx').api.getEffectVolume(0.6))
// ...

In module

game.modules.get('lancer-weapon-fx').api = class {
  static getMacroVariables(messageId, actor) {
    // ... 
  }

  static getEffectVolume(volume) {
    return volume * game.settings.get("lancer-weapon-fx", "volume")
  }
}

[cirrahn]

@dodgepong I have updated this with the following changes and rework. I think it turned out much better!

• Each macro now starts with a call to the module's API to get source, targets, and hit/miss information
  • If a message ID is passed in to the macro (using the command prepend trick from before) then this is passed via the API call and the message is parsed to get the most accurate info possible from the message's encoded roll data
  • If there is no message ID the macro runs anyway and picks sensible defaults (game.user.targets for the targets, canvas.tokens.controlled[0] ?? game.combat?.current?.tokenId for the source, assume all hits)
• An API method has been added to get volume instead of copy-pasting the same code in each macro
• (Some sounds which did not have a .volume(...) set now have a 1.0 volume set which can be modified by config)

Demo video:

demo_2.webm

This is a PR into my other PR #24 with the same commits. Since this is now even more bloated I have also made https://github.com/cirrahn/lancer-weapon-fx/pull/1 for viewing only the commits from my second branch.

[Z3nner]

I'm just the art guy, but that branch indicates it has conflicts that must be resolved in packs/weaponfx.db

[cirrahn]

@Z3nner just from duplication with #22. Fixed!

[Z3nner]

Awesome, thank you! I just wanna give dodgepong one more chance to review what he'd like before we move forward. I greatly appreciate all this effort and I'm excited to see how much better the module is for it.

[dodgepong]

I hope to have a chance to look at this more closely sometime this weekend, but it looks promising so far! The API call trick seems perfect!

One "quick" note: does it make sense to add a step to the CI build script to pack the macros, or should the macro repacks be committed to git? I could see a world where the .db files are added to .gitignore and we purely rely on the packing scripts to make the .db files so that no mistakes are made with them (see: Displacer merge mistake).

Also I haven't yet checked if the pack scripts are deterministic/stable such that they always produce an identical output (that is, running the script over and over on the same input will result in identical output with no git diff).

[cirrahn]

does it make sense to add a step to the CI build script

I have seen examples of it done every which way! If you want that added it is easily done. A method I like is committing the file but testing in CI to ensure there is no diff when re-generating it.

if the pack scripts are deterministic/stable

I was relying on Node returning the files in a stable order when reading the directory, but I don't know if that's guaranteed on every platform. Added a .sort.

@dodgepong I have pushed two commits here with the above changes: https://github.com/cirrahn/lancer-weapon-fx/pull/2 (can be added to this branch if wanted) and here is the result of the new test action: https://github.com/cirrahn/lancer-weapon-fx/actions/runs/6508098038/job/17676790735

If you would prefer something else let me know!

[dodgepong]

Alright, finally got a chance to play around with this a bit. All in all it works really well! The source/target token fallbacks work excellently, that's going to be a huge QoL change for people.

Regarding the Volume setting:

• Is there a reason the minimum is 0.1 instead of 0? I can see some people wanting to just mute them entirely in some cases, not sure if that breaks anything.
• Now that we have a shiny new en.json, I suppose we might as well move the "Sound Volume" string into there.
• Does the Volume setting affect all users, or could they each set their own personal volume? Or would that change based on who "ran" the macro?

Regarding the miss animations:

• Overall I think they look good! (Though I did not test every one of them.) I'll defer to @Z3nner on the artistry of them (e.g. perhaps some of the misses from explosive weapons should still have their explosions, just off-target?).
• One odd issue I noticed, and I'm not sure how much it can even be considered a bug, is that for some attacks, if they target multiple targets, the animations for "hit" attacks all wait for the previous animation to complete, but the animations for "misses" all play immediately. I think this is related to the presence or absence of a .waitUntilFinished() call, perhaps? For example, the Missile animation works correctly (always plays in order regardless of miss) whereas Thermal Rifle behaves like this: Foundry_Virtual_Tabletop_2023-10-13_19-09-34.webm
• The reason I don't if this can be considered a bug is because it's sort of up in the air how a given weapon should handle multi-targeting. Most multi-targeting is an AOE attack, so the attack ought to only be played once total rather than shooting once per target, and even if they shoot once per target, sometimes it ought to be sequential and sometimes it ought to be simultaneous. Some multi-targets (e.g. Sharanga Missiles, T2 Barricade Graviton Lance) ought to be fired twice. And let's not get started on attacks that both shoot twice and are AOE (Pinaka 😓). So in that regard, there's a solid argument to be made that this was already broken in a way. I guess it would be nice to at least keep it only as broken as it already was? At some point we may need to have a way to pass in AOE handling to each macro, e.g. how it handles multi-targeting. Flamethrower is a bit unique because it calculates the midpoint of all the targets and shoots at that, which could be applies to other AOE weapons, but whether or not an attack is AOE is dependent on the weapon, not the macro... it's a rabbit hole.

A method I like is committing the file but testing in CI to ensure there is no diff when re-generating it.

This is probably fine enough! I can see pros and cons of having the .db files as part of the repo so I suppose having a CI check is good enough. The pack scripts seem to work great from my quick test, and those are going to help a lot. Manually making the .db file by copying from the Foundry data files was super obnoxious and produced unstable diffs all the time.

[cirrahn]

Volume setting

• Can now be set to 0 (it works!)
• Added name to lang file
• The volume setting is "world" level, so set by the GM. I have now multiplied it by the user's "ambient" volume setting so that each user can adjust their own volume to taste. This mirrors: setting a playlist sound's volume as the GM -> each user's settings modify it.

---

Miss animation chaining

I am not very familiar with Sequencer so I spent some time reading the docs/code and experimenting and I could not find an easy solution.

My initial idea was: make one sequence per target (i.e. move the new Sequence() ... sequence.play() into the loop) and await it. Unfortunately, unless a sequence section is explicitly told to wait, it "completes" instantly due to the lack of await (or some other mechanism) here. So there is no built-in way to ask for "I have queued up 2 sounds and 2 effects, wait for them all to 100% finish playing" without manually calculating the longest duration between those four sections (including handling for delays, repetitions, ...) and using that duration as the wait value. Suffice to say this is very clunky, and would be difficult to work with and maintain. The "easiest" solution would be to submit a patch to Sequencer to add a per-sequence all-sections-executed-100% mode or hook, but at best this pollutes Sequencer's API and at worst is a breaking change.

I then tried a less extreme version, again using one sequence per target, but forcing the last section to have a delay via an API addition of the form:

const ensureWait = sequence => {
    if (!sequence.sections.length) return;

    const finalSection = sequence.sections.at(-1);
    if (finalSection.shouldWaitUntilFinished) return; // Uses a @protected Sequencer getter https://github.com/fantasycalendar/FoundryVTT-Sequencer/blob/master/src/sections/section.js#L38

    finalSection.waitUntilFinished();
}

This hack produced an improvement in the timings of most miss animations, but it was not a magic bullet. It also relies on Sequencer internals, so I would be hesitant to use it.

The best result I found was from adjusting the last "executes on both hit and miss" section with a wait of the form:

.waitUntilFinished(targetsMissed.has(target.id) ? 1000 : -99_999)

where the first value, 1000 here, pads the end of the sequence if it is a miss, and the large negative -99_999 value effectively acts as "no delay" if it is a hit, allowing the sequence to proceed as normal.

At this point however we are talking about manually tuning each miss animation, which I will have to step away from as "not an artist"!

To make it easier to tune them I have added the following debug option:

debug_miss.webm

This flips the default on no data from "assume all hits" to "assume all misses" which means that any macro run on its own (e.g. directly from the compendium) will display its miss behaviour. I found this helpful when testing. If you or @Z3nner have any workflows or ideas for making and editing these macros which you would like to see better expressed then I am happy to expand on this.

And here is the list of the macros which I found had "too fast" miss animations:

packs_source/weaponfx.db/AMR.RPjF4SvsMP3dolyX.js
packs_source/weaponfx.db/Bolt Thrower.6lzGhwqHNYxXY25N.js
packs_source/weaponfx.db/Brood Siblings Molt.qfPYlbW1NHKOEJXF.js
packs_source/weaponfx.db/Charged Blade.kozGi0iZPK18rFlI.js
packs_source/weaponfx.db/Combat Drill.Tyi49hWX1axZsrq2.js
packs_source/weaponfx.db/DD 288.XVCMT31Y9x5NgCf6.js
packs_source/weaponfx.db/DefaultMelee.JRAC43fdq4sRWQl8.js
packs_source/weaponfx.db/Hammer.uIr2lCpJWXbDaowG.js
packs_source/weaponfx.db/Lasers.hWPrLPzlj6Y5RvC5.js
packs_source/weaponfx.db/Nanobot Whip.jHll38UQ9hHzwsf3.js
packs_source/weaponfx.db/Needle Beam.Aj3QspxsjcnNjfol.js
packs_source/weaponfx.db/Plasma Maul.gBqPs99uLDUAroZu.js
packs_source/weaponfx.db/Plasma Rifle.noDQfz2lYaqIc1ii.js
packs_source/weaponfx.db/Plasma Talons.ui1a07lCGBOk4Rp7.js
packs_source/weaponfx.db/Power Knuckle.oGArtZBtIRBcRHCh.js
packs_source/weaponfx.db/Railgun.iOD860W9KokfR3Hy.js
packs_source/weaponfx.db/Retort Loop.NbTm6IW88EKyM6sN.js
packs_source/weaponfx.db/Thermal Rifle.LixHY6HTrY7D3b7S.js
packs_source/weaponfx.db/Torch.zsccsSD1tOOqRc5Q.js
packs_source/weaponfx.db/War Pike.CqlCKrYyJCYdmLks.js

---

.db building

If you (or whoever does the release process) would prefer something else it is easy to change! The same script can be copied into the release action trivially

---

---

Again this is several additional commits so I have made the separate PR here https://github.com/cirrahn/lancer-weapon-fx/pull/3 which stacks on top of this and the previous PR feedback commits.

[dodgepong]

Apologies for taking so long to get back to this review!

At this point however we are talking about manually tuning each miss animation, which I will have to step away from as "not an artist"!

That makes sense. Manually tuning each miss repeat is a bit of a tedious process, and most weapons aren't going to need to worry about this anyway, so I think it makes sense to leave this as something to be improved later.

I say go ahead and push your latest branch to this PR. The only other thing I can think of adding is an update to the README with some dev instructions about how to use the macro packer. Is it worth leaving in the manual-usage instructions if people want to run these macros the old way? I'll defer to @Z3nner on that.

[Z3nner]

manual method is garbage that won't last a resync, I'm okay with it disappearing.

[cirrahn]

Great! I will get back to this when I get some free time. There was one thing I noticed when using it "in anger" (and really from reading sequencer's audio code), which is: sequencer already respects the user's audio settings, so my latest volume changes mean user volume multiplier gets factored in twice. I still think having volume slider for the sound effects of this module is useful, but I need to remove the extra multiplication.

So...

• [x] avoid factoring user volume level into volume calculations
• [x] add dev notes/update README (maybe a CONTRIBUTING.md would be a good place for these as they are not relevant to end users?)
• [x] update PR with all commits

---

Is it worth leaving in the manual-usage instructions if people want to run these macros the old way?

The macros can still be run manually (if the module is active) but creating/updating macros in-game will not persist (although I do not know what the original workflow or "old way" was here!). You can still, for example, unlock the compendium and edit macros in-game for ease/speed when testing, so long as you remember to manually copy the resulting script back out to a .js file prior to re-packing.

[dodgepong]

CONTRIBUTING.md is typically for contribution guidelines/policies, yeah? Or do some people still use it for developer documentation?

I expect the most common questions we'll get are 1) how to add a new animation, and 2) how to edit existing animations. For example, I'm looking at adding a new animation now and I see that we're adopting the convention of naming the .js/.json file as MacroName.MacroID.ext, but I'm not sure if it's fair game to just invent a new macro ID out of whole cloth. Can I just generate any old 16-characater alphanumeric string as long as it doesn't collide with an existing ID in the compendium pack? What of the author and flags.core.sourceId fields?

[cirrahn]

CONTRIBUTING.md is typically for contribution guidelines/policies, yeah? Or do some people still use it for developer documentation?

I've seen combinations of either/both, e.g. 64k stars https://github.com/tiangolo/fastapi/blob/master/CONTRIBUTING.md links to a page with script instructions, 215k stars https://github.com/facebook/react/blob/main/CONTRIBUTING.md links to a page with general guidelines but also script instructions, ...

It tends to be that bigger projects with more contributors use a more robust solution for their contribution guides, but my point is that the first place a potential contributor should look, after the README.md, is the CONTRIBUTING.md file so it seems a good place to put simple developer instructions. This keeps the README as short as possible for module users!

I expect the most common questions we'll get are 1) how to add a new animation, and 2) how to edit existing animations. For example, I'm looking at adding a new animation now and I see that we're adopting the convention of naming the .js/.json file as MacroName.MacroID.ext, but I'm not sure if it's fair game to just invent a new macro ID out of whole cloth.

I will add a script which takes a name and generates the empty files (e.x. npm run new-effect "My Weapon Name").

Can I just generate any old 16-characater alphanumeric string as long as it doesn't collide with an existing ID in the compendium pack?

Yes, that does work! You can even use e.x. "0000000000000000", "0000000000000001", ...

The easiest option is to use Foundry's random ID method. Foundry relies on collisions between randomly-generated IDs being statistically "impossible" so we can do the same. I will reimplement Foundry's ID-generation function as the EULA only permits directly copying Foundry code as part of a package, and this would be part of the build system for a package, so is not covered.

What of the author and flags.core.sourceId fields?

I will check which of those extra flags/fields (author) can be stripped and simplify there. From memory they are not needed.

[cirrahn]

@dodgepong here is another separate PR(!) with commits based on the above round of feedback: https://github.com/cirrahn/lancer-weapon-fx/pull/4

I stripped the extra flags/fields from the macros and there was no functional change when testing. The only "extra" field which had to stay was "author" as it is required. For "author", I put in an obvious generic ID (which is also generated by the tooling to create a new macro).

[dodgepong]

That looks excellent! Can you merge that branch into your feature/cirrahn-tweaks branch to update this PR? Then we can start looking at a final merge!

[cirrahn]

Commits now pushed to this branch!

[Z3nner]

First, I want to say that my lack of knowledge in this field is showing a lot lately. I don't exactly have a dev environment to take advantage of some of the backend work you put in and honestly 90% of what I do is on the fly changes and tweaks to tune timings live in the system. On the flip side, I've been working with sequencer enough that some of the issues that were raised were pretty straight forward fixes. Regardless, I'm excited for the update and have been working through the macros tweaking and tuning them as well as adding some new ones.

When it comes time for me to push these, I'm probably going to want my hand held to make sure its done correctly.

Here's what I have so far.

NEW MISSILE MACROS Added a few different macros to handle a bigger variety of missiles. dodgepong also made a Pinaka Missile macro that uses some pretty intense math to calculate two center points for any group of targets to direct the missile and explosion fx to. I also added a template to make placing a zone for the Pinaka missile simple, its a macro that dodgepong gave me awhile back, very simply drops a template that plays one of the zoning animations from JB2A.

MissileUpdateDemo.webm MissileUpdateDemo3.webm

NEW MISSILE AIRBURST and CANNONAIRBURST MACRO this was at the request of a user to handle the hurricane cluster projector from SSMR. I don't intend to add ALL the new 1st party weapons, but this one was too cool not to do, plus I had one that was 95% the way there anyway. Its the same as the second missile video above, but uses a big bullet animation instead of a missile. Travels 60% of the distance between the source and the target group, detonates, spawns a bunch of bullets that fly at the targets the attack Hit, then an explosion plays on each hit target.

APOCALYPSE RAIL Removed .missed function (Its cooler if it always explodes). Moved animations from target 0 to the average location for all targets.

RAILGUN Only one bullet animation, targeted at the furthest target from the source, plus one hit animation per hit target will play. Removed .missed function (Its a single bullet, travels in straight lines)

COMBAT DRILL Fixed animations, wind animation only plays on hit, fewer hit FX play on miss to simulate a sort of "glancing off"

AMR New prepatory sfx to introduce delay before it appears to fire verified timings

BOLT THROWER Explosion animation will always play and will do so at the bolt's destination. The explosion SFX will only play on a hit.

MISSILE swapped VFX from JK to JB2A, made missiles always explode even when missed, but explosion SFX only plays on hit.

DEFAULTMELEE changed from JB2A's divine smite to their new default melee and tweaked timings.

DD 288 introduced some random offset and tweaked timings.

HAMMER and PLASMA MAUL Moved ground cracking impact effect behind tokens, adjusted scale, offset of hammer, tweaked timings.

NANOBOT WHIP Tweaked timings, changed movement mode of animation to look better at longer range (its Threat 3)

NEEDLEBEAM Adjusted timings of primary and secondary shots

LASERS and PLASMARIFLE tweaked timings

I plan to continue working through the list you provided and some others I wanted to look at and then make another pass to remove and replace as many of the JK effects as possible.

I'm also thinking of changing the smoke grenade effect. I'm pretty sure its the only macro that uses Token Magic FX and JB2A has some alternatives that I'd like to try.

I think, if we can maintain the same fidelity with all the macros, that I'd like to remove both JK modules and TMFX from the dependencies.

[cirrahn]

Those missiles look amazing. I am excited to see the other updated FX too!

For pushing updates I am happy to help where needed. Since the release GitHub Action only triggers on "release published" I think it would be best to merge this PR (if you and @dodgepong are happy with it) then make changes on top of the updated main branch. The same would go for removing Token Magic FX and Animated Spell Effects.

[dodgepong]

I was hoping to do a final review this weekend but life seems to be happening to me instead. I agree that we should wait for this to be merged before submitting new animation changes. After this is merged, I'd like to add a couple more module API methods for macros to call to aid in targeting so that things like Pinaka won't result in huge blocks of code, especially since that kind of functionality might be duplicated across macros.

I have a few more thoughts related to that but they are out of scope with this PR so I'll save those for post-merge.

[dodgepong]

Alright, I checked out the latest changes and they all look great!

I'm going to go ahead and merge this. Thank you so much for these additions, they make things so much nicer for users and developers!

[cirrahn]

🎊

My pleasure. Thank you for all the reviews and bug-hunting!