Merge pull request #1660 from jramiresbrito/fix/gen2-exp-award-announce

docs/mod-api-gen2-compat.md

@@ -550,6 +550,17 @@ gains a field instead of the name gaining a prefix.
   screen has no `.data` field, so the Gen 2 site **adds** `ctx.data` beside the
   Gen 1 keys. A mod that calls `nextFn` is unaffected; one that reaches through
   `ctx.battle.data` instead gets nil on Gold.
+  `battle.exp_award`'s `ctx.applyShare(mon, split, announce)` reads its third
+  argument on both generations: truthy prints the mon's GainedText, falsy pays
+  it silently, so one mod source can print a single summary line for a
+  party-wide award instead of a box per recipient. Gold honours it **only when
+  it is passed**, by argument count -- `applyShare(mon, split)` was written
+  against a seam that always announced on Gold and keeps announcing there,
+  while `applyShare(mon, split, nil)` is silent on both. Pass the argument
+  explicitly and the two generations agree; omit it and Gen 1 stays silent
+  where Gold speaks. Only the line is affected: the exp, the stat exp,
+  `battle.exp_gained`, the level-up line, learned moves and the forget prompt
+  happen either way.
 - *The catch and the evolution:* `pokemon.caught`, `pokemon.evolved`; hook
   `evolution.check`. `src/ui/gen2/BattleState.lua:pushCaught` emits
   `pokemon.caught` once the mon is in the party or the box, and

docs/rfcs/0012-gen2-exp-award-announce.md

