fix: level-up gains read signed, so a loss prints -2, as 1.12.1 does (#791)

- Change `LevelUpInfo` to signed to match vanilla behavior
- Add parse test for negative values in SMSG_LEVELUP_INFO, update comments
- grammar
- level-up gains: cite the %d fire and vmangos's wrapped deltas; pin the reported line

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Sam <samuv.eth@gmail.com>
This commit is contained in:
elipp
2026-10-04 10:18:24 +07:00
committed by GitHub
co-authored by Claude Opus 5.5 Sam
parent 4a389add47
commit aa103d5a65
4 changed files with 77 additions and 49 deletions
+33 -35
View File
@@ -2288,31 +2288,9 @@ fn level_up_gains_are_matched_by_level_and_a_miss_is_not_an_absence() {
);
}
/// Stock `ChatFrame_OnEvent` prints the whole level-up block from `PLAYER_LEVEL_UP`
/// (`ChatFrame.lua:1283-1323`); the app composes none of it.
#[test]
fn the_ding_block_is_printed_once() {
let _data = benilla_formats::wow_data_or_skip!();
use benilla_protocol::messages::LevelUpInfo;
let mut s = chat_vm();
let mut log = super::ChatLog::default();
let info = LevelUpInfo {
level: 10,
health: 22,
powers: [15, 0, 0, 0, 0],
stats: [1, 0, 0, 0, 0],
};
// The packet's apply parks the gains and prints nothing.
log.push_level_up_gains(&info, 1);
let before = lines_in_window(&s);
assert_eq!(
lines_in_window(&s) - before,
0,
"the app composes no ding line of its own"
);
// Tap the window's `AddMessage` to read the block itself.
/// Fires `PLAYER_LEVEL_UP` with the nine reference args and returns every line the stock handler
/// added to `ChatFrame1`.
fn ding_lines(s: &mut benilla_ui::script::UiScript, args: [i64; 9]) -> Vec<String> {
s.run(
r#"
DingLines = {}
@@ -2325,23 +2303,27 @@ fn the_ding_block_is_printed_once() {
)
.unwrap();
// The event `ui_unit` fires, with the reference's nine arguments.
let args: Vec<benilla_ui::script::ScriptValue> = [10i64, 22, 15, 1, 1, 0, 0, 0, 0]
let args = args
.into_iter()
.map(benilla_ui::script::ScriptValue::Int)
.collect();
s.fire_event("PLAYER_LEVEL_UP", args);
assert!(s.errors().is_empty(), "handler errors: {:?}", s.errors());
assert_eq!(
lines_in_window(&s) - before,
4,
"LEVEL_UP, the health/mana pair, CHAR_POINTS, and one STAT — once each"
);
// The singular `LEVEL_UP_CHAR_POINTS` (`GetText`'s plural pick) and one `LEVEL_UP_STAT`.
let lines: Vec<String> = (1..=4)
let n: usize = s.eval("return table.getn(DingLines)").unwrap();
(1..=n)
.map(|i| s.eval::<String>(&format!("return DingLines[{i}]")).unwrap())
.collect();
.collect()
}
/// Stock `ChatFrame_OnEvent` prints the whole level-up block from `PLAYER_LEVEL_UP`
/// (`ChatFrame.lua:1283-1323`), once; the app composes none of it. The singular
/// `LEVEL_UP_CHAR_POINTS` is `GetText`'s plural pick.
#[test]
fn the_ding_block_is_printed_once() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = chat_vm();
let lines = ding_lines(&mut s, [10, 22, 15, 1, 1, 0, 0, 0, 0]);
assert_eq!(
lines,
[
@@ -2353,6 +2335,22 @@ fn the_ding_block_is_printed_once() {
);
}
/// A loss reaches stock signed, as the reference's `%d` fire passes it (`0x5e413e`): the health
/// line prints it, and a lost stat prints no line (`if ( argN > 0 )`, `ChatFrame.lua:1302-1321`).
#[test]
fn a_negative_gain_prints_signed() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = chat_vm();
let lines = ding_lines(&mut s, [2, -2, 5, 0, -1, 0, 0, 0, 0]);
assert_eq!(
lines,
[
"Congratulations, you have reached level 2!",
"You have gained -2 hit points and 5 mana.",
]
);
}
/// On `CHARACTER_POINTS_CHANGED` with `arg2 > 0`, stock `ChatFrame_OnEvent` prints
/// `LEVEL_UP_SKILL_POINTS` (`ChatFrame.lua:1324-1334`).
#[test]
+1 -1
View File
@@ -1444,7 +1444,7 @@ fn feed_units(
// `ChatFrame.lua:1283-1320` reads them; missing gains are zeros, since stock's
// `if ( argN > 0 )` raises on nil.
let (info, talent_points) = sink.chat.take_level_up_gains(level).unzip();
let gain = |f: fn(&benilla_protocol::messages::LevelUpInfo) -> u32| {
let gain = |f: fn(&benilla_protocol::messages::LevelUpInfo) -> i32| {
ScriptValue::Int(i64::from(info.as_ref().map_or(0, f)))
};
script.fire_event(
@@ -2,7 +2,7 @@
use std::io::{self, Read};
use crate::wire::{read_f32_le, read_u32_le, read_u64_le, read_u8};
use crate::wire::{read_f32_le, read_i32_le, read_u32_le, read_u64_le, read_u8};
/// `SMSG_LOG_XPGAIN`: an XP award; `victim` is 0 for non-kill XP (vmangos `Misc.cpp:512-522`).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -92,27 +92,29 @@ pub fn talent_wipe_confirm(trainer_guid: u64) -> Vec<u8> {
pub struct LevelUpInfo {
/// The level just reached.
pub level: u32,
/// Hit points gained.
pub health: u32,
/// Hit points gained, negative on a loss.
pub health: i32,
/// Power gains: mana, rage, focus, energy, happiness. The server fills only mana, which is
/// the reference chat line's mana argument.
pub powers: [u32; 5],
pub powers: [i32; 5],
/// Stat gains in `SPELL_STAT0..4` order (str, agi, stam, int, spirit), one chat line per
/// positive entry.
pub stats: [u32; 5],
pub stats: [i32; 5],
}
/// Read `SMSG_LEVELUP_INFO`: `u32` level, health, 5 powers, 5 stats (`Misc.cpp:524-532`).
/// Read `SMSG_LEVELUP_INFO`: `u32` level, `i32` health, 5 powers, 5 stats (`Misc.cpp:524-532`).
/// The reference fires the gains as `%d` (`0x5e413e`), so a loss the server wraps into its `u32`
/// (vmangos `Player.cpp:3203-3215`) reads negative.
pub(super) fn read_level_up_info(r: &mut impl Read) -> io::Result<LevelUpInfo> {
let level = read_u32_le(r)?;
let health = read_u32_le(r)?;
let mut powers = [0u32; 5];
let health = read_i32_le(r)?;
let mut powers = [0; 5];
for p in &mut powers {
*p = read_u32_le(r)?;
*p = read_i32_le(r)?;
}
let mut stats = [0u32; 5];
let mut stats = [0; 5];
for s in &mut stats {
*s = read_u32_le(r)?;
*s = read_i32_le(r)?;
}
Ok(LevelUpInfo {
level,
+30 -2
View File
@@ -1003,8 +1003,8 @@ fn combat_log_wire_golden() {
other => panic!("exploration xp event, got {other:?}"),
}
// SMSG_LEVELUP_INFO: twelve u32, level, healthGain, powerGains[5] (mana..happiness) and
// statGains[5] (str..spirit), no guid (vmangos `Misc.cpp:524-532`).
// SMSG_LEVELUP_INFO: u32 level, then the i32 gains, healthGain, powerGains[5] (mana..happiness)
// and statGains[5] (str..spirit), no guid (vmangos `Misc.cpp:524-532`).
let mut body = Vec::new();
for v in [7u32, 22, 15, 0, 0, 0, 0, 1, 0, 1, 2, 1] {
body.extend_from_slice(&v.to_le_bytes());
@@ -1029,6 +1029,34 @@ fn combat_log_wire_golden() {
SessionEvent::LevelUp(l) => assert_eq!((l.level, l.health, l.powers[0]), (7, 22, 15)),
other => panic!("level up event, got {other:?}"),
}
// A loss the server wraps into its u32 reads negative, as the reference's `%d` fire prints it.
let mut body = Vec::new();
for v in [
7u32, 0xFFFFFFFE, 0xFFFFFFFD, 0, 0, 0, 0, 0xFFFFFFFE, 0, 0xFFFFFFFB, 0xFFFFFFFA, 0xFFFFFFF9,
] {
body.extend_from_slice(&v.to_le_bytes());
}
assert_eq!(body.len(), 48);
let packet = messages::parse_server(messages::opcode::SMSG_LEVELUP_INFO, &body).unwrap();
match &packet {
ServerPacket::LevelUp(l) => {
assert_eq!(
l,
&LevelUpInfo {
level: 7,
health: -2,
powers: [-3, 0, 0, 0, 0],
stats: [-2, 0, -5, -6, -7],
}
);
}
other => panic!("level up, got {}", other.name()),
}
match decode(packet).pop().unwrap() {
SessionEvent::LevelUp(l) => assert_eq!((l.level, l.health, l.powers[0]), (7, -2, -3)),
other => panic!("level up event, got {other:?}"),
}
}
/// Raw caster guid, u32 spellId, u32 count, raw guids (vmangos `Spell.cpp:4974-4997`); in the