Merge pull request #2398 from thibautbus/fix/game3-trainer-class-ids

Recognise FireRed trainer classes by id, not by their name

src/core/game3/battle/init.lua

@@ -534,10 +534,12 @@ function Battle.start(opts)
   }))
 
   st.trainerId = trainerId
+  st.trainerClass = trainerInfo and tonumber(trainerInfo.class)
   st.trainerClassName = trainerInfo and trainerInfo.className
   st.trainerName = (trainerInfo and trainerInfo.name) or opts.trainerName
   -- pokefirered/src/battle_message.c:394 the link opponent is named, never classed
   if st.link and not st.unionRoom and st.peerName then
+    st.trainerClass = nil
     st.trainerClassName = ""
     st.trainerName = st.peerName
   end

src/core/game3/battle_bridge.lua

@@ -480,6 +480,9 @@ function BattleBridge.start(mod, game, foe, opts)
       playerLevel = playerLv,
       enemyLevel = foeLv,
       trainerId = startOpts.trainerId,
+      trainerClass = (not opts.wild) and foe and foe.trainerClass or nil,
+      trainerTower = startOpts.trainerTower,
+      eReader = startOpts.eReader,
       playerGender = startOpts.playerGender,
       transitionId = opts.transitionId,
     }

src/core/game3/battle_transition.lua

@@ -121,19 +121,36 @@ function BattleTransition.pickWild(opts)
   end
 end
 
+-- pokefirered/include/constants/trainers.h:270, :273
+local TRAINER_CLASS_ELITE_FOUR = 87
+local TRAINER_CLASS_CHAMPION = 90
+-- pokefirered/include/constants/opponents.h:416-419, :741-744 (first run, rematch)
+local ELITE_FOUR_TRANSITION = {
+  [410] = ID.LORELEI, [735] = ID.LORELEI,
+  [411] = ID.BRUNO, [736] = ID.BRUNO,
+  [412] = ID.AGATHA, [737] = ID.AGATHA,
+  [413] = ID.LANCE, [738] = ID.LANCE,
+}
+
+-- pokefirered/src/battle_setup.c:624 GetTrainerBattleTransition: the Elite Four
+-- and the champion are recognised by class id, never by the class's name.
 function BattleTransition.pickTrainer(opts)
   opts = opts or {}
   local tid = tonumber(opts.trainerId) or 0
-  local tClass = opts.trainerClass
+  -- A Trainer Tower or e-Reader foe carries a facility class, whose numbers
+  -- overlap the trainer classes (FACILITY_CLASS_LASS is 90, the champion's,
+  -- pokefirered/include/constants/trainers.h:381); pret never picks their
+  -- transition by class (battle_setup.c:660).
+  local tClass = not (opts.trainerTower or opts.eReader) and tonumber(opts.trainerClass) or nil
 
-  if tClass == "ELITE_FOUR" or tClass == 57 then
-    if tid == 412 or tid == 413 or opts.isLorelei then return ID.LORELEI end
-    if tid == 414 or tid == 415 or opts.isBruno then return ID.BRUNO end
-    if tid == 416 or tid == 417 or opts.isAgatha then return ID.AGATHA end
-    if tid == 418 or tid == 419 or opts.isLance then return ID.LANCE end
-    return ID.BLUE
+  if tClass == TRAINER_CLASS_ELITE_FOUR then
+    if opts.isLorelei then return ID.LORELEI end
+    if opts.isBruno then return ID.BRUNO end
+    if opts.isAgatha then return ID.AGATHA end
+    if opts.isLance then return ID.LANCE end
+    return ELITE_FOUR_TRANSITION[tid] or ID.BLUE
   end
-  if tClass == "CHAMPION" or tClass == "RIVAL" or tClass == 58 or opts.isRival or opts.isChampion then
+  if tClass == TRAINER_CLASS_CHAMPION or opts.isRival or opts.isChampion then
     return ID.BLUE
   end
   if opts.isLorelei then return ID.LORELEI end

src/core/game3/quest_log_recorder.lua

@@ -91,12 +91,14 @@ function R.battle(session,st)
     local outcome=hp>=math.floor(max/3)*2 and 'Handily' or (hp>=math.floor(max/3) and 'Tenaciously' or 'Somehow')
     local args={D0=loc,D1=st.trainerName or 'TRAINER',D2=enemy,D3=player,D4={text=outcome}}
     local key='TookOnTrainersMonWithMonAndWon'
-    local class=st.trainerClassName or ''
-    if class=='LEADER' then key='TookOnGymLeadersMonWithMonAndWon'
-    elseif class=='ELITE FOUR' then
+    -- pokefirered/src/quest_log_battle.c:25 switches on the class id, which a
+    -- mod renaming the class leaves alone (include/constants/trainers.h:267-273)
+    local class=tonumber(st.trainerClass)
+    if class==84 then key='TookOnGymLeadersMonWithMonAndWon'
+    elseif class==87 then
       key='TookOnEliteFoursMonWithMonAndWon'
       args={D0=st.trainerName,D1=enemy,D2=player,D3={text=outcome}}