@@ -0,0 +1,140 @@
+# RFC 0012: `applyShare`'s announce argument on Gen 2
+
+## Status
+
+Proposed.
+
+## Motivation
+
+`battle.exp_award` hands a mod `ctx.applyShare(mon, split, announce)` on both
+generations. On Gen 1 the third argument decides whether the mon's GainedText
+box is printed (`src/battle/BattleState.lua`, `if announce then`), which is how
+a mod that pays the whole party prints **one** summary line instead of a box
+per recipient.
+
+Gold accepts the argument and ignores it, as its own comment above the hook
+call says. So the same mod source, running the same code, prints one line on
+Red and six on Gold — one for the participant plus one for every bench mon it
+paid.
+
+This is not hypothetical. The [Exp Share](https://github.com/ShaneMcGovernIE/exp_share)
+mod declares `"games": ["gen1", "gen2"]` and its description promises "a single
+shared-exp line instead of one message per Pokemon". It passes `true` for the
+fighters and `nil` for the bench, exactly as the Gen 1 seam asks. On Gold every
+one of those `nil` calls announces anyway, so a five-mon party turns every KO
+into six boxes to click through.
+
+There is no mod-side fix. The announcement is emitted inside
+`Battle:giveExperiencePass`, behind no hook, and a mod cannot ask for silence
+because the argument that means "quietly" is discarded. The only workaround is
+to intercept the battle's event queue afterwards and delete the boxes, which is
+what a mod written for this had to do — a mod reaching into engine internals to
+undo something the public seam should never have done.
+
+## Decision and plan extended
+
+This does not add a seam. It finishes one: `battle.exp_award` is documented as
+"the same hook `BattleState:awardExp` calls on Gen 1 and with the same ctx",
+and `docs/mod-api-gen2-compat.md` lists it among the hooks shared with Gen 1.
+The third `applyShare` argument is the one part of that ctx whose meaning did
+not survive the crossing, so the promise the catalog already makes is what this
+change delivers.
+
+The delta follows Route B's additive, guarded convention in
+`CONTRIBUTING-mods.md`: nothing is renamed, nothing is removed, and no mod that
+exists today changes behaviour.
+
+## Exact API delta
+
+`ctx.applyShare(mon, split, announce)` on Gen 2 now reads `announce`:
+
+| Call | Gen 1 | Gen 2 before | Gen 2 after |
+|---|---|---|---|
+| `applyShare(mon, split)` | silent | announces | **announces** (unchanged) |
+| `applyShare(mon, split, nil)` | silent | announces | **silent** |
+| `applyShare(mon, split, false)` | silent | announces | **silent** |
+| `applyShare(mon, split, true)` | announces | announces | announces |
+| `applyShare(mon, split, "expAll")` | announces | announces | announces |
+
+The argument is honoured **only when it is actually passed**, decided by
+argument count rather than by value:
+
+```lua
+local function applyShare(mon, split, ...)
+  local announce = ...
+  local silent = select("#", ...) > 0 and not announce
+  ...
+end
+```
+
+`select("#", ...)` counts an explicit `nil`, so `applyShare(mon, split)` and
+`applyShare(mon, split, nil)` are distinguishable — and they have to be, because
+the first is a Gen 2-era call written against a seam that always announced, and
+the second is a deliberate "pay this one quietly".
+
+Only the `{ kind = "experience" }` event is affected. A silent award is still a
+whole award: the exp, the stat exp, the `battle.exp_gained` event, the
+`grew to level` line, learned moves and the interactive forget-a-move prompt all
+happen exactly as before, in the same order.
+
+Internally `Battle:giveExperiencePass` takes a sixth parameter, `silent`. It
+defaults to announcing, so both of the cart's own passes are untouched.
+
+## Migration and compatibility
+
+**Existing mods change nothing.** A Gen 2 mod calling `applyShare(mon, split)`
+gets the behaviour it was written against. A Gen 1 mod is untouched: no Gen 1
+file is modified. The v1 surface — `content.X:register/override/get`,
+`events:on`, `hooks:wrap`, `mod.log`, `mod:read`, the manifest v1 fields and
+`pokemon.before_give` — is not involved; `mods/example_mew_starter` neither
+calls this seam nor loads differently.
+
+A mod that wants parity passes the argument explicitly, which is what the Gen 1
+seam has always documented. Exp Share already does, and needs no edit to get
+its own README's behaviour on Gold.
+
+One residual difference is deliberate and now documented rather than silent: an
+**omitted** third argument still means "silent" on Gen 1 and "announce" on
+Gold. Closing that would change what an existing Gen 2 mod prints, which Route
+B rejects. Passing the argument makes the two generations agree, so the rule an
+author needs is one sentence: *say what you mean and both games do the same
+thing.*
+
+## Verification
+
+- `tests/gen2_exp_share_test.lua` grows two sections and 14 checks, and the 23
+  checks it already had are unchanged — which is itself the vanilla-parity
+  evidence for this file.
+  - **The no-mod test.** With nothing subscribed to `battle.exp_award`, a solo
+    participant still prints one line and the EXP.SHARE double pass still
+    prints both. The hot path is unchanged: `Runtime.wantsHook` still guards
+    the ctx allocation, and `vanillaAward` never passes `silent`.
+  - **The mod-API test.** The seam is driven through `hooks:wrap` on a real
+    `Runtime.install`ed bus, not by calling internals: the omitted argument
+    announces, an explicit `nil` and an explicit `false` are silent, a truthy
+    value (including Gen 1's `"expAll"`) announces, the exp and stat exp paid
+    are identical either way, and no `experience` event leaks into the queue on
+    a silent pass.
+- `tests/engine/gate_gen2_mod_api.lua` (943 checks),
+  `tests/engine/gate_hooks.lua` (493) and `tests/engine/gate_events.lua` (529)
+  pass unchanged; `battle.exp_award` was already in the shared catalog, so no
+  gate list moves.
+- `tests/gen2_battle_test.lua` (690), `gen2_battle_end_test.lua` (32),
+  `gen2_battle_items_test.lua` (99), `gen2_badge_boosts_test.lua` (34) and
+  `gen2_battle_loss_test.lua` (15) pass unchanged.
+
+## Docs with the change
+
+`docs/mod-api-gen2-compat.md` gains the `applyShare` reading beside the
+existing `battle.low_health_alarm` payload note, in the same section that lists
+`battle.exp_award` as shared — including the argument-count rule and the
+residual difference above.
+
+No registry or schema field changes, so `src/mods/Schemas.lua` is untouched and
+`tools/gen_registry_docs.lua` has nothing new to emit.
+
+## Deprecation etiquette
+
+Nothing is removed, renamed, superseded or deprecated. The two-argument call is
+not deprecated either — it keeps its current Gen 2 meaning permanently, and the
+docs name the explicit form as the one that behaves the same on both games.

src/battle/gen2/Battle.lua

@@ -3368,7 +3368,16 @@ end
 -- `count` is the pass's own divisor -- the participant count for the first
 -- pass, the holder count for the EXP.SHARE pass -- and `halved` is whether
 -- any Share holder taxed the whole pool.
-function Battle:giveExperiencePass(loser, def, recipients, count, halved)
+--
+-- `silent` suppresses only the GainedText line.  It exists for the
+-- battle.exp_award seam below, where a mod paying the bench wants one summary
+-- line rather than a box per mon; the cart's own two passes never pass it, so
+-- vanilla prints exactly what it always did.  Everything else about the pass
+-- -- the exp, the stat exp, battle.exp_gained, "grew to level", learned moves
+-- and the forget prompt -- is unaffected, because a silent award is still an
+-- award.
+function Battle:giveExperiencePass(loser, def, recipients, count, halved,
+                                   silent)
   for _, index in ipairs(recipients) do
     local mon = self.party[index]
     if mon and (mon.hp or 0) > 0 and not mon.isEgg then
@@ -3419,10 +3428,12 @@ function Battle:giveExperiencePass(loser, def, recipients, count, halved)
           index = index,
         })
       end
