Merge branch 'dev' of https://github.com/bryanthaboi/gen1recomp into dev

docs/mod-api-gen2-compat.md

@@ -433,8 +433,13 @@ differences an author meets:
 - **`encounters`.** The id is the encounter *kind*, not the map:
   `mod.content.encounters:patch("grass", { ROUTE_29 = { rates = { NITE = 40 } } })`.
   A map's row carries a `rates` set per time of day and one slot list.
-  `fishGroups`, `trees` / `treeSets`, `rocks`, `bugContest` and `roamMaps` are
-  ids of their own.
+  `fishGroups`, `trees` / `treeSets`, `rocks`, `bugContest`, `roamMaps` and
+  `roamMons` are ids of their own. This is the one registry whose ids are a
+  **closed set** -- the kinds above are exactly the lookups the engine makes
+  into `Data.gen2Encounters`, so an id outside the set would be a write
+  nothing reads. Patching an unknown id (a Gen 1 encounters mod ported
+  unchanged passes the *map* here) fails the mod and names the ids that do
+  exist, rather than accepting the call and leaving the game vanilla.
 - **`trainers`.** The id is the trainer *class*, and the record is
   `{ name, index, attributes, baseMoney, encounterMusic, trainers, items }`,
   with one entry per named trainer of the class. The registry writes one level

docs/modding/reference/registries.md

@@ -366,8 +366,10 @@ mod.content.encounters:patch("ROUTE_1", { grass = { rate = 30 } })
 The record differs; the registry name, the verbs and the id space
 do not.
 
-Id = a top-level key of the target table. Keys not listed here are
-accepted and merged as-is.
+Id = a top-level key of the target table. The set below is **closed**:
+an id that is not one of these is rejected rather than merged, because
+the engine reads this table by name and a key it does not name is a
+write nothing reads.
 
 | key | type |
 |---|---|
@@ -376,6 +378,7 @@ accepted and merged as-is.
 | `generation` | integer >= 1 |
 | `grass` | map of string -> {map?, rates, slots} |
 | `roamMaps` | list of {map, to} |
+| `roamMons` | list of {level?, map?, mapGroup?, mapNumber?, species?} |
 | `rocks` | map of string -> string |
 | `source` | string |
 | `swarmGrass` | map of string -> {map?, rates, slots} |