-    elseif class=='CHAMPION' then
+    elseif class==90 then
       key='PlayerBattledChampionRival';args={D0=session.name,D1=st.trainerName}
     end
     R.event(session,key,args)

src/core/game3/scripting/trainers.lua

@@ -8,6 +8,11 @@ local SPECIES_BULBASAUR = 1
 local SPECIES_CHARMANDER = 4
 local SPECIES_SQUIRTLE = 7
 
+-- pokefirered/include/constants/trainers.h:264, :272, :273
+local TRAINER_CLASS_RIVAL_EARLY = 81
+local TRAINER_CLASS_RIVAL_LATE = 89
+local TRAINER_CLASS_CHAMPION = 90
+
 local TRAINER_RIVAL_OAKS_LAB_SQUIRTLE = 326
 local TRAINER_RIVAL_OAKS_LAB_BULBASAUR = 327
 local TRAINER_RIVAL_OAKS_LAB_CHARMANDER = 328
@@ -263,7 +268,8 @@ function Trainers.foeFromId(trainerId)
 end
 
 --- ROM-derived trainer presentation info (class / name / pic / partySize / dialogs).
--- opts.rivalName replaces placeholder "TERRY" for class RIVAL when provided.
+-- opts.rivalName replaces the placeholder "TERRY" of the rival and champion
+-- classes when provided.
 function Trainers.info(trainerId, opts)
   opts = opts or {}
   trainerId = tonumber(trainerId)
@@ -289,7 +295,11 @@ function Trainers.info(trainerId, opts)
     dialogs = t.dialogs,
   }
 
-  if info.className == "RIVAL" and opts.rivalName and opts.rivalName ~= "" then
+  -- pokefirered/src/battle_message.c:2078 names these classes by the player's
+  -- rival, recognised by class id: a mod may rename the class itself.
+  local class = tonumber(info.class)
+  if (class == TRAINER_CLASS_RIVAL_EARLY or class == TRAINER_CLASS_RIVAL_LATE
+      or class == TRAINER_CLASS_CHAMPION) and opts.rivalName and opts.rivalName ~= "" then
     info.name = opts.rivalName
   end
   return info

tests/engine/game3_trainer_class_ids_test.lua

@@ -0,0 +1,28 @@
+#!/usr/bin/env luajit
+-- The rival and the champion take the player's chosen rival name by class id,
+-- as pret's B_TXT_TRAINER1_NAME does (pokefirered/src/battle_message.c:2078),
+-- so a mod that translates the class ("RIVALE", "MAÎTRE") keeps the name.
+
+package.path = "./?.lua;./?/init.lua;" .. package.path
+love = require("tests.love_stub")
+
+local T = require("tests.harness")
+local check = T.check
+
+local Trainers = require("src.core.game3.scripting.trainers")
+local rows = {
+  [326] = { id = 326, class = 81, className = "RIVALE", name = "TERRY", party = {} },
+  [438] = { id = 438, class = 90, className = "MAÎTRE", name = "TERRY", party = {} },
+  [500] = { id = 500, class = 89, className = "RIVAL", name = "TERRY", party = {} },
+  [347] = { id = 347, class = 82, className = "RIVAL", name = "IVAN", party = {} },
+}
+Trainers.get = function(id) return rows[tonumber(id)] end
+
+local function nameOf(id) return Trainers.info(id, { rivalName = "GARY" }).name end
+check(nameOf(326) == "GARY", "the early rival takes the rival name, whatever its class is called")
+check(nameOf(500) == "GARY", "so does the late rival")
+check(nameOf(438) == "GARY", "and the champion, as on the cart")
+check(nameOf(347) == "IVAN", "another class keeps its own trainer's name, even one called RIVAL")
+check(Trainers.info(326, {}).name == "TERRY", "with no rival name, the ROM's placeholder stays")
+
+T.finish("game3_trainer_class_ids_test")

tests/game3_battle_transition_test.lua