+      if not silent then
         self:emit({ kind = "experience", index = index, amount = amount,
           -- BoostedExpPointsText, keyed on the traded arm alone.
           text = self:monName(mon) .. " gained "
             .. (traded and "a boosted " or "") .. amount .. " EXP. Points!" })
+      end
       if result.levels > 0 then
         -- "level up happiness mod", the cart's own comment, sitting right
         -- after the stat recalc and before the "grew to level" text.  It fires
@@ -3518,22 +3529,38 @@ function Battle:awardExperience(loser)
 
   -- battle.exp_award, the same hook BattleState:awardExp calls on Gen 1 and
   -- with the same ctx: the participant COUNT, the live participants, and an
-  -- applyShare(mon, split) a mod can call to pay one mon its own share.  The
-  -- third applyShare argument is Gen 1's EXP.ALL announcement variant; Gen 2
-  -- has no EXP.ALL (the EXP.SHARE pass below is its replacement), so it is
-  -- accepted and ignored rather than changing what is printed.  `recipients`,
-  -- `holders` and `halved` are the Gen 2 additions.
+  -- applyShare(mon, split, announce) a mod can call to pay one mon its own
+  -- share.  `recipients`, `holders` and `halved` are the Gen 2 additions.
+  --
+  -- `announce` is Gen 1's third argument (src/battle/BattleState.lua
+  -- applyShare) and means the same thing here: truthy prints the mon's
+  -- GainedText, falsy pays it silently.  That is what lets one mod source
+  -- print ONE summary line for a party-wide award on both generations instead
+  -- of a box per mon -- which is what the Exp Share mod documents and could
+  -- not do on Gold, because this argument used to be accepted and ignored.
+  --
+  -- It is honoured only when it is actually PASSED, by argument count rather
+  -- than by value.  A Gen 2-era mod calling applyShare(mon, split) was written
+  -- against a seam that always announced and keeps announcing; a caller that
+  -- passes the argument -- including an explicit nil, which is what a "pay
+  -- this one quietly" call looks like -- gets Gen 1's reading.  So no existing
+  -- mod changes behaviour, and a mod that opts in gets parity.
   if Runtime.wantsHook("battle.exp_award") then
     local alive = {}
     for _, index in ipairs(participants) do
       local mon = self.party[index]
       if mon and (mon.hp or 0) > 0 then alive[#alive + 1] = mon end
     end
-    local function applyShare(mon, split)
+    local function applyShare(mon, split, ...)
+      local announce = ...
+      -- select("#") counts an explicit nil; `announce == nil` alone could not
+      -- tell applyShare(mon, split) from applyShare(mon, split, nil), and
+      -- those two have to mean different things here.
+      local silent = select("#", ...) > 0 and not announce
       for index, candidate in ipairs(self.party) do
         if candidate == mon then
           return self:giveExperiencePass(loser, def, { index },
-            math.max(1, split or 1), halved)
+            math.max(1, split or 1), halved, silent)
         end
       end
     end

tests/gen2_exp_share_test.lua

@@ -213,4 +213,115 @@ do
     "all four arms compose in the cart's order")
 end
 
