Larkinabout / fvtt-token-action-hud-core

Token Action HUD is a repositionable HUD of actions for a selected token.
14 stars 21 forks source link

[BUG] Documentation Issues #52

Closed farling42 closed 1 year ago

farling42 commented 1 year ago
farling42 commented 1 year ago

Also the description of using tokenActionHudSystemReady might be better by describing it as:

Larkinabout commented 1 year ago
  • The document for doRenderItem doesn't mention that the action's name must be "itemMacro" in order for this to work.

All good, I think, apart from this quoted one, which I don't understand. "itemMacro" is used in reference to the Item Macro module when building and handling its actions, but shouldn't go anywhere near the doRenderItem method. doRenderItem uses the item object to render its sheet.

farling42 commented 1 year ago
  • The document for doRenderItem doesn't mention that the action's name must be "itemMacro" in order for this to work.

All good, I think, apart from this quoted one, which I don't understand. "itemMacro" is used in reference to the Item Macro module when building and handling its actions, but shouldn't go anywhere near the doRenderItem method. doRenderItem uses the item object to render its sheet.

That comment is based on looking at the code in HUD Core. It checks for a value of "itemMacro" before calling doRenderItem (in the PreRollHandler class?). Specifically at https://github.com/Larkinabout/fvtt-token-action-hud-core/blob/1562fad87201c365d2f88830d98d46311f6067d6/scripts/roll-handlers/pre-item-macro.js#L17

I assumed that was code that would trigger doRenderItem from HUD Core, rather than all modules having to manually code if (this.isRenderItem()) this.doRenderItem(actorId, tokenId, actionId)

Larkinabout commented 1 year ago

Ah, okay, that pre-roll handler is just for handling the Item Macro actions.

I'm not sure if there would be a good way of handling rendering of the item sheet in TAH Core as the encodedValue is defined by the system module and could be in any format.

Larkinabout commented 1 year ago

Think these are all resolved now.