@@ -391,6 +394,12 @@ accepted and merged as-is.
 mod.content.encounters:patch("grass", { ROUTE_29 = { rates = { NITE = 40 } } })
 ```
 
+Gold's encounter ids are a **closed set**: the kinds above are
+the complete set of lookups the engine makes into `Data.gen2Encounters`, so an
+id that is not one of them is a write nothing reads. An id outside the set is
+rejected -- a Gen 1 mod ported unchanged passes the map where Gold wants the
+kind, and that call is refused rather than silently dropped.
+
 ## evolution_methods
 
 - semantics: `record`

src/battle/gen2/Battle.lua

@@ -2278,6 +2278,13 @@ end
 -- additions.  The ctx table is built only when a chain is installed.
 function Battle:accuracyRoll(def, attacker, defender, accuracy)
   accuracy = accuracy or (def and def.accuracy)
+  -- Weather-based accuracy overrides (e.g. Thunder: 100 in rain, 50 in sun).
+  -- Stage modifiers and Bright Powder still apply on top; see Effects.lua.
+  -- BattleCommand_CheckHit (engine/battle/effect_commands.asm): weather can
+  -- override a move's base accuracy; see Effects.WEATHER_ACCURACY_OVERRIDES.
+  local weatherOverride = Effects.weatherAccuracyOverride(self.weather,
+    def and def.effect)
+  if weatherOverride then accuracy = weatherOverride end
   if Runtime.wantsHook("battle.accuracy") then
     return Runtime.call("battle.accuracy", function(c)
       return c.battle:vanillaAccuracyRoll(c.accuracy, c.user, c.target)

src/battle/gen2/Effects.lua

@@ -512,6 +512,19 @@ function Effects.weatherModifier(weather, moveType, effect)
   return 1
 end
 
+-- BattleCommand_CheckHit (engine/battle/effect_commands.asm): Thunder's base
+-- accuracy is overridden by weather before stage modifiers are applied.
+Effects.WEATHER_ACCURACY_OVERRIDES = {
+  rain = { EFFECT_THUNDER = 100 },
+  sun  = { EFFECT_THUNDER = 50  },
+}
+
+function Effects.weatherAccuracyOverride(weather, effect)
+  if not weather or not effect then return nil end
+  local row = Effects.WEATHER_ACCURACY_OVERRIDES[weather]
+  return row and row[effect]
+end
+
 -- HandleWeather's .SandstormDamage: an eighth of max HP, and Rock, Ground and
 -- Steel are immune.  A mon underground (Dig) is skipped too.
 Effects.SANDSTORM_IMMUNE = { ROCK = true, GROUND = true, STEEL = true }

src/mods/Schemas.lua

@@ -273,8 +273,24 @@ function Schemas.check(spec, registryName, id, value, mode, generation)
     -- deep registries are open namespaces: a key the catalog does not
     -- describe is a mod's own data, not a mistake.  keyValue types every
     -- key alike, for namespaces whose keys are content (one per map).
+    --
+    -- `keysClosed` is the opt-out, for the registries where an unknown id
+    -- cannot be a mod's own data because NOTHING reads it: Gold's encounters
+    -- table is consumed by a fixed set of lookups (encounters.grass,
+    -- encounters.water, ...), so a key the catalog does not describe is a
+    -- write that lands nowhere and does nothing.  That silence is the whole
+    -- bug in #2369 -- a Gen 1 author patching "ROUTE_29" where Gold wants
+    -- the encounter KIND, "grass" -- so the ids are named back instead.
     local keyType = (spec.keys and spec.keys[id]) or spec.keyValue
-    if keyType then checkValue(keyType, value, path, patchMode, errors, true) end
+    if keyType then
+      checkValue(keyType, value, path, patchMode, errors, true)
+    elseif spec.keysClosed then
+      local names = {}
+      for keyName in pairs(spec.keys) do names[#names + 1] = keyName end
+      table.sort(names)
+      errors[#errors + 1] = ("%s: unknown id; this registry's ids are %s")
+        :format(path, table.concat(names, ", "))
+    end
   elseif spec.value then
     checkValue(spec.value, value, path, patchMode, errors, true)
     if #errors == 0 and not patchMode and spec.extra then
@@ -700,10 +716,10 @@ end
 --
 -- So beside `value` / `fields` / `keys` / `keyValue` a spec may carry
 -- `gen2Value` / `gen2Fields` / `gen2Keys` / `gen2KeyValue`, and beside
--- `semantics` / `extra` / `write` / `baseAt` / `baseIds` / `reservedIds` /
--- `example` / `notes` the matching `gen2*`.  Absent means "the Gen 1 shape is
--- right here too", which is the common case and why most registries carry
--- none of this.
+-- `semantics` / `extra` / `keysClosed` / `write` / `baseAt` / `baseIds` /
+-- `reservedIds` / `example` / `notes` the matching `gen2*`.  Absent means
+-- "the Gen 1 shape is right here too", which is the common case and why most
+-- registries carry none of this.
 -- The registry NAME, the verbs and (wherever the id space allows it) the ids
 -- stay shared, exactly as the routing table keeps them shared.
 --
@@ -723,6 +739,7 @@ end
 local SHAPE_SLOTS = {
   Value = "value", Fields = "fields", Keys = "keys",
   KeyValue = "keyValue", Extra = "extra",
+  KeysClosed = "keysClosed",
   Semantics = "semantics", Write = "write",
   BaseAt = "baseAt", BaseIds = "baseIds",
   ReservedIds = "reservedIds",
@@ -2015,6 +2032,17 @@ R.encounters = {
   -- a namespace: a slot table is an ORDERED list whose position is the
   -- encounter roll, and Merge.deepMerge appends lists under "deep" semantics,
   -- so a mod rewriting a seven-slot table would get a fourteen-slot one.
+  --
+  -- This is the one registry whose ids are CLOSED (`gen2KeysClosed`): the
+  -- kind list below is not an open namespace a mod may add to, it is the
+  -- complete set of lookups Gold makes into the table (encounters.grass,
+  -- encounters.water, encounters.trees, ...).  A Gen 1 author porting an
+  -- encounters mod writes the MAP where Gold wants the KIND -- patch
+  -- ("ROUTE_29", { grass = ... }) -- and under an open id space that call was
+  -- accepted, written to gen2Encounters.ROUTE_29 and read by nothing: the
+  -- game stayed vanilla with no error anywhere (#2369).  Closing the set
+  -- turns that silence into the one thing the author needs, the list of ids
+  -- that do exist.
   gen2Keys = {
     grass = f.map(f.str, gen2GrassRow),
     -- the swarm variants shadow their base table while a swarm is running
@@ -2046,8 +2074,22 @@ R.encounters = {
                                chance = f.int(0, 255) }),
     -- where a roaming beast may walk next, keyed by the map it is on
     roamMaps = f.list(f.rec{ map = f.str, to = f.list(f.str) }),
+    -- the three beasts' starting slots, straight out of InitRoamMons
+    -- (src/import/RomExtractorGen2.lua readRoamMons).  Absent from an older
+    -- cache, which is why src/core/gen2/Roamers.lua keeps a fallback table.
+    roamMons = f.opt(f.list(f.rec{ species = f.opt(f.id("pokemon")),
+                                   level = f.opt(f.int(1)),
+                                   mapGroup = f.opt(f.int(0, 255)),
+                                   mapNumber = f.opt(f.int(0, 255)),
+                                   map = f.opt(f.str) })),
     source = f.str, generation = f.int(1),
   },
+  gen2KeysClosed = true,
+  gen2Notes = [[Gold's encounter ids are a **closed set**: the kinds above are
+the complete set of lookups the engine makes into `Data.gen2Encounters`, so an
+id that is not one of them is a write nothing reads. An id outside the set is
+rejected -- a Gen 1 mod ported unchanged passes the map where Gold wants the
+kind, and that call is refused rather than silently dropped.]],
   example = 'mod.content.encounters:patch("ROUTE_1", { grass = { rate = 30 } })',
   gen2Example = 'mod.content.encounters:patch("grass", '
     .. '{ ROUTE_29 = { rates = { NITE = 40 } } })',

tests/engine/gate_gen2_mod_api.lua

@@ -1026,6 +1026,130 @@ do
   run.release()
 end
 
+-- ------- 5c. Gold's encounters ids are a closed set (#2369)
+--
+-- Gold keys wild encounters by KIND first and map second, so the id a mod
+-- patches is "grass", not "ROUTE_29".  The id space was open, and a Gen 1
+-- encounters mod ported unchanged writes the MAP there: the call was accepted,
+-- merged into data.gen2Encounters.ROUTE_29 and read by nothing -- vanilla
+-- game, no error, nothing in the Mod Manager.  That key cannot be a mod's own
+-- data the way an extra palette id can, because the engine reads this table by
+-- name and the set of names is fixed, so the id space is closed and an unknown
+-- id fails the mod instead.
+do
+  local function encountersData()
+    return {
+      gen2Encounters = {
+        grass = {
+          ROUTE_29 = {
+            rates = { MORN = 51, DAY = 51, NITE = 51 },
+            slots = {
+              MORN = { { level = 3, species = "HOOTHOOT" } },
+              DAY = { { level = 3, species = "PIDGEY" } },
+              NITE = { { level = 3, species = "HOOTHOOT" } },
+            },
+          },
+        },
+      },
+    }
+  end
+
+  local function encountersMod(body)
+    return {
+      ["mods/fix_encounters/manifest.json"] = [[{
+        "id": "fix_encounters",
+        "name": "Fixture Encounters",
+        "version": "1.0.0",
+        "entry": "main.lua",
+        "api": 2,
+        "gen2compat": true
+      }]],
+      ["mods/fix_encounters/main.lua"] = body,
+    }
+  end
+
+  -- the shape itself, before any load: closed on Gold, unchanged on Red
+  do
+    local spec = Schemas.REGISTRIES.encounters
+    local ok, err = Schemas.check(spec, "encounters", "ROUTE_29",
+      { grass = { rate = 30 } }, "patch", 2)
+    T.check(not ok and err and err:match("unknown id"),
+      "Gen 2: an unknown encounters id is refused")
+    T.check(err and err:match("grass") and err:match("bugContest"),
+      "Gen 2: the refusal lists the ids that do exist: " .. tostring(err))
+    T.check(Schemas.check(spec, "encounters", "ROUTE_29",
+        { grass = { rate = 30 } }, "patch", 1) == true,
+      "Gen 1: the same call is the right form there and still passes")
+    T.check(Schemas.check(spec, "encounters", "roamMons",
+        { { species = "RAIKOU" } }, "patch", 2) == true,
+      "Gen 2: roamMons is a known id, not an unknown one")
+    T.check(Schemas.check(Schemas.REGISTRIES.palettes, "palettes",
+        "MOD_PALETTE", { colors = { 1, 2, 3, 4 } }, "patch", 2) == true,
+      "Gen 2: other id namespaces stay open -- an unknown id is a mod's own")
+  end
+
+  -- the documented Crystal form: the id is the kind, the map is a field
+  do
+    local run = T.sdk.loadMods({ "mods/fix_encounters" }, {
+      fs = T.sdk.memfs(encountersMod([[
+        local mod = ...
+        mod.content.encounters:patch("grass",
+          { ROUTE_29 = { rates = { NITE = 40 } } })
+      ]])),
+      data = encountersData(), generation = 2,
+    })
+    T.eq(statusOf(run, "fix_encounters").state, "loaded",
+      "Gen 2: the documented encounters form loads")
+    local row = run.data.gen2Encounters.grass.ROUTE_29
+    T.eq(row.rates.NITE, 40, "Gen 2: the patched rate lands")
+    T.eq(row.rates.DAY, 51, "Gen 2: the rates not named are untouched")
+    T.eq(row.slots.NITE[1].species, "HOOTHOOT",
+      "Gen 2: and the slot list survives the merge")
+    run.release()
+  end
+
+  -- a Gen 1 encounters mod ported unchanged: the map where Gold wants the kind
+  do
+    local run = T.sdk.loadMods({ "mods/fix_encounters" }, {
+      fs = T.sdk.memfs(encountersMod([[
+        local mod = ...
+        mod.content.encounters:patch("ROUTE_29", { grass = { rate = 30 } })
+      ]])),
+      data = encountersData(), generation = 2,
+    })
+    T.eq(statusOf(run, "fix_encounters").state, "failed",
+      "Gen 2: an unknown encounters id fails the mod rather than no-opping")
+    local told
+    for _, message in ipairs(run.errors) do
+      if message:match("encounters%.ROUTE_29") then told = message end
+    end
+    T.check(told ~= nil, "Gen 2: the failure names the id that was refused")
+    T.check(told and told:match("grass"),
+      "Gen 2: and lists the ids that do exist: " .. tostring(told))
+    T.eq(run.data.gen2Encounters.ROUTE_29, nil,
+      "Gen 2: nothing lands at the unknown id")
+    T.eq(run.data.gen2Encounters.grass.ROUTE_29.rates.NITE, 51,
+      "Gen 2: and the vanilla table is untouched")
+    run.release()
+  end
+
+  -- a malformed payload at a KNOWN id: the id resolves, so the type check runs
+  do
+    local run = T.sdk.loadMods({ "mods/fix_encounters" }, {
+      fs = T.sdk.memfs(encountersMod([[
+        local mod = ...
+        mod.content.encounters:patch("grass", "FORCE_ERROR_STRING")
+      ]])),
+      data = encountersData(), generation = 2,
+    })
+    T.eq(statusOf(run, "fix_encounters").state, "failed",
+      "Gen 2: a malformed encounters payload fails the mod")
+    T.eq(run.data.gen2Encounters.grass.ROUTE_29.rates.NITE, 51,
+      "Gen 2: and the vanilla table is untouched")
+    run.release()
+  end
+end
+
 -- ------- 6. StateStack:clear, which is what Gold's boot cinema hands off
 -- through now that it runs the engine stack
 

tests/gen2_battle_test.lua

@@ -981,6 +981,36 @@ check("switch spent no PP", switchParty[2].moves[1].pp, 35)
 
 local Effects = require("src.battle.gen2.Effects")
 
+-- Thunder's weather override replaces its base accuracy, but accuracy and
+-- evasion stages still apply to that overridden value.  Wrap the block in a
+-- local closure to keep the main chunk under Lua 5.1's 200-local ceiling.
+;(function()
+  local thunder = { id = "THUNDER", accuracy = 70, effect = "EFFECT_THUNDER" }
+  local accuracyAttacker = {}
+  local accuracyDefender = {}
+  local accuracyBattle = setmetatable({
+    data = { items = {} },
+    player = accuracyAttacker,
+    enemy = accuracyDefender,
+    stages = { player = Battle.newStages(), enemy = Battle.newStages() },
+    weather = nil,
+    random = maxRandom,
+  }, { __index = Battle })
+
+  check("Thunder can miss without weather",
+    accuracyBattle:accuracyRoll(thunder, accuracyAttacker, accuracyDefender), false)
+  accuracyBattle.weather = "rain"
+  check("Thunder cannot miss in rain with neutral stages",
+    accuracyBattle:accuracyRoll(thunder, accuracyAttacker, accuracyDefender), true)
+  accuracyBattle.stages.player.accuracy = -1
+  check("Thunder can miss in rain after accuracy drop",
+    accuracyBattle:accuracyRoll(thunder, accuracyAttacker, accuracyDefender), false)
+  accuracyBattle.stages.player.accuracy = 0
+  accuracyBattle.stages.enemy.evasion = 1
+  check("Thunder can miss in rain after evasion rise",
+    accuracyBattle:accuracyRoll(thunder, accuracyAttacker, accuracyDefender), false)
+end)()
+
 -- Stat stages clamp at +/-6 and report how far they actually moved, which is
 -- what decides between "rose" and "sharply rose".
 local stages = Battle.newStages()

tools/gen_registry_docs.lua

@@ -92,8 +92,15 @@ end
 local function renderSchema(spec)
   if spec.keys then
     line("")
-    line("Id = a top-level key of the target table. Keys not listed here are")
-    line("accepted and merged as-is.")
+    if spec.keysClosed then
+      line("Id = a top-level key of the target table. The set below is **closed**:")
+      line("an id that is not one of these is rejected rather than merged, because")
+      line("the engine reads this table by name and a key it does not name is a")
+      line("write nothing reads.")
+    else
+      line("Id = a top-level key of the target table. Keys not listed here are")
+      line("accepted and merged as-is.")
+    end
     line("")
     line("| key | type |")
     line("|---|---|")