+-- ---- battle.exp_award: applyShare's third argument ------------------------
+--
+-- Gen 1's applyShare(mon, split, announce) pays the mon and prints its
+-- GainedText only when `announce` is truthy (src/battle/BattleState.lua).
+-- Gold accepted the argument and ignored it, so a mod that pays the whole
+-- party -- the Exp Share mod's own documented behaviour -- could not print one
+-- summary line here; it got a box per recipient.
+--
+-- Two tests, per Route B in CONTRIBUTING-mods.md: vanilla is unchanged with no
+-- subscriber, and the seam is driven through the public hook API.
+
+local Events = require("src.mods.Events")
+local Hooks = require("src.mods.Hooks")
+local Runtime = require("src.mods.Runtime")
+
+-- Counts the GainedText boxes an award produced.
+local function expLines(events)
+  local out = {}
+  for _, event in ipairs(events) do
+    if event.kind == "experience" then out[#out + 1] = event.text end
+  end
+  return out
+end
+
+local function kindsOf(events)
+  local out = {}
+  for _, event in ipairs(events) do out[#out + 1] = event.kind end
+  return table.concat(out, ",")
+end
+
+-- ---- the no-mod test: nothing subscribed, nothing changes -----------------
+do
+  local _, events = award({ { otId = PLAYER_ID, participant = true } })
+  eq(#expLines(events), 1, "no subscriber: a solo participant still prints one line")
+
+  local _, twoPass = award({
+    { otId = PLAYER_ID, participant = true },
+    { otId = PLAYER_ID, item = "EXP_SHARE" },
+  })
+  eq(#expLines(twoPass), 2,
+    "no subscriber: the EXP.SHARE double pass still prints both lines")
+end
+
+-- ---- the mod-API test: driven through hooks:wrap, not internals -----------
+do
+  local savedEvents, savedHooks = Runtime.events, Runtime.hooks
+  local hooks = Hooks.new()
+  Runtime.install(Events.new(), hooks)
+
+  -- Each case pays a two-mon party through ctx.applyShare and reports the
+  -- GainedText boxes that survived.  The party mon in slot 2 never fights, so
+  -- it stands in for the bench a party-wide mod pays.
+  local function awardVia(payer)
+    local unsub = hooks:wrap("battle.exp_award", function(_nextFn, ctx)
+      payer(ctx)
+    end)
+    local gained, events, party = award({
+      { otId = PLAYER_ID, participant = true },
+      { otId = PLAYER_ID },
+    })
+    unsub()
+    return gained, events, party
+  end
+
+  -- omitted: the Gen 2-era call, which has always announced
+  local gained, events = awardVia(function(ctx)
+    ctx.applyShare(ctx.battle.party[1], 1)
+    ctx.applyShare(ctx.battle.party[2], 1)
+  end)
+  eq(#expLines(events), 2,
+    "applyShare(mon, split) still announces -- no existing mod changes")
+  check(gained[1] > 0 and gained[2] > 0, "and both mons were paid")
+
+  -- passed nil: "pay this one quietly", Gen 1's reading
+  local quietGained, quietEvents = awardVia(function(ctx)
+    ctx.applyShare(ctx.battle.party[1], 1, true)
+    ctx.applyShare(ctx.battle.party[2], 1, nil)
+  end)
+  eq(#expLines(quietEvents), 1,
+    "an explicit nil third argument pays the mon silently")
+  eq(expLines(quietEvents)[1], "MACHOP gained 110 EXP. Points!",
+    "and the announced participant keeps its own line")
+  check(quietGained[2] > 0, "the silent mon is still paid the same exp")
+  eq(quietGained[1], gained[1], "and the announced mon's exp is untouched")
+  eq(quietGained[2], gained[2], "as is the silent one's")
+
+  -- false reads the same as nil; a truthy string is Gen 1's EXP.ALL variant
+  local _, falseEvents = awardVia(function(ctx)
+    ctx.applyShare(ctx.battle.party[1], 1, false)
+  end)
+  eq(#expLines(falseEvents), 0, "false is silent too")
+
+  local _, expAllEvents = awardVia(function(ctx)
+    ctx.applyShare(ctx.battle.party[1], 1, "expAll")
+  end)
+  eq(#expLines(expAllEvents), 1,
+    "any truthy value announces, so Gen 1's \"expAll\" carries over")
+
+  -- a silent award is still a whole award: only the line goes
+  local _, levelEvents, levelParty = awardVia(function(ctx)
+    ctx.applyShare(ctx.battle.party[2], 1, nil)
+  end)
+  eq(#expLines(levelEvents), 0, "the silent pass prints no GainedText")
+  check(levelParty[2].statExp.attack > 0,
+    "but stat exp is still awarded on a silent pass")
+  check(not kindsOf(levelEvents):find("experience", 1, true),
+    "and no experience event leaks into the queue")
+
+  Runtime.install(savedEvents, savedHooks)
+end
+
 S.finish()