@@ -89,12 +89,23 @@ check(BattleTransition.pickTrainer({ terrain = TERRAIN.WATER, playerLevel = 5, e
   "trainer water high-level foe -> RIPPLE")
 
 print("[test] 5. Elite Four & Rival Mugshots Routing")
-check(BattleTransition.pickTrainer({ trainerClass = "ELITE_FOUR", trainerId = 412 }) == ID.LORELEI, "Lorelei -> LORELEI")
-check(BattleTransition.pickTrainer({ trainerClass = "ELITE_FOUR", trainerId = 414 }) == ID.BRUNO, "Bruno -> BRUNO")
-check(BattleTransition.pickTrainer({ trainerClass = "ELITE_FOUR", trainerId = 416 }) == ID.AGATHA, "Agatha -> AGATHA")
-check(BattleTransition.pickTrainer({ trainerClass = "ELITE_FOUR", trainerId = 418 }) == ID.LANCE, "Lance -> LANCE")
-check(BattleTransition.pickTrainer({ trainerClass = "ELITE_FOUR", trainerId = 420 }) == ID.BLUE, "E4 Champion -> BLUE")
-check(BattleTransition.pickTrainer({ trainerClass = "CHAMPION" }) == ID.BLUE, "Champion -> BLUE")
+-- pokefirered/src/battle_setup.c:633, opponents.h:416-419 and :741-744
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 410 }) == ID.LORELEI, "Lorelei -> LORELEI")
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 411 }) == ID.BRUNO, "Bruno -> BRUNO")
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 412 }) == ID.AGATHA, "Agatha -> AGATHA")
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 413 }) == ID.LANCE, "Lance -> LANCE")
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 737 }) == ID.AGATHA, "Agatha's rematch -> AGATHA")
+check(BattleTransition.pickTrainer({ trainerClass = 87, trainerId = 999 }) == ID.BLUE, "other Elite Four -> BLUE")
+check(BattleTransition.pickTrainer({ trainerClass = 90, trainerId = 438 }) == ID.BLUE, "Champion -> BLUE")
+check(BattleTransition.pickTrainer({ trainerClass = 81, trainerId = 326, terrain = TERRAIN.NORMAL,
+  playerLevel = 5, enemyLevel = 5 }) ~= ID.BLUE, "an early rival battle keeps the terrain transition")
+check(BattleTransition.pickTrainer({ trainerClass = "CHAMPION", playerLevel = 5, enemyLevel = 5 }) ~= ID.BLUE,
+  "a class is recognised by its id, not by its (translatable) name")
+-- pokefirered/src/trainer_tower_sets.c:8663 MIKAELA, FACILITY_CLASS_LASS (90)
+check(BattleTransition.pickTrainer({ trainerTower = true, trainerClass = 90, trainerId = 0,
+  playerLevel = 5, enemyLevel = 5 }) ~= ID.BLUE, "a Trainer Tower LASS is not taken for the champion")
+check(BattleTransition.pickTrainer({ eReader = true, trainerClass = 87, trainerId = 0,
+  playerLevel = 5, enemyLevel = 5 }) ~= ID.BLUE, "nor an e-Reader trainer for the Elite Four")
 check(BattleTransition.pickTrainer({ isRival = true }) == ID.BLUE, "Rival -> BLUE")
 
 print("[test] 6. Cache Contract & Extract Pipeline")

tests/game3_quest_log_integration_test.lua

@@ -42,9 +42,20 @@ local a={'TEST'};Recorder.event(session,'ArrivedInLocation',a);a[1]='WRONG'
 assert(session.questLog.scenes[#session.questLog.scenes].events[2].args[1]=='TEST')
 local other={map='FR_OTHER'};Recorder.event(other,'ArrivedInLocation',a);assert(other.questLog==nil)
 local Pokemon=require('src.core.game3.pokemon');Pokemon.displayMonName=function(m)return m.name end
-local st={wild=false,result='win',trainerName='BROCK',trainerClassName='LEADER',
+-- The French cart calls a gym LEADER "CHAMPION": the class id decides, not its name.
+local st={wild=false,result='win',trainerName='BROCK',trainerClass=84,trainerClassName='CHAMPION',
  player={mon={name='BULBASAUR',hp=20,maxHp=30}},enemy={mon={name='ONIX'}}}
 Recorder.battle(session,st)
 local e=session.questLog.scenes[#session.questLog.scenes].events[3]
 assert(e.key=='TookOnGymLeadersMonWithMonAndWon' and e.args.D4.text=='Handily')
+local function keyFor(class,className)
+  local s={wild=false,result='win',trainerName='X',trainerClass=class,trainerClassName=className,
+   player={mon={name='BULBASAUR',hp=20,maxHp=30}},enemy={mon={name='ONIX'}}}
+  Recorder.battle(session,s)
+  local evs=session.questLog.scenes[#session.questLog.scenes].events
+  return evs[#evs].key
+end
+assert(keyFor(87,'CONSEIL 4')=='TookOnEliteFoursMonWithMonAndWon')
+assert(keyFor(90,'MAÎTRE')=='PlayerBattledChampionRival')
+assert(keyFor(82,'LEADER')=='TookOnTrainersMonWithMonAndWon')
 print('PASS Quest Log Continue, legacy save, quit, RNG isolation, event scoping and battle summary')