fix: prevent duplicate cross-client defense dialog, clear bleed on heal

- Only send attackBoosted socket when attackerHandledBonus || attackerHasNonGMOwner (GM→player: hook handles it, no socket needed; PC→PC: socket needed) - Clear bleeding wounds when HP restored via token HUD heal buttons
2026-06-12 17:23:39 +02:00
parent cbeaaeec99
commit 2570bf707e
3 changed files with 148 additions and 22 deletions
@@ -0,0 +1,105 @@
+# Lethal Fantasy FoundryVTT System — Session Context
+
+## Current Goal
+Fix Grit/Luck defense reaction dialog UX (stacking dialogs, multiple clicks, revert on close) and cross-client sync of defense bonuses.
+
+## Accomplished
+
+### Pass 1 — Critical Issues
+- **Telemetry removed**: `ClassCounter`, `registerWorldCount`, orphaned `worldKey` setting deleted from system.json
+- **globalThis side effects**: `globalThis.SYSTEM`, `globalThis.pendingDefenses` moved from top-level to `init` hook
+- **console.log → log()**: All runtime console.log replaced with `log()` helper guarded by `lethalFantasy.debug` setting
+- **Stale Tenebris refs**: `macros.mjs` — `TENEBRIS.Label.jet` → `LETHALFANTASY.Label.jet`, `TENEBRIS.Manager.*` → `LETHALFANTASY.Label.*`, `tenebris.macro` flag → `lethalFantasy.macro`
+
+### Pass 2 — V1/V2 Mixing, Fire-and-Forget
+- **V1 sheet registrations removed**: `foundry.appv1.sheets.*` in system.json
+- **V1 `activateListeners`/jQuery**: removed dead `defaultOptions`, V1 tab code from `combat.mjs`
+- **V2 API paths**: `FilePicker` → V2, `TextEditor.getDragEventData` → V2, `item.sheet.render(true)` → `render({force:true})`, `super._onRender()` → `super._onRender(context, options)`, `token._id` → `token.id`
+- **Fire-and-forget Promises**: All `actor.update()`, `ChatMessage.create()`, `prepareRoll()`, `prepareMonsterRoll()`, socket handler calls now awaited
+- **Misnamed class**: `LethalFantasySkill` → `LethalFantasyWeapon`; added missing `WEAPON_TYPE` import; fixed `weaponCategory`
+
+### Pass 3 — Code Review Fixes
+- **Duplicated dialogs**: Per-element `.rollable`/`.wound-data` bindings moved to `_onRender` (V2 destroys/recreates DOM each render); `_activateListeners` reverted
+- **renderChatMessage reverted**: V2 hook `renderChatMessage` passes jQuery html, `querySelectorAll` fails; kept `renderChatMessageHTML`
+- **Roll actions broken**: Fixed `async` base-actor-sheet methods; `_onRender` bindings for rollable elements restored
+- **Token HUD guard**: `html.querySelector()` → `html.find().length` (html is jQuery object)
+- **All review awaits confirmed**: `showDefenseRequest`/`socket` handlers all awaited
+
+## Defense Dialog Investigation — Status
+
+### Symptom (user process)
+1. Monster (GM) attacks player — hits
+2. Player uses Grit/Luck to boost defense
+3. Defense now beats attack — reports new result
+4. Dialog **stays open** — Grit/Luck/bonus dice options still visible
+5. Closing dialog (Continue or X) causes "rolls vanish" — reverts to original result
+
+### Root Cause Found — Duplicate cross-client processing (FIXED)
+
+When monster (GM) attacks player, the `createChatMessage` hook fires on **both** clients:
+
+```
+Player's client:                         GM's client:
+  defense msg created                      defense msg synced
+  ↓                                        ↓
+  hook fires (line 557)                    hook fires (line 557)
+  isPrimaryController(defender)=true       isPrimaryController(defender)=false
+  ↓                                        ↓
+  Defense dialog A shows                   Defense dialog skipped
+  Player spends Grit                       Cross-client code (line 1009):
+  defenseRoll=10→16                        isPrimaryController(attacker)=true
+  While loop exits                         defenderOwner=player (≠GM)
+  Comparison: "miss"                       ↓
+                                           **Sends attackBoosted with ORIGINAL
+                                            defenseRoll=10 (stale!)**
+                                           ↓
+                                           Player receives socket → handleAttackBoosted
+                                           → Defense dialog B shows with OLD values
+                                           → When closed, comparison: "hit" (overwrites!)
+```
+
+Player sees **two** dialogs (A then B). Dialog B uses unboosted values, so closing/ignoring it produces a stale "hit" result that overwrites the correct "miss."
+
+### Fix
+`lethal-fantasy.mjs:1016` — only send `attackBoosted` socket when `attackerHandledBonus || attackerHasNonGMOwner`. Guards against stale-socket overwrite for GM→player combat (where hook-based processing works without socket), while preserving socket delegation for PC→PC cross-client (where `attackerIsCrossClient` suppresses the hook-based processing on the defender's client).
+
+Before:
+```js
+if (defenderOwner && defenderOwner.id !== game.user.id) {
+  game.socket.emit(`system.${SYSTEM.id}`, { type: "attackBoosted", ... })
+  return
+}
+```
+
+After:
+```js
+if (defenderOwner && defenderOwner.id !== game.user.id) {
+  if (attackerHandledBonus || attackerHasNonGMOwner) {
+    game.socket.emit(`system.${SYSTEM.id}`, { type: "attackBoosted", ... })
+  }
+  return
+}
+```
+
+### Same-Client Path
+Code pattern is identical between attack and defense dialogs — both use `await DialogV2.wait({rejectClose:false})` in a while loop. Same-client defense works correctly because no duplicate socket messages arrive.
+
+### Other Findings
+- `offerGritLuckBonus` (`utils.mjs:1121`) is dead code — never called
+- `promptCombatBonusDie` (`utils.mjs:975`) is correct — DialogV2 resolves to callback return value, not `action`
+- Cross-client `handleAttackBoosted` (`utils.mjs:291`) still uses `else if` chain without `continue` — functionally correct but differs from same-client pattern
+
+### Code Paths
+| Flow | File | Line |
+|------|------|------|
+| Same-client attack | `lethal-fantasy.mjs` | 918-1004 |
+| Same-client defense | `lethal-fantasy.mjs` | 697-870 |
+| Cross-client defense | `module/utils.mjs` | 291-445 |
+| Cross-client socket guard | `lethal-fantasy.mjs` | 1006-1037 |
+| Attack Grit offer | `module/utils.mjs` | 1210-1290 |
+
+### Key Files
+- `lethal-fantasy.mjs` — Main system hooks, same-client attack/defense reactions
+- `module/utils.mjs` — Cross-client defense flow, bonus dialogs, compareAttackDefense
+- `module/documents/actor.mjs` — `prepareRoll()` entry point
+- `module/documents/roll.mjs` — Roll resolution pipeline
@@ -1003,13 +1003,22 @@ Hooks.on("createChatMessage", async (message) => {
      }
    }

-    // Cross-client coordination: delegate the remaining reaction + message
-    // to the defender's controller via socket. Only the attacker's owning
-    // client sends — preventing duplicate emissions from other clients.
+    // Cross-client coordination: only delegate to the defender's client
+    // when the attacker boosted past the defense. When no attacker boost
+    // occurred, the defender's client already processed the defense via
+    // the createChatMessage hook and will create the correct comparison.
+    // Sending attackBoosted with stale (unboosted) values would cause
+    // the defender to see a duplicate dialog and overwrite the result.
    if (defender && isPrimaryController(attacker)) {
      const defenderOwner = game.users.find(u => u.active && !u.isGM && defender.testUserPermission(u, "OWNER"))
        || game.users.find(u => u.active && u.isGM)
      if (defenderOwner && defenderOwner.id !== game.user.id) {
+        // Send attackBoosted when the attacker actually boosted (so defender
+        // can respond to the new numbers), OR when the attacker has an active
+        // non-GM owner (PC-vs-PC cross-client) — the defender's hook-based
+        // processing is suppressed by attackerIsCrossClient, so the socket
+        // handler must show the defense dialog instead.
+        if (attackerHandledBonus || attackerHasNonGMOwner) {
          const sData = LethalFantasyUtils.getShieldReactionData(defender)
          game.socket.emit(`system.${SYSTEM.id}`, {
            type: "attackBoosted",
@@ -1028,6 +1037,7 @@ Hooks.on("createChatMessage", async (message) => {
            shieldDr: sData?.damageReduction || 0,
            canAdHocShield: !sData,
          })
+        }
        return
      }
      // Same client: restart for defender loop if attacker boosted past defense
@@ -29,7 +29,7 @@ export default class LethalFantasyUtils {
  static setHookListeners() {

    Hooks.on('renderTokenHUD', async (hud, html, token) => {
-      if (html.querySelector(".lethal-hp-loss-hud")) return
+      if (html.find(".lethal-hp-loss-hud").length) return
      // HP Loss Button (existing)
      const lossHPButton = await foundry.applications.handlebars.renderTemplate('systems/fvtt-lethal-fantasy/templates/loss-hp-hud.hbs', {})
      $(html).find('div.left').append(lossHPButton);
@@ -99,6 +99,17 @@ export default class LethalFantasyUtils {
          log(tokenFull, token)
          let actor = tokenFull.actor;
          await actor.applyDamage(Number(hpGain)); // Positive value to add HP
+          // Clear bleeding wounds on heal — regardless of heal amount, any
+          // healing is enough to stop bleeding (field dressing / magic / rest).
+          const wounds = foundry.utils.duplicate(actor.system.hp.wounds || [])
+          const hadBleeding = wounds.some(w => w.description === "Bleeding")
+          if (hadBleeding) {
+            await actor.update({
+              "system.hp.wounds": wounds.map(w =>
+                w.description === "Bleeding" ? { value: 0, duration: 0 } : w
+              )
+            })
+          }
          $(html).find('.hp-gain-wrap')[0].classList.remove('hp-gain-hud-active');
          $(html).find('.hp-gain-wrap')[0].classList.add('hp-gain-hud-disabled');
          $(html).find('.hp-gain-wrap')[1].classList.remove('hp-gain-hud-active');