foundryvtt-lancer icon indicating copy to clipboard operation
foundryvtt-lancer copied to clipboard

[REFACTOR] Macro Cleanup and De-duplication

Open mason-bially opened this issue 3 years ago • 5 comments

This is a tracking issue for macro cleanup and de-duplication that can be done to improve macro code readability and understand-ability.

As an example of what this is aimed to fix:

  • prepareActivationMacro dispatches from chips
    • for systems, is partially responsible for handling charges
    • for most things this is just displaying text
  • prepareItemMacro dispatches from the item sheets
    • does not handle charges at all
    • for systems invokes rollSystemMacro
      • does not handle charges
      • dispatches into displaying text
    • failure to dispatch goes to displaying text

Related issues:

  • #371 An issue for tracking macro renames and their depreciation.
  • #370 what originally kicked this off, rearranging the macros into files for improved readability.
  • #357 deals with activation chips for mechs.
  • Also need to deal with activation chips for NPCs.

mason-bially avatar Jan 09 '22 06:01 mason-bially

I think most of the *MacroData interfaces could be moved into their relevant macro files. Most of them are never invoked outside of their prepare*Macro / roll*Macro pairs, and when they are those pairs are being imported anyway.

mason-bially avatar Jan 09 '22 06:01 mason-bially

Okay so NPCs don't have activation chips. They have tags that say "quick action" (and tags can be dragged, though we should probably reject putting them on the hot bar).

mason-bially avatar Jan 09 '22 23:01 mason-bially

So currently the prepareItemMacro leads to some weird behavior on the hot bar. Consider this scenario:

  • I drag an item T from actor A's sheet to the hot bar.
  • Scenario: I click the hotbar with actor A selected
    • Outcome: Actor A performs the action.
  • Scenario: I click the hotbar with actor B selected, who also has the item.
    • Outcome: Actor B performs the action.
  • Scenario: I click the hotbar with actor C selected, who does not have the item.
    • Outcome: Actor A performs the action.
  • Scenario: I click the hotbar with no actor selected.
    • Outcome: Actor A performs the action.

I think the last two cases are in need of massaging if the second is going to be an intended feature.

It's possible this is a scenario where automation settings might be useful in determining the outcome (re: automation feature off, always actor A, automation feature on, 4 different outcomes (of which 2 I imagine would be error messages)).

mason-bially avatar Jan 10 '22 00:01 mason-bially

So where I am thinking is limiting the space of public macros to this:

  • prepareActivationMacro for activation chips and calling specific actions on things
  • prepareItemMacro for item references directly
    • though I may want to interdict this / rename the macro for things like traits and talents which have to index into an "item" to display properly (which, btw, we need a cleaner interface for). I'm thinking prepareDisplayMacro
      • this is also what dragging the little "message" button will make as opposed to the "item box" (which is also used for building sheet content)
    • the reason for separating this is that "display" is a fixed kind of macro where as "item" is a more general contextual macro.
  • prepareActionMacro for more general triggering of action economy and flow
    • will encompass a number of things like the current overcharge, attack, and tech attack macros but with action economy support and automation
    • should also be the main call into the stabilize macro.
  • prepareStatMacro should probably remain (oh hey, contested rolls at some point) with perhaps a generic prepareBasicAttackMacro that can be clicked as well to perform attacks (melee/ranged/tech).
    • the point of these is to skip the action macro manipulations for various special systems.

In theory these should be all the public facing macros that would be required (e.g. generated hot bar macros, custom user macros, and token action HUD), besides some for like "full repair" and "short rest".

From there letting users access various pieces of the internal macro functionality (stabilize, take structure, take stress) can be more "internal and subject to change".

mason-bially avatar Jan 10 '22 00:01 mason-bially

Work left to do since PR #375 got merged:

  • Deal with structured items better when drag and dropped. This is mostly talents (rank 1-3), core powers (subset of frames, and a mess of passive, actives, and other things), frame passives (an even worse mess), and probably weapons with profiles.
    • Will probably necessitate refactoring the interfaces so that these share a structure.
  • Separate out the four possible "intents" when dragging and dropping. These are:
    • Activation Macro - do turn accounting properly and run the item as if done as part of an action, e.g. if I clicked the [quick] activation chip. Created by dragging the chip.
      • This is complicated in the context of things like weapons (skirmish or barrage; is this an aux weapon firing; am I firing this weapon via another ability?). Thankfully these don't have activation chips at the moment.
    • "DO IT" Macro - where we execute the action regardless of turn accounting. This is how tech attacks and weapon attacks work at the moment. This is also the intent of "item" macros. Effectively this is the internal macro. Created by dragging a dice.
      • Systems without macros don't have this (yet), but we should still separate out this case anyway. It's the difference between "display a system" and "a system was performed". One difference may be highlighting certain keywords with buttons.
    • Display Macro - where we display the content of the thing, the little chat bubble, created by dragging it. This shouldn't do any automation regardless of mode (and no/few buttons), and should probably be noted differently in the chat log.
    • Contextual Macro - This one is the trickiest. Basically it should do any of the above depending on context. Created by dragging an item. Some thoughts:
      • If it's your turn this should do activation macro, if not display macro (unless it's a reaction).
      • If you are in the middle of an attack action, and this is a weapon, it should read it as "attack with this weapon+mount" and do the internal "DO IT" macro. If you aren't it should prompt you "do you want to skirmish/barrage/do this as part of some other action".

mason-bially avatar Apr 20 '22 01:04 mason-bially