pf2e icon indicating copy to clipboard operation
pf2e copied to clipboard

Expose aura logic for other grid type support

Open FolkvangrForgent opened this issue 1 year ago • 3 comments

Description

This MR serves to expose the aura related classes and make a minor change. This will allow modules such as pf2e-hex to add support for additional grid types to the systems aura automation.

Changes

  • Move grid type check from checkAuras function to containsToken. This was done to make it so that modules extending the behavior only need to override containsToken.
  • Expose AuraRenderers, AuraRenderer, TokenAura, and EffectAreaSquare classes via CONFIG.PF2E to allow modules to access them to wrap functions.

FolkvangrForgent avatar Sep 15 '24 20:09 FolkvangrForgent

This almost looks fine to me, though I wonder if those classes are exposed if we should not construct instances from the config instead of importing directly in our own uses of it, to discourage monkey patching and encourage subclassing. But this is something that @stwlam is more familiar with.

CarlosFdez avatar Sep 20 '24 20:09 CarlosFdez

Are other things in CONFIG setup to use subclassing? I can certainly look into changing it to a subclassing friendly implementation if that is the design the project is aiming for. I also just want to note the AuraRenderers, AuraRenderer, and TokenAura are the only things I am monkey patching. EffectAreaSquare is just exposed so I can create instances to use within my monkey patching.

FolkvangrForgent avatar Sep 22 '24 19:09 FolkvangrForgent

Hi Just wondering if you've had a chance to take a look at this MR @stwlam . I've been using a build with this in my home games and haven't run into any issues with the current implementation.

FolkvangrForgent avatar Oct 11 '24 01:10 FolkvangrForgent

So far I don''t think stwlam is amenable to exposing any aura logic at all, so I don't expect this will ever be merged.

You specifically just need some way to determine "is in aura"? Its kinda bad code design wise, but the best thing I can think of as a compromise is moving containsToken() to a check in token for isInAura(), as tokens are foundry api and thus overridable.

CarlosFdez avatar Nov 03 '24 23:11 CarlosFdez

Hi CarlosFdez, If the system does not want to expose the aura objects then the token object having a function would be better than nothing. It would prevent me from piggybacking off the existing rendering calls but I could always try to implement my own render. I am curious what the hesitation for exposing the aura logic would be; I would certainly not be expecting a stable api or anything of that sort from it's exposure. I'd also love to add hex (and maybe even gridless) support directly to the aura logic in the pf2e system if this is something the system would be amenable to. Although currently I would want fvtt to expose some functions so I don't have duplicate versions of fvtt functions: https://github.com/foundryvtt/foundryvtt/issues/11795 .

FolkvangrForgent avatar Nov 07 '24 01:11 FolkvangrForgent

@stwlam Could this be reconsidered? The latest position on auras in gridless/hexes was "paizo didn't make firm rules, so the pf2e system won't try to support them, but a module could override this" -- and in cases like this, modules require the system to add some API support without increasing the maintenance load for the system devs.

shemetz avatar Jan 04 '25 09:01 shemetz

There is a module now that let's you start macros with hex-grid supported auras: https://github.com/Wibble199/FoundryVTT-Grid-Aware-Auras

SilverLous avatar May 08 '25 11:05 SilverLous