From e97461186757ca5bc34f4d11ab67ee3e28a478ee Mon Sep 17 00:00:00 2001 From: cDmonio Date: Tue, 30 Jun 2026 21:11:21 +0000 Subject: [PATCH 1/6] Fix spell power spend and persistence --- .../wow-database/src/statements/character.rs | 10 + crates/wow-packet/src/packets/update.rs | 100 ++++- crates/wow-world/src/handlers/character.rs | 21 + crates/wow-world/src/handlers/spell.rs | 380 +++++++++++++++--- crates/wow-world/src/session.rs | 136 +++++++ 5 files changed, 595 insertions(+), 52 deletions(-) diff --git a/crates/wow-database/src/statements/character.rs b/crates/wow-database/src/statements/character.rs index 69123235..265bbea5 100644 --- a/crates/wow-database/src/statements/character.rs +++ b/crates/wow-database/src/statements/character.rs @@ -1174,6 +1174,9 @@ pub enum CharStatements { /// UPDATE characters SET money = ? WHERE guid = ? UPD_CHAR_MONEY, /// C++ `CHAR_UPD_CHARACTER` persists these fields in the full save. + /// UPDATE characters SET power1 = ?, ..., power10 = ? WHERE guid = ? + UPD_CHAR_POWERS, + /// C++ `CHAR_UPD_CHARACTER` persists these fields in the full save. /// UPDATE characters SET resettalents_cost = ?, resettalents_time = ? WHERE guid = ? UPD_CHAR_TALENT_RESET_STATE, /// UPDATE characters SET xp = ? WHERE guid = ? @@ -2740,6 +2743,9 @@ impl StatementDef for CharStatements { Self::UPD_CHAR_XP => "UPDATE characters SET xp = ? WHERE guid = ?", Self::UPD_CHAR_LEVEL => "UPDATE characters SET level = ?, xp = ? WHERE guid = ?", Self::UPD_CHAR_MONEY => "UPDATE characters SET money = ? WHERE guid = ?", + Self::UPD_CHAR_POWERS => { + "UPDATE characters SET power1 = ?, power2 = ?, power3 = ?, power4 = ?, power5 = ?, power6 = ?, power7 = ?, power8 = ?, power9 = ?, power10 = ? WHERE guid = ?" + } Self::UPD_CHAR_TALENT_RESET_STATE => { "UPDATE characters SET resettalents_cost = ?, resettalents_time = ? WHERE guid = ?" } @@ -4579,6 +4585,10 @@ mod tests { CharStatements::UPD_CHAR_MONEY.sql(), "UPDATE characters SET money = ? WHERE guid = ?" ); + assert_eq!( + CharStatements::UPD_CHAR_POWERS.sql(), + "UPDATE characters SET power1 = ?, power2 = ?, power3 = ?, power4 = ?, power5 = ?, power6 = ?, power7 = ?, power8 = ?, power9 = ?, power10 = ? WHERE guid = ?" + ); assert_eq!( CharStatements::UPD_CHAR_DIFFICULTIES.sql(), "UPDATE characters SET dungeonDifficulty = ?, raidDifficulty = ?, legacyRaidDifficulty = ? WHERE guid = ?" diff --git a/crates/wow-packet/src/packets/update.rs b/crates/wow-packet/src/packets/update.rs index 2111a8d5..a6b4d250 100644 --- a/crates/wow-packet/src/packets/update.rs +++ b/crates/wow-packet/src/packets/update.rs @@ -1550,6 +1550,8 @@ pub struct PlayerCreateData { pub base_armor: i32, /// Max mana from level stats (for caster classes). pub max_mana: i64, + /// Current primary power stored in `UnitData::Power[0]`. + pub current_power0: i32, /// Melee attack power. pub attack_power: i32, /// Ranged attack power. @@ -1659,13 +1661,13 @@ impl PlayerCreateData { } } - /// Get the power value for slot 0, using real mana for caster classes. + /// Get the max power value for slot 0, using real mana for caster classes. /// /// - Warrior (1): rage = 1000 (stored as 10×) /// - Rogue (4): energy = 100 /// - DK (6): runic power = 1000 (stored as 10×) /// - All others: mana from `max_mana` field (loaded from player_levelstats) - fn power_for_slot0(&self) -> i32 { + fn max_power_for_slot0(&self) -> i32 { match self.class { 1 => 1000, // Warrior: rage 4 => 100, // Rogue: energy @@ -1674,6 +1676,19 @@ impl PlayerCreateData { } } + fn current_power_for_slot0(&self) -> i32 { + self.current_power0 + .clamp(0, self.max_power_for_slot0().max(0)) + } + + fn base_mana_for_create_like_cpp(&self) -> i32 { + if power_type_for_class(self.class) == 0 { + self.max_mana.max(0).min(i64::from(i32::MAX)) as i32 + } else { + 0 + } + } + /// Write the complete values block for CREATE (no change masks). /// /// Format: `[u32 size][u8 flags][ObjectData][UnitData][PlayerData][ActivePlayerData?]` @@ -1804,11 +1819,12 @@ impl PlayerCreateData { } // Power[10], MaxPower[10], ModPowerRegen[10] - let power0 = self.power_for_slot0(); + let current_power0 = self.current_power_for_slot0(); + let max_power0 = self.max_power_for_slot0(); for i in 0..10 { if i == 0 { - buf.write_int32(power0); - buf.write_int32(power0); + buf.write_int32(current_power0); + buf.write_int32(max_power0); } else { buf.write_int32(0); buf.write_int32(0); @@ -1943,7 +1959,7 @@ impl PlayerCreateData { } // BaseMana — use real mana from stats store for caster classes - buf.write_int32(self.power_for_slot0()); + buf.write_int32(self.base_mana_for_create_like_cpp()); // BaseHealth (Owner only) if is_owner { @@ -3745,6 +3761,12 @@ impl UpdateObject { stats: combat.stats, base_armor: combat.base_armor, max_mana: combat.max_mana, + current_power0: match class { + 1 => 1000, + 4 => 100, + 6 => 1000, + _ => combat.max_mana.max(0).min(i64::from(i32::MAX)) as i32, + }, attack_power: combat.attack_power, ranged_attack_power: combat.ranged_attack_power, min_damage: combat.min_damage, @@ -3829,6 +3851,24 @@ impl UpdateObject { } } + /// Override `UnitData::Power[0]` for the self player create block. + /// + /// C++ `Player::BuildValuesCreate` serializes the live current power and + /// max power separately. Login loads current `characters.power1`; callers + /// that do not have that DB value keep the default full-power create data. + pub fn set_player_current_power0_like_cpp(&mut self, current_power0: i32) { + for block in &mut self.blocks { + if let UpdateBlock::CreateObject { + create_data, + is_self: true, + .. + } = block + { + create_data.current_power0 = current_power0; + } + } + } + /// Populate C++ `Player::m_actionButtons` for the self create block. pub fn set_player_action_buttons_like_cpp( &mut self, @@ -10936,6 +10976,7 @@ mod tests { stats: [0; 5], base_armor: 0, max_mana: 0, + current_power0: 1000, attack_power: 0, ranged_attack_power: 0, min_damage: 1.0, @@ -11266,6 +11307,53 @@ mod tests { assert_eq!(create_data.farsight_object, ObjectGuid::EMPTY); } + #[test] + fn create_player_self_current_power_can_use_saved_db_value_like_cpp() { + let guid = ObjectGuid::create_player(1, 42); + let pos = Position::new(0.0, 0.0, 0.0, 0.0); + let mut combat = PlayerCombatStats::default(); + combat.max_mana = 1000; + let mut packet = UpdateObject::create_player( + guid, + 1, + 5, + 0, + 1, + 49, + &pos, + 0, + 12, + true, + [(0, 0, 0); 19], + [ObjectGuid::EMPTY; 141], + combat, + Vec::new(), + 0, + Vec::new(), + ); + + packet.set_player_current_power0_like_cpp(321); + + let UpdateBlock::CreateObject { create_data, .. } = &packet.blocks[0] else { + panic!("create_player should emit one CreateObject block"); + }; + assert_eq!( + create_data.current_power_for_slot0(), + 321, + "C++ login self UpdateObject serializes current UnitData::Power[0] from characters.power1" + ); + assert_eq!( + create_data.max_power_for_slot0(), + 1000, + "current power must not overwrite UnitData::MaxPower[0]" + ); + assert_eq!( + create_data.base_mana_for_create_like_cpp(), + 1000, + "mana percentage spell costs still use create/base mana" + ); + } + #[test] fn create_player_non_self() { // Non-self player should be smaller (no ActivePlayerData) diff --git a/crates/wow-world/src/handlers/character.rs b/crates/wow-world/src/handlers/character.rs index 98d2ceb3..f9a6e35b 100644 --- a/crates/wow-world/src/handlers/character.rs +++ b/crates/wow-world/src/handlers/character.rs @@ -13692,6 +13692,17 @@ impl WorldSession { primary_max_power, primary_base_mana, ); + if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { + info!( + guid = ?guid, + class, + power_type = ?primary_power_type, + current_power0, + max_power0 = primary_max_power, + base_mana = primary_base_mana, + "RUST_LOGIN_POWER_SYNC" + ); + } self.login_time = Some(std::time::Instant::now()); self.suppress_creature_movement_queued_at_or_before_like_cpp = None; // Clear per-session loot/combat state as part of the Rust AddToWorld @@ -13752,6 +13763,16 @@ impl WorldSession { quest_log, self.party_member_party_type_like_cpp(), ); + player_pkt.set_player_current_power0_like_cpp(current_power0); + if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { + info!( + guid = ?guid, + current_power0, + max_power0 = primary_max_power, + base_mana = primary_base_mana, + "RUST_LOGIN_POWER_CREATE" + ); + } player_pkt.set_player_account_guids_like_cpp( ObjectGuid::create_global(HighGuid::WowAccount, 0, self.account_id as i64), ObjectGuid::create_global( diff --git a/crates/wow-world/src/handlers/spell.rs b/crates/wow-world/src/handlers/spell.rs index 334ed2f2..c847421f 100644 --- a/crates/wow-world/src/handlers/spell.rs +++ b/crates/wow-world/src/handlers/spell.rs @@ -205,13 +205,17 @@ inventory::submit! { // ── Handler implementations ─────────────────────────────────────── impl WorldSession { - fn check_and_take_spell_power_like_cpp( + fn spell_power_cost_snapshot_like_cpp( &mut self, spell_info: &wow_data::SpellInfo, cast_id: ObjectGuid, spell_id: i32, - visual: &SpellCastVisual, - ) -> bool { + phase: &'static str, + ) -> Option<( + i32, + Vec, + Vec<(i8, i32, i32)>, + )> { let trace_spell_power = spell_power_trace_enabled_like_cpp(); let Some((caster_create_mana, power_costs, before_power)) = self .mutate_canonical_player_like_cpp(|player| { @@ -233,27 +237,17 @@ impl WorldSession { else { if trace_spell_power { info!( - "RUST_SPELL_POWER_COST spell_id={} spell_info_id={} cast_id={:?} result=no_canonical_player rows={:?}", - spell_id, spell_info.spell_id, cast_id, spell_info.power_costs + "RUST_SPELL_POWER_COST phase={} spell_id={} spell_info_id={} cast_id={:?} result=no_canonical_player rows={:?}", + phase, spell_id, spell_info.spell_id, cast_id, spell_info.power_costs ); } - if spell_info.power_costs.is_empty() { - return true; - } - self.send_packet(&CastFailed { - cast_id, - spell_id, - visual: visual.clone(), - reason: SpellCastResult::NoPower as i32, - fail_arg1: 0, - fail_arg2: 0, - }); - return false; + return None; }; if trace_spell_power { info!( - "RUST_SPELL_POWER_COST spell_id={} spell_info_id={} cast_id={:?} base_mana={} rows={:?} calculated_costs={:?} before_power={:?}", + "RUST_SPELL_POWER_COST phase={} spell_id={} spell_info_id={} cast_id={:?} base_mana={} rows={:?} calculated_costs={:?} before_power={:?}", + phase, spell_id, spell_info.spell_id, cast_id, @@ -264,17 +258,14 @@ impl WorldSession { ); } - if power_costs.is_empty() { - if trace_spell_power { - info!( - "RUST_SPELL_POWER_COST spell_id={} cast_id={:?} result=no_represented_cost base_mana={}", - spell_id, cast_id, caster_create_mana - ); - } - return true; - } + Some((caster_create_mana, power_costs, before_power)) + } - let has_power = power_costs.iter().all(|cost| { + fn represented_spell_power_has_power_like_cpp( + power_costs: &[wow_data::SpellPowerCostLikeCpp], + before_power: &[(i8, i32, i32)], + ) -> bool { + power_costs.iter().all(|cost| { if cost.amount <= 0 { return true; } @@ -292,23 +283,103 @@ impl WorldSession { .find(|(snapshot_power_type, _, _)| *snapshot_power_type == cost.power_type) .map(|(_, current, _)| *current >= cost.amount) .unwrap_or(false) + }) + } + + fn send_spell_power_no_power_like_cpp( + &mut self, + cast_id: ObjectGuid, + spell_id: i32, + visual: &SpellCastVisual, + ) { + self.send_packet(&CastFailed { + cast_id, + spell_id, + visual: visual.clone(), + reason: SpellCastResult::NoPower as i32, + fail_arg1: 0, + fail_arg2: 0, }); + } - if !has_power { + pub(crate) fn check_spell_power_like_cpp( + &mut self, + spell_info: &wow_data::SpellInfo, + cast_id: ObjectGuid, + spell_id: i32, + visual: &SpellCastVisual, + ) -> bool { + let trace_spell_power = spell_power_trace_enabled_like_cpp(); + let Some((caster_create_mana, power_costs, before_power)) = + self.spell_power_cost_snapshot_like_cpp(spell_info, cast_id, spell_id, "check") + else { + if spell_info.power_costs.is_empty() { + return true; + } + self.send_spell_power_no_power_like_cpp(cast_id, spell_id, visual); + return false; + }; + + if power_costs.is_empty() { + if trace_spell_power { + info!( + "RUST_SPELL_POWER_COST phase=check spell_id={} cast_id={:?} result=no_represented_cost base_mana={}", + spell_id, cast_id, caster_create_mana + ); + } + return true; + } + + if !Self::represented_spell_power_has_power_like_cpp(&power_costs, &before_power) { if trace_spell_power { info!( - "RUST_SPELL_POWER_COST spell_id={} cast_id={:?} result=no_power costs={:?} before_power={:?}", + "RUST_SPELL_POWER_COST phase=check spell_id={} cast_id={:?} result=no_power costs={:?} before_power={:?}", spell_id, cast_id, power_costs, before_power ); } - self.send_packet(&CastFailed { - cast_id, - spell_id, - visual: visual.clone(), - reason: SpellCastResult::NoPower as i32, - fail_arg1: 0, - fail_arg2: 0, - }); + self.send_spell_power_no_power_like_cpp(cast_id, spell_id, visual); + return false; + } + + true + } + + pub(crate) fn take_spell_power_like_cpp( + &mut self, + spell_info: &wow_data::SpellInfo, + cast_id: ObjectGuid, + spell_id: i32, + visual: &SpellCastVisual, + ) -> bool { + let trace_spell_power = spell_power_trace_enabled_like_cpp(); + let Some((caster_create_mana, power_costs, before_power)) = + self.spell_power_cost_snapshot_like_cpp(spell_info, cast_id, spell_id, "take") + else { + if spell_info.power_costs.is_empty() { + return true; + } + self.send_spell_power_no_power_like_cpp(cast_id, spell_id, visual); + return false; + }; + + if power_costs.is_empty() { + if trace_spell_power { + info!( + "RUST_SPELL_POWER_COST phase=take spell_id={} cast_id={:?} result=no_represented_cost base_mana={}", + spell_id, cast_id, caster_create_mana + ); + } + return true; + } + + if !Self::represented_spell_power_has_power_like_cpp(&power_costs, &before_power) { + if trace_spell_power { + info!( + "RUST_SPELL_POWER_COST phase=take spell_id={} cast_id={:?} result=no_power costs={:?} before_power={:?}", + spell_id, cast_id, power_costs, before_power + ); + } + self.send_spell_power_no_power_like_cpp(cast_id, spell_id, visual); return false; } @@ -344,7 +415,7 @@ impl WorldSession { if let Some((update, after_power)) = update { if trace_spell_power { info!( - "RUST_SPELL_POWER_COST spell_id={} cast_id={:?} result=deducted costs={:?} before_power={:?} after_power={:?}", + "RUST_SPELL_POWER_COST phase=take spell_id={} cast_id={:?} result=deducted costs={:?} before_power={:?} after_power={:?}", spell_id, cast_id, power_costs, before_power, after_power ); } @@ -352,7 +423,7 @@ impl WorldSession { } else { if trace_spell_power { info!( - "RUST_SPELL_POWER_COST spell_id={} cast_id={:?} result=lost_canonical_player_before_deduct costs={:?}", + "RUST_SPELL_POWER_COST phase=take spell_id={} cast_id={:?} result=lost_canonical_player_before_deduct costs={:?}", spell_id, cast_id, power_costs ); } @@ -580,10 +651,10 @@ impl WorldSession { } } - // C++ `Spell::CheckCast` verifies power and `Spell::TakePower` deducts - // the calculated `SpellInfo::PowerCosts` for accepted casts. This - // represented path covers flat DB2 costs and mana percentage costs. - if !self.check_and_take_spell_power_like_cpp(&spell_info, cast_id, spell_id, &req.visual) { + // C++ `Spell::CheckCast` verifies power before cast start/execution. + // `Spell::TakePower` happens later, after represented cast-failure + // gates have passed and immediately before `SMSG_SPELL_GO`. + if !self.check_spell_power_like_cpp(&spell_info, cast_id, spell_id, &req.visual) { return; } @@ -624,7 +695,12 @@ impl WorldSession { spell_visual_id: req.visual.spell_visual_id, script_visual_id: 0, }, - metadata: crate::session::SpellCastMetadata::default(), + metadata: crate::session::SpellCastMetadata { + from_client: true, + misc: req.misc, + original_cast_id: cast_id, + ..crate::session::SpellCastMetadata::default() + }, }); info!( @@ -641,7 +717,22 @@ impl WorldSession { "Instant cast, executing immediately" ); if let Err(e) = self - .execute_spell_with_target_data(spell_id, target_guid, spell_target) + .execute_spell_with_visual_and_target_data_with_metadata( + spell_id, + target_guid, + cast_id, + SpellCastVisual { + spell_visual_id: req.visual.spell_visual_id, + script_visual_id: 0, + }, + spell_target, + crate::session::SpellCastMetadata { + from_client: true, + misc: req.misc, + original_cast_id: cast_id, + ..crate::session::SpellCastMetadata::default() + }, + ) .await { warn!( @@ -3135,6 +3226,78 @@ mod tests { Arc::new(spell_store) } + fn spell_store_with_global_cooldown_and_mana_power_cost_like_cpp( + spell_id: i32, + cooldown_ms: u32, + mana_cost: i32, + power_cost_pct: f32, + ) -> Arc { + let mut spell_store = wow_data::SpellStore::new(); + spell_store.insert( + spell_id, + wow_data::SpellInfo { + spell_id, + cast_time_ms: 0, + cooldown_ms, + recovery_time_ms: 0, + effect_type: 0, + effect_base_points: 0, + effect_bonus_coefficient: 0.0, + aura_type: None, + display_flags: 0, + requires_spell_focus: 0, + power_costs: vec![wow_data::SpellPowerCostInfoLikeCpp { + order_index: 0, + power_type: PowerType::Mana as i8, + mana_cost, + power_cost_pct, + power_cost_max_pct: 0.0, + required_aura_spell_id: 0, + optional_cost: 0, + }], + effects: Vec::new(), + }, + ); + Arc::new(spell_store) + } + + fn spell_store_with_mana_cost_and_missing_focus_like_cpp( + spell_id: i32, + mana_cost: i32, + ) -> Arc { + let mut spell_store = wow_data::SpellStore::new(); + spell_store.insert( + spell_id, + wow_data::SpellInfo { + spell_id, + cast_time_ms: 0, + cooldown_ms: 0, + recovery_time_ms: 0, + effect_type: wow_data::spell::spell_effect_types::SPELL_EFFECT_SUMMON_OBJECT_WILD, + effect_base_points: 0, + effect_bonus_coefficient: 0.0, + aura_type: None, + display_flags: 0, + requires_spell_focus: 777, + power_costs: vec![wow_data::SpellPowerCostInfoLikeCpp { + order_index: 0, + power_type: PowerType::Mana as i8, + mana_cost, + power_cost_pct: 0.0, + power_cost_max_pct: 0.0, + required_aura_spell_id: 0, + optional_cost: 0, + }], + effects: vec![wow_data::SpellEffectInfo { + effect_index: 0, + effect: wow_data::spell::spell_effect_types::SPELL_EFFECT_SUMMON_OBJECT_WILD, + ..Default::default() + }], + }, + ); + Arc::new(spell_store) + } + fn set_canonical_player_mana_like_cpp( session: &mut crate::session::WorldSession, current: i32, @@ -3154,6 +3317,15 @@ mod tests { .unwrap_or(0) } + fn canonical_player_power_like_cpp( + session: &mut crate::session::WorldSession, + power: PowerType, + ) -> i32 { + session + .mutate_canonical_player_like_cpp(|player| player.get_power(power)) + .unwrap_or(0) + } + fn spell_store_with_global_cooldown( spell_id: i32, cooldown_ms: u32, @@ -4668,6 +4840,122 @@ mod tests { ); } + #[tokio::test] + async fn cast_spell_late_check_failure_does_not_deduct_power_like_cpp() { + let (mut session, send_rx) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 42); + let spell_id = 13_353; + install_canonical_player(&mut session, &canonical, player_guid); + set_canonical_player_mana_like_cpp(&mut session, 500, 1000); + session.set_known_spells_like_cpp(vec![spell_id]); + session.set_spell_store(spell_store_with_mana_cost_and_missing_focus_like_cpp( + spell_id, 150, + )); + + session + .handle_cast_spell(cast_spell_packet(spell_id, player_guid)) + .await; + + assert_eq!( + canonical_player_mana_like_cpp(&mut session), + 500, + "C++ Spell::TakePower happens after represented CheckCast failures such as missing spell focus" + ); + let packets = drain_server_packet_bytes(&send_rx); + assert_eq!(packets.len(), 1); + assert_eq!( + cast_failed_reason_like_cpp(&packets[0]), + SpellCastResult::RequiresSpellFocus as i32 + ); + } + + #[tokio::test] + async fn queued_spell_with_mana_cost_deducts_when_executed_like_cpp() { + let (mut session, send_rx) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 42); + let spell_id = 13_354; + install_canonical_player(&mut session, &canonical, player_guid); + set_canonical_player_mana_like_cpp(&mut session, 500, 1000); + session.set_known_spells_like_cpp(vec![spell_id]); + session.set_spell_store( + spell_store_with_global_cooldown_and_mana_power_cost_like_cpp( + spell_id, 1_500, 50, 10.0, + ), + ); + session.last_spell_cast_time = + Some(std::time::Instant::now() - std::time::Duration::from_millis(1_200)); + + session + .handle_cast_spell(cast_spell_packet(spell_id, player_guid)) + .await; + + assert!( + session + .represented_pending_spell_cast_request_like_cpp + .as_ref() + .is_some_and(|pending| pending.spell_id == spell_id) + ); + assert!( + send_rx.is_empty(), + "C++ Player::_pendingSpellCastRequest queues without executing or spending power" + ); + assert_eq!(canonical_player_mana_like_cpp(&mut session), 500); + + session.last_spell_cast_time = + Some(std::time::Instant::now() - std::time::Duration::from_millis(1_500)); + session.tick_pending_spell_cast_request_like_cpp().await; + + assert!( + session + .represented_pending_spell_cast_request_like_cpp + .is_none() + ); + assert_eq!( + canonical_player_mana_like_cpp(&mut session), + 350, + "queued C++ casts still run Spell::TakePower when the pending request executes" + ); + let opcodes = drain_server_opcodes(&send_rx); + assert!(opcodes.contains(&ServerOpcodes::UpdateObject)); + assert!(opcodes.contains(&ServerOpcodes::SpellGo)); + assert!(opcodes.contains(&ServerOpcodes::CooldownEvent)); + } + + #[tokio::test] + async fn mana_cost_spell_on_non_mana_class_does_not_spend_primary_power_like_cpp() { + let (mut session, send_rx) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 42); + let spell_id = 13_355; + install_canonical_player(&mut session, &canonical, player_guid); + assert!(session.sync_canonical_player_primary_power_like_cpp( + PowerType::Rage, + 500, + 1000, + 0 + )); + session.set_known_spells_like_cpp(vec![spell_id]); + session.set_spell_store(spell_store_with_mana_power_cost_like_cpp(spell_id, 50, 0.0)); + + session + .handle_cast_spell(cast_spell_packet(spell_id, player_guid)) + .await; + + assert_eq!( + canonical_player_power_like_cpp(&mut session, PowerType::Rage), + 500, + "C++ GetPowerIndexByClass has no Mana slot for warrior, so mana-cost spells cannot spend Rage as Mana" + ); + let packets = drain_server_packet_bytes(&send_rx); + assert_eq!(packets.len(), 1); + assert_eq!( + cast_failed_reason_like_cpp(&packets[0]), + SpellCastResult::NoPower as i32 + ); + } + #[tokio::test] async fn cast_spell_rejects_gcd_outside_spell_queue_window_like_cpp() { let (mut session, send_rx) = make_session(); diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index 539b04aa..502dce86 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -1131,6 +1131,7 @@ pub(crate) struct PlayerSaveToDbSnapshotLikeCpp { pub level: u8, pub xp: u32, pub money: u64, + pub powers: [i32; 10], } #[derive(Debug, Clone, Copy, PartialEq)] @@ -7176,6 +7177,7 @@ impl WorldSession { level: player.unit().data().level.clamp(0, i32::from(u8::MAX)) as u8, xp: player.active_data().xp.max(0) as u32, money: player.active_data().coinage, + powers: player.unit().data().power, }); }); if snapshot.is_some() { @@ -7204,6 +7206,7 @@ impl WorldSession { level: self.player_level_like_cpp(), xp: self.player_xp_like_cpp(), money: self.player_gold_like_cpp(), + powers: [0; 10], }) } @@ -7349,6 +7352,9 @@ impl WorldSession { base_mana: i32, ) -> bool { self.mutate_canonical_player_like_cpp(|player| { + for raw_power in 0..=25 { + player.set_power_index(power_type_from_u8_like_cpp(raw_power), None); + } player.set_power_index(power_type, Some(0)); player.unit_mut().set_display_power(power_type); player.unit_mut().set_create_mana_like_cpp(base_mana.max(0)); @@ -21630,6 +21636,45 @@ impl WorldSession { let _ = char_db.execute(&stmt).await; } + fn build_character_powers_save_statement_like_cpp( + powers: [i32; 10], + guid_counter: u64, + ) -> PreparedStatement { + let mut stmt = PreparedStatement::new(CharStatements::UPD_CHAR_POWERS.sql()); + for (index, power) in powers.into_iter().enumerate() { + stmt.set_i32(index, power.max(0)); + } + stmt.set_u64(10, guid_counter); + stmt + } + + async fn save_player_powers_like_cpp(&self, snapshot: &PlayerSaveToDbSnapshotLikeCpp) { + let Some(char_db) = self.char_db().map(Arc::clone) else { + return; + }; + let stmt = Self::build_character_powers_save_statement_like_cpp( + snapshot.powers, + snapshot.guid.counter() as u64, + ); + match char_db.execute(&stmt).await { + Ok(_) => { + if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { + info!( + guid = ?snapshot.guid, + powers = ?snapshot.powers, + "RUST_PLAYER_POWER_SAVE" + ); + } + } + Err(err) => { + warn!( + "Failed to save represented player powers for guid {}: {err}", + snapshot.guid.counter() + ); + } + } + } + async fn save_player_level_xp_like_cpp(&self) { let (Some(guid), Some(char_db)) = (self.player_guid(), self.char_db().map(Arc::clone)) else { @@ -21802,6 +21847,7 @@ impl WorldSession { self.save_player_position_like_cpp(&snapshot).await; self.save_player_level_xp_like_cpp().await; self.save_player_gold().await; + self.save_player_powers_like_cpp(&snapshot).await; self.save_player_talent_reset_state_like_cpp().await; self.save_player_explored_zones_like_cpp().await; self.save_player_skills_like_cpp().await; @@ -48945,6 +48991,12 @@ impl WorldSession { return Ok(()); } + if metadata.from_client + && !self.take_spell_power_like_cpp(&spell_info, cast_id, spell_id, &spell_visual) + { + return Ok(()); + } + // Send SMSG_SPELL_GO use wow_packet::packets::spell::SpellGoPkt; @@ -98899,6 +98951,8 @@ mod tests { player.unit_mut().set_level(42); player.set_xp(1234); player.set_money(5678); + player.unit_mut().set_max_power(PowerType::Mana, 1000); + player.unit_mut().set_power(PowerType::Mana, 321); }) .unwrap(); session.set_player_position_like_cpp(latest_session_position); @@ -98916,6 +98970,7 @@ mod tests { level: 42, xp: 1234, money: 5678, + powers: [321, 0, 0, 0, 0, 0, 0, 0, 0, 0], } ); assert_eq!( @@ -98971,6 +99026,7 @@ mod tests { assert_eq!(snapshot.level, 42); assert_eq!(snapshot.xp, 1234); assert_eq!(snapshot.money, 5678); + assert_eq!(snapshot.powers, [0; 10]); } #[test] @@ -99068,6 +99124,29 @@ mod tests { ); } + #[test] + fn character_powers_save_statement_matches_cpp_power_fields_like_cpp() { + let guid = ObjectGuid::create_player(1, 5004); + let powers = [321, -1, 0, 100, 5, 6, 7, 8, 9, 10]; + + let stmt = WorldSession::build_character_powers_save_statement_like_cpp( + powers, + guid.counter() as u64, + ); + + assert_eq!(stmt.sql(), CharStatements::UPD_CHAR_POWERS.sql()); + assert!(matches!(stmt.params()[0], wow_database::SqlParam::I32(321))); + assert!( + matches!(stmt.params()[1], wow_database::SqlParam::I32(0)), + "C++ power fields are unsigned in practice; represented save clamps stale negatives" + ); + assert!(matches!(stmt.params()[3], wow_database::SqlParam::I32(100))); + assert!(matches!(stmt.params()[9], wow_database::SqlParam::I32(10))); + assert!( + matches!(stmt.params()[10], wow_database::SqlParam::U64(v) if v == guid.counter() as u64) + ); + } + #[test] fn character_position_save_uses_captured_snapshot_map_instance_like_cpp() { let guid = ObjectGuid::create_player(1, 5003); @@ -99079,6 +99158,7 @@ mod tests { level: 80, xp: 0, money: 42, + powers: [0; 10], }; let stmt = WorldSession::build_character_position_save_statement_from_snapshot_like_cpp( @@ -99236,6 +99316,62 @@ mod tests { ); } + #[test] + fn sync_canonical_player_primary_power_clears_stale_mana_index_like_cpp() { + let (mut session, _, _) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 0xE103); + session.set_canonical_map_manager(Arc::clone(&canonical)); + + session.ensure_login_player_controller_like_cpp( + player_guid, + "Warrior".to_string(), + Position::new(1.0, 2.0, 3.0, 0.0), + 1, + 1, + 1, + 80, + 0, + ); + let player = session + .canonical_player_entity_snapshot_like_cpp() + .expect("canonical player snapshot"); + { + let mut manager = canonical.lock().expect("canonical map manager"); + let managed = manager.create_world_map(1, 0); + session.sync_canonical_player_entity_like_cpp(managed, player); + } + + assert!(session.sync_canonical_player_primary_power_like_cpp( + PowerType::Rage, + 500, + 1_000, + 0, + )); + + let (mana_index, rage_index, mana, rage, rage_max, create_mana) = session + .mutate_canonical_player_like_cpp(|player| { + ( + player.get_power_index(PowerType::Mana), + player.get_power_index(PowerType::Rage), + player.get_power(PowerType::Mana), + player.get_power(PowerType::Rage), + player.get_max_power(PowerType::Rage), + player.unit().get_create_mana_like_cpp(), + ) + }) + .expect("canonical player"); + assert_eq!( + mana_index, None, + "C++ DB2Manager::GetPowerIndexByClass has no Mana power slot for warrior" + ); + assert_eq!(rage_index, Some(0)); + assert_eq!(mana, 0); + assert_eq!(rage, 500); + assert_eq!(rage_max, 1_000); + assert_eq!(create_mana, 0); + } + #[test] fn update_area_records_enter_leave_area_criteria_like_cpp() { let (mut session, _, _) = make_session(); From 9ca70a227324fa62fae9e93983c5f29bea7dac91 Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 08:15:33 +0000 Subject: [PATCH 2/6] Fix login stat update preserving current power --- crates/wow-data/src/player_stats.rs | 9 ++ crates/wow-world/src/handlers/character.rs | 141 ++++++++++++++++++--- crates/wow-world/src/session.rs | 12 ++ 3 files changed, 147 insertions(+), 15 deletions(-) diff --git a/crates/wow-data/src/player_stats.rs b/crates/wow-data/src/player_stats.rs index 0411ac6f..328466d7 100644 --- a/crates/wow-data/src/player_stats.rs +++ b/crates/wow-data/src/player_stats.rs @@ -253,6 +253,15 @@ impl PlayerStatsStore { self.stats.get(&(race, class, level)) } + /// Build a store from known rows. + pub fn from_entries( + entries: impl IntoIterator, + ) -> Self { + Self { + stats: entries.into_iter().collect(), + } + } + /// Number of entries loaded. pub fn len(&self) -> usize { self.stats.len() diff --git a/crates/wow-world/src/handlers/character.rs b/crates/wow-world/src/handlers/character.rs index f9a6e35b..3affb7a1 100644 --- a/crates/wow-world/src/handlers/character.rs +++ b/crates/wow-world/src/handlers/character.rs @@ -12388,13 +12388,14 @@ impl WorldSession { None } - /// Recalculate all stats from base + gear and send a VALUES update to the client. + /// Recalculate all stats from base + gear. /// - /// Called after equip/desequip changes to gear slots (0-18). - pub(crate) fn send_stat_update(&self) { + /// C++ `Player::UpdateAllStats` updates max power but preserves current power, + /// clamping only when the max drops below current (`Unit::SetMaxPower`). + fn player_stat_changes_like_cpp(&self) -> Option<(ObjectGuid, PlayerStatChanges)> { let player_guid = match self.player_guid() { Some(g) => g, - None => return, + None => return None, }; let race = self.player_race_like_cpp(); @@ -12402,7 +12403,7 @@ impl WorldSession { let level = self.player_level_like_cpp(); if race == 0 || class == 0 || level == 0 { - return; // Not fully logged in yet + return None; // Not fully logged in yet } // Sum gear stat bonuses from equipped items (slots 0-18) @@ -12454,11 +12455,11 @@ impl WorldSession { // Compute total stats from base + gear let store = match self.player_stats() { Some(s) => s.clone(), - None => return, + None => return None, }; let ls = match store.get(race, class, level) { Some(ls) => ls, - None => return, + None => return None, }; let total_str = ls.strength as i32 + gear_stats[0]; @@ -12514,12 +12515,18 @@ impl WorldSession { (0.0, 0.0) }; - // Power for slot 0 (mana/rage/energy/runic) - let power0 = match class { - 1 => 1000, // Warrior: rage - 4 => 100, // Rogue: energy - 6 => 1000, // DK: runic power - _ => max_mana as i32, // Casters: mana + // Power for slot 0 (mana/rage/energy/runic). Keep current power from + // the runtime player and update only the max, like C++ `SetMaxPower`. + let primary_power_type = primary_power_type_for_class_like_cpp(class); + let max_power0 = primary_max_power_for_class_like_cpp(class, max_mana); + let power0 = self + .canonical_player_power_snapshot_like_cpp(primary_power_type) + .map(|(current, _)| current.max(0).min(max_power0)) + .unwrap_or(max_power0); + let base_mana = if primary_power_type == PowerType::Mana { + base_mp.max(0).min(i64::from(i32::MAX)) as i32 + } else { + 0 }; // CombatRatings[32]: copy 25 used indices, rest 0 @@ -12647,14 +12654,14 @@ impl WorldSession { max_health, min_damage: min_d, max_damage: max_d, - base_mana: power0, + base_mana, base_health: max_health as i32, attack_power: melee_ap, ranged_attack_power: ranged_ap, min_ranged_damage: min_rd, max_ranged_damage: max_rd, power0, - max_power0: power0, + max_power0, stats: [total_str, total_agi, total_sta, total_int, total_spi], stat_pos_buff: gear_stats, armor: total_armor, @@ -12687,6 +12694,17 @@ impl WorldSession { mod_spell_power_pct: 1.0, }; + if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { + info!( + guid = ?player_guid, + power_type = ?primary_power_type, + current_power0 = power0, + max_power0, + base_mana, + "RUST_STAT_POWER_UPDATE" + ); + } + debug!( "Stat update for {:?}: HP={} AP={} STR/AGI/STA/INT/SPI={:?} Armor={} SP={} Crit={:.1}% SCrit={:.1}% Dodge={:.1}% Parry={:.1}% Exp={:.1} ManaRegen={:.1}", player_guid, @@ -12703,6 +12721,17 @@ impl WorldSession { spirit_regen ); + Some((player_guid, changes)) + } + + /// Recalculate all stats from base + gear and send a VALUES update to the client. + /// + /// Called after equip/desequip changes to gear slots (0-18). + pub(crate) fn send_stat_update(&self) { + let Some((player_guid, changes)) = self.player_stat_changes_like_cpp() else { + return; + }; + let update = UpdateObject::player_stat_update(player_guid, self.player_map_id_like_cpp(), changes); self.send_packet(&update); @@ -14043,6 +14072,7 @@ mod tests { QUEST_ITEM_DROP_COUNT, QUEST_REWARD_CHOICES_COUNT, QUEST_REWARD_DISPLAY_SPELL_COUNT, QUEST_REWARD_ITEM_COUNT, QUEST_REWARD_REPUTATIONS_COUNT, QuestStore, QuestTemplate, }; + use wow_data::{PlayerLevelStats, PlayerStatsStore}; use wow_database::StatementDef; use wow_entities::EQUIPMENT_SLOT_MAINHAND; use wow_packet::WorldPacket; @@ -14333,6 +14363,51 @@ mod tests { ) } + fn stats_store_with_priest_level80(base_mana: u16, intellect: u16) -> PlayerStatsStore { + PlayerStatsStore::from_entries([( + (1, 5, 80), + PlayerLevelStats { + strength: 10, + agility: 10, + stamina: 10, + intellect, + spirit: 30, + base_health: 100, + base_mana, + }, + )]) + } + + fn attach_stat_update_player_with_mana( + session: &mut WorldSession, + player_guid: ObjectGuid, + current_mana: i32, + max_mana: i32, + ) { + let mut player = wow_entities::Player::new(Some(1), false); + player + .unit_mut() + .world_mut() + .object_mut() + .create(player_guid); + player.unit_mut().world_mut().set_map(571, 0).unwrap(); + player + .unit_mut() + .world_mut() + .relocate(Position::new(10.0, 20.0, 30.0, 0.0)); + player.unit_mut().set_power_index(PowerType::Mana, Some(0)); + player.unit_mut().set_max_power(PowerType::Mana, max_mana); + player.unit_mut().set_power(PowerType::Mana, current_mana); + + let mut manager = wow_map::MapManager::default(); + manager + .create_world_map(571, 0) + .map_mut() + .insert_map_object_record(wow_entities::MapObjectRecord::new_player(player).unwrap()) + .unwrap(); + attach_map_manager(session, manager); + } + fn drain_server_opcodes(send_rx: &flume::Receiver>) -> Vec { let mut opcodes = Vec::new(); while let Ok(bytes) = send_rx.try_recv() { @@ -14344,6 +14419,42 @@ mod tests { opcodes } + #[test] + fn stat_update_preserves_current_mana_like_cpp() { + let (mut session, _send_rx) = make_session_with_send_capacity(1); + let player_guid = ObjectGuid::create_player(1, 77); + session.set_player_guid(Some(player_guid)); + session.set_loaded_player_identity_like_cpp(571, 1, 5, 80, 0); + session.set_player_stats(Arc::new(stats_store_with_priest_level80(1000, 40))); + attach_stat_update_player_with_mana(&mut session, player_guid, 777, 1320); + + let (_, changes) = session + .player_stat_changes_like_cpp() + .expect("stat changes"); + + assert_eq!(changes.power0, 777); + assert_eq!(changes.max_power0, 1320); + assert_eq!(changes.base_mana, 1000); + } + + #[test] + fn stat_update_clamps_current_mana_to_new_max_like_cpp() { + let (mut session, _send_rx) = make_session_with_send_capacity(1); + let player_guid = ObjectGuid::create_player(1, 78); + session.set_player_guid(Some(player_guid)); + session.set_loaded_player_identity_like_cpp(571, 1, 5, 80, 0); + session.set_player_stats(Arc::new(stats_store_with_priest_level80(1000, 40))); + attach_stat_update_player_with_mana(&mut session, player_guid, 2_000, 2_500); + + let (_, changes) = session + .player_stat_changes_like_cpp() + .expect("stat changes"); + + assert_eq!(changes.power0, 1320); + assert_eq!(changes.max_power0, 1320); + assert_eq!(changes.base_mana, 1000); + } + #[tokio::test] async fn show_trade_skill_is_noop_null_like_cpp() { let (mut session, send_rx) = make_session_with_send_capacity(1); diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index 502dce86..5014bf7a 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -18370,6 +18370,18 @@ impl WorldSession { Some(f(player)) } + pub(crate) fn canonical_player_power_snapshot_like_cpp( + &self, + power_type: PowerType, + ) -> Option<(i32, i32)> { + self.canonical_player_snapshot_like_cpp(|player| { + ( + player.unit().get_power(power_type), + player.unit().get_max_power(power_type), + ) + }) + } + pub(crate) fn canonical_player_reputation_standings_snapshot_like_cpp( &self, ) -> Vec<(u32, i32)> { From cae2a4e0ee6c027c0c22564c56c30f48ca400e13 Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 08:42:54 +0000 Subject: [PATCH 3/6] Address spell power review feedback --- crates/wow-world/src/handlers/character.rs | 31 +++++++-- crates/wow-world/src/session.rs | 77 +++++++++++++++++++--- 2 files changed, 91 insertions(+), 17 deletions(-) diff --git a/crates/wow-world/src/handlers/character.rs b/crates/wow-world/src/handlers/character.rs index 3affb7a1..a35cb17b 100644 --- a/crates/wow-world/src/handlers/character.rs +++ b/crates/wow-world/src/handlers/character.rs @@ -12392,7 +12392,7 @@ impl WorldSession { /// /// C++ `Player::UpdateAllStats` updates max power but preserves current power, /// clamping only when the max drops below current (`Unit::SetMaxPower`). - fn player_stat_changes_like_cpp(&self) -> Option<(ObjectGuid, PlayerStatChanges)> { + fn player_stat_changes_like_cpp(&mut self) -> Option<(ObjectGuid, PlayerStatChanges)> { let player_guid = match self.player_guid() { Some(g) => g, None => return None, @@ -12518,16 +12518,25 @@ impl WorldSession { // Power for slot 0 (mana/rage/energy/runic). Keep current power from // the runtime player and update only the max, like C++ `SetMaxPower`. let primary_power_type = primary_power_type_for_class_like_cpp(class); - let max_power0 = primary_max_power_for_class_like_cpp(class, max_mana); - let power0 = self - .canonical_player_power_snapshot_like_cpp(primary_power_type) - .map(|(current, _)| current.max(0).min(max_power0)) - .unwrap_or(max_power0); + let computed_max_power0 = primary_max_power_for_class_like_cpp(class, max_mana); let base_mana = if primary_power_type == PowerType::Mana { base_mp.max(0).min(i64::from(i32::MAX)) as i32 } else { 0 }; + let (power0, max_power0) = self + .sync_canonical_player_primary_power_max_like_cpp( + primary_power_type, + computed_max_power0, + base_mana, + ) + .or_else(|| { + self.canonical_player_power_snapshot_like_cpp(primary_power_type) + .map(|(current, _)| { + (current.max(0).min(computed_max_power0), computed_max_power0) + }) + }) + .unwrap_or((computed_max_power0, computed_max_power0)); // CombatRatings[32]: copy 25 used indices, rest 0 let mut combat_ratings = [0i32; 32]; @@ -12727,7 +12736,7 @@ impl WorldSession { /// Recalculate all stats from base + gear and send a VALUES update to the client. /// /// Called after equip/desequip changes to gear slots (0-18). - pub(crate) fn send_stat_update(&self) { + pub(crate) fn send_stat_update(&mut self) { let Some((player_guid, changes)) = self.player_stat_changes_like_cpp() else { return; }; @@ -14435,6 +14444,10 @@ mod tests { assert_eq!(changes.power0, 777); assert_eq!(changes.max_power0, 1320); assert_eq!(changes.base_mana, 1000); + assert_eq!( + session.canonical_player_power_snapshot_like_cpp(PowerType::Mana), + Some((777, 1320)) + ); } #[test] @@ -14453,6 +14466,10 @@ mod tests { assert_eq!(changes.power0, 1320); assert_eq!(changes.max_power0, 1320); assert_eq!(changes.base_mana, 1000); + assert_eq!( + session.canonical_player_power_snapshot_like_cpp(PowerType::Mana), + Some((1320, 1320)) + ); } #[tokio::test] diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index 5014bf7a..a4d94d60 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -1131,7 +1131,7 @@ pub(crate) struct PlayerSaveToDbSnapshotLikeCpp { pub level: u8, pub xp: u32, pub money: u64, - pub powers: [i32; 10], + pub powers: Option<[i32; 10]>, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -7177,7 +7177,7 @@ impl WorldSession { level: player.unit().data().level.clamp(0, i32::from(u8::MAX)) as u8, xp: player.active_data().xp.max(0) as u32, money: player.active_data().coinage, - powers: player.unit().data().power, + powers: Some(player.unit().data().power), }); }); if snapshot.is_some() { @@ -7206,7 +7206,7 @@ impl WorldSession { level: self.player_level_like_cpp(), xp: self.player_xp_like_cpp(), money: self.player_gold_like_cpp(), - powers: [0; 10], + powers: None, }) } @@ -7364,6 +7364,27 @@ impl WorldSession { .is_some() } + pub(crate) fn sync_canonical_player_primary_power_max_like_cpp( + &mut self, + power_type: PowerType, + max: i32, + base_mana: i32, + ) -> Option<(i32, i32)> { + self.mutate_canonical_player_like_cpp(|player| { + if player.unit().get_power_index(power_type).is_none() { + player.set_power_index(power_type, Some(0)); + } + player.unit_mut().set_display_power(power_type); + player.unit_mut().set_create_mana_like_cpp(base_mana.max(0)); + // C++ `Unit::SetMaxPower` updates max and clamps current if needed. + player.unit_mut().set_max_power(power_type, max.max(0)); + ( + player.unit().get_power(power_type), + player.unit().get_max_power(power_type), + ) + }) + } + pub(crate) fn set_canonical_chosen_title_like_cpp( &mut self, title_id: i32, @@ -21660,14 +21681,30 @@ impl WorldSession { stmt } + fn build_character_powers_save_statement_from_snapshot_like_cpp( + snapshot: &PlayerSaveToDbSnapshotLikeCpp, + ) -> Option { + Some(Self::build_character_powers_save_statement_like_cpp( + snapshot.powers?, + snapshot.guid.counter() as u64, + )) + } + async fn save_player_powers_like_cpp(&self, snapshot: &PlayerSaveToDbSnapshotLikeCpp) { let Some(char_db) = self.char_db().map(Arc::clone) else { return; }; - let stmt = Self::build_character_powers_save_statement_like_cpp( - snapshot.powers, - snapshot.guid.counter() as u64, - ); + let Some(stmt) = + Self::build_character_powers_save_statement_from_snapshot_like_cpp(snapshot) + else { + if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { + info!( + guid = ?snapshot.guid, + "RUST_PLAYER_POWER_SAVE skipped: no authoritative canonical power snapshot" + ); + } + return; + }; match char_db.execute(&stmt).await { Ok(_) => { if std::env::var_os("RUSTYCORE_SPELL_POWER_TRACE").is_some() { @@ -98982,7 +99019,7 @@ mod tests { level: 42, xp: 1234, money: 5678, - powers: [321, 0, 0, 0, 0, 0, 0, 0, 0, 0], + powers: Some([321, 0, 0, 0, 0, 0, 0, 0, 0, 0]), } ); assert_eq!( @@ -99038,7 +99075,7 @@ mod tests { assert_eq!(snapshot.level, 42); assert_eq!(snapshot.xp, 1234); assert_eq!(snapshot.money, 5678); - assert_eq!(snapshot.powers, [0; 10]); + assert_eq!(snapshot.powers, None); } #[test] @@ -99159,6 +99196,26 @@ mod tests { ); } + #[test] + fn character_powers_save_statement_skips_missing_authoritative_powers_like_cpp() { + let snapshot = PlayerSaveToDbSnapshotLikeCpp { + guid: ObjectGuid::create_player(1, 5005), + map_id: 571, + instance_id: 0, + position: Position::new(11.0, 22.0, 33.0, 1.5), + level: 80, + xp: 0, + money: 42, + powers: None, + }; + + assert!( + WorldSession::build_character_powers_save_statement_from_snapshot_like_cpp(&snapshot) + .is_none(), + "fallback snapshots preserve old DB powers by skipping the power update" + ); + } + #[test] fn character_position_save_uses_captured_snapshot_map_instance_like_cpp() { let guid = ObjectGuid::create_player(1, 5003); @@ -99170,7 +99227,7 @@ mod tests { level: 80, xp: 0, money: 42, - powers: [0; 10], + powers: None, }; let stmt = WorldSession::build_character_position_save_statement_from_snapshot_like_cpp( From 654c29f040a90bd890d7faa1c2cfd91ce93ce2d7 Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 09:50:57 +0000 Subject: [PATCH 4/6] Preserve represented player power snapshots --- crates/wow-world/src/handlers/character.rs | 30 +++- crates/wow-world/src/handlers/spell.rs | 65 +++++++++ crates/wow-world/src/session.rs | 157 ++++++++++++++++++--- 3 files changed, 228 insertions(+), 24 deletions(-) diff --git a/crates/wow-world/src/handlers/character.rs b/crates/wow-world/src/handlers/character.rs index a35cb17b..20e53078 100644 --- a/crates/wow-world/src/handlers/character.rs +++ b/crates/wow-world/src/handlers/character.rs @@ -1331,6 +1331,13 @@ fn default_health_mana(class: u8) -> (u32, u32) { } } +fn default_character_power1_like_cpp(class: u8, mana: u32) -> u32 { + match primary_power_type_for_class_like_cpp(class) { + PowerType::Energy => 100, + _ => mana, + } +} + /// Maximum characters per account. const MAX_CHARACTERS_PER_ACCOUNT: u32 = 10; @@ -2748,6 +2755,7 @@ impl WorldSession { // Default health/power for a fresh level 1 character let (health, mana) = default_health_mana(pkt.class); + let power1 = default_character_power1_like_cpp(pkt.class, mana); let create_time = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -2808,7 +2816,7 @@ impl WorldSession { ins_stmt.set_i32(50, 0); // watchedFaction ins_stmt.set_u8(51, 0); // drunk ins_stmt.set_u32(52, health); // health - ins_stmt.set_u32(53, mana); // power1 + ins_stmt.set_u32(53, power1); // power1 ins_stmt.set_u32(54, 0); // power2 ins_stmt.set_u32(55, 0); // power3 ins_stmt.set_u32(56, 0); // power4 @@ -5494,7 +5502,14 @@ impl WorldSession { ([0i32; 5], 0, 0, 0, 0) }; - let current_power0 = result.try_read::(52).unwrap_or(0).min(i32::MAX as u32) as i32; + let loaded_powers = std::array::from_fn(|index| { + result + .try_read::(52 + index) + .unwrap_or(0) + .min(i32::MAX as u32) as i32 + }); + self.set_loaded_player_powers_like_cpp(loaded_powers); + let current_power0 = loaded_powers[0]; // Compute real stats from player_levelstats + gear bonuses let (combat, base_mana_like_cpp) = if let Some(store) = self.player_stats() { @@ -14678,6 +14693,17 @@ mod tests { ); } + #[test] + fn default_character_power1_seeds_energy_classes_like_cpp() { + assert_eq!( + default_character_power1_like_cpp(4, 0), + 100, + "C++ level-1 rogues enter with full base Energy, not zeroed mana" + ); + assert_eq!(default_character_power1_like_cpp(5, 160), 160); + assert_eq!(default_character_power1_like_cpp(1, 0), 0); + } + #[test] fn character_rename_name_validation_matches_represented_cpp_gates() { assert_eq!( diff --git a/crates/wow-world/src/handlers/spell.rs b/crates/wow-world/src/handlers/spell.rs index c847421f..0eb6feac 100644 --- a/crates/wow-world/src/handlers/spell.rs +++ b/crates/wow-world/src/handlers/spell.rs @@ -405,6 +405,7 @@ impl WorldSession { let power_type = PowerType::from_i8(cost.power_type)?; Some(( cost.power_type, + player.unit().get_power_index(power_type), player.get_power(power_type), player.get_max_power(power_type), )) @@ -413,6 +414,11 @@ impl WorldSession { (player.values_update(true), after_power) }); if let Some((update, after_power)) = update { + for (_, slot, current, max) in &after_power { + if let Some(slot) = slot { + self.set_represented_player_power_slot_like_cpp(*slot, *current, Some(*max)); + } + } if trace_spell_power { info!( "RUST_SPELL_POWER_COST phase=take spell_id={} cast_id={:?} result=deducted costs={:?} before_power={:?} after_power={:?}", @@ -4810,6 +4816,65 @@ mod tests { ); } + #[tokio::test] + async fn cast_spell_power_spend_survives_canonical_resync_like_cpp() { + let (mut session, send_rx) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 42); + let spell_id = 13_356; + install_canonical_player(&mut session, &canonical, player_guid); + session.set_map_store(Arc::new(wow_data::MapStore::from_entries([ + wow_data::MapEntry { + id: 571, + instance_type: wow_data::map::MAP_COMMON, + expansion_id: 0, + parent_map_id: -1, + cosmetic_parent_map_id: -1, + flags1: 0, + flags2: 0, + }, + ]))); + session.set_loaded_player_identity_like_cpp(571, 1, 5, 80, 0); + session.set_loaded_player_powers_like_cpp([500, 222, 0, 0, 0, 0, 0, 0, 0, 0]); + assert!(session.sync_canonical_player_primary_power_like_cpp( + PowerType::Mana, + 500, + 1_000, + 1_000, + )); + session.set_known_spells_like_cpp(vec![spell_id]); + session.set_spell_store(spell_store_with_mana_power_cost_like_cpp( + spell_id, 50, 10.0, + )); + + session + .handle_cast_spell(cast_spell_packet(spell_id, player_guid)) + .await; + + assert_eq!(canonical_player_mana_like_cpp(&mut session), 350); + assert_eq!( + session.represented_player_power_values_like_cpp().unwrap()[0], + 350, + "represented session power must mirror Spell::TakePower before later AddToMap-style resync" + ); + let _ = session.ensure_canonical_world_map_for_current_player_like_cpp(); + assert_eq!( + canonical_player_mana_like_cpp(&mut session), + 350, + "resync from the session snapshot must not resurrect pre-cast mana" + ); + let snapshot = session + .current_player_save_to_db_snapshot_like_cpp() + .unwrap(); + assert_eq!(snapshot.powers[0], Some(350)); + assert_eq!( + snapshot.powers[1], + Some(222), + "saving power1 after a cast must preserve other loaded character power columns" + ); + let _ = drain_server_opcodes(&send_rx); + } + #[tokio::test] async fn cast_spell_with_insufficient_mana_fails_no_power_like_cpp() { let (mut session, send_rx) = make_session(); diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index a4d94d60..37aa5965 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -145,7 +145,7 @@ use wow_entities::{ ITEM_DATA_CONTAINED_IN_BIT, ITEM_DATA_DURABILITY_BIT, Item, ItemCreateInfo, ItemDataUpdate, ItemLimitCategoryTemplate, ItemPosCount, ItemSlotRef, ItemStorageRef, ItemStorageTemplate, ItemValuesUpdate, MAX_BAG_SIZE, MAX_ITEM_SPELLS, MAX_MONEY_AMOUNT, MAX_POWERS, - MovementGeneratorKind, MovementSlot, NULL_BAG, NULL_SLOT, ObjectAccessor, + MAX_POWERS_PER_CLASS, MovementGeneratorKind, MovementSlot, NULL_BAG, NULL_SLOT, ObjectAccessor, PLAYER_EXPLORED_ZONES_SIZE_LIKE_CPP, PLAYER_SLOT_END, Pet, PetAuraLikeCpp, PetDeclinedNamesLikeCpp, PetSaveMode, PetSpellState, PetSpellType, PetStable, PetStableInfo, PetType, PhaseShift, Player, PlayerEnchantTimeUpdate, PlayerInventoryStorage, @@ -427,6 +427,15 @@ const fn power_type_from_u8_like_cpp(power: u8) -> PowerType { } } +const fn primary_power_type_for_player_class_like_cpp(class_id: u8) -> PowerType { + match class_id { + 1 => PowerType::Rage, + 4 => PowerType::Energy, + 6 => PowerType::RunicPower, + _ => PowerType::Mana, + } +} + const fn unit_stand_state_from_u8_like_cpp(value: u8) -> UnitStandStateType { match value { 1 => UnitStandStateType::Sit, @@ -1122,6 +1131,28 @@ pub(crate) struct RepresentedLootRollState { pub voters: HashMap, } +pub(crate) type CharacterPowerSnapshotLikeCpp = [Option; MAX_POWERS_PER_CLASS]; + +fn empty_character_power_snapshot_like_cpp() -> CharacterPowerSnapshotLikeCpp { + [None; MAX_POWERS_PER_CLASS] +} + +fn loaded_character_power_snapshot_like_cpp( + powers: [i32; MAX_POWERS_PER_CLASS], +) -> CharacterPowerSnapshotLikeCpp { + powers.map(|power| Some(power.max(0))) +} + +fn character_power_snapshot_values_like_cpp( + powers: &CharacterPowerSnapshotLikeCpp, +) -> Option<[i32; MAX_POWERS_PER_CLASS]> { + let mut values = [0; MAX_POWERS_PER_CLASS]; + for (index, power) in powers.iter().copied().enumerate() { + values[index] = power?; + } + Some(values) +} + #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) struct PlayerSaveToDbSnapshotLikeCpp { pub guid: ObjectGuid, @@ -1131,7 +1162,7 @@ pub(crate) struct PlayerSaveToDbSnapshotLikeCpp { pub level: u8, pub xp: u32, pub money: u64, - pub powers: Option<[i32; 10]>, + pub powers: CharacterPowerSnapshotLikeCpp, } #[derive(Debug, Clone, Copy, PartialEq)] @@ -4034,6 +4065,12 @@ pub struct WorldSession { player_bank_bag_slot_count_like_cpp: u8, /// C++ `UF::ActivePlayerData::CharacterPoints`, recalculated by InitTalentForLevel/LearnTalent. player_character_points_like_cpp: i32, + /// Current `characters.power1..power10` values loaded from DB or mutated by represented runtime. + represented_player_powers_like_cpp: CharacterPowerSnapshotLikeCpp, + /// Max powers known by represented runtime for canonical player rebuilds. + represented_player_max_powers_like_cpp: CharacterPowerSnapshotLikeCpp, + /// C++ `UnitData::BaseMana` known by represented runtime. + represented_player_base_mana_like_cpp: i32, represented_bank_bag_slot_flags_like_cpp: [u32; 7], represented_current_banker_guid_like_cpp: Option, represented_bank_item_moves_like_cpp: Vec, @@ -5888,6 +5925,9 @@ impl WorldSession { represented_talent_reset_time_secs_like_cpp: 0, player_bank_bag_slot_count_like_cpp: 0, player_character_points_like_cpp: 0, + represented_player_powers_like_cpp: empty_character_power_snapshot_like_cpp(), + represented_player_max_powers_like_cpp: empty_character_power_snapshot_like_cpp(), + represented_player_base_mana_like_cpp: 0, represented_bank_bag_slot_flags_like_cpp: [0; 7], represented_current_banker_guid_like_cpp: None, represented_bank_item_moves_like_cpp: Vec::new(), @@ -7043,6 +7083,7 @@ impl WorldSession { player .unit_mut() .set_health(u64::from(self.player_health_like_cpp)); + self.apply_represented_player_powers_to_canonical_like_cpp(&mut player); player.set_xp(self.player_xp_like_cpp() as i32); player.set_next_level_xp(self.player_next_level_xp_like_cpp() as i32); player.set_money(self.player_gold_like_cpp()); @@ -7081,6 +7122,26 @@ impl WorldSession { Some(player) } + fn apply_represented_player_powers_to_canonical_like_cpp(&self, player: &mut Player) { + let Some(current) = self.represented_player_powers_like_cpp[0] else { + return; + }; + let primary_power_type = + primary_power_type_for_player_class_like_cpp(self.player_class_like_cpp()); + for raw_power in 0..=25 { + player.set_power_index(power_type_from_u8_like_cpp(raw_power), None); + } + player.set_power_index(primary_power_type, Some(0)); + player.unit_mut().set_display_power(primary_power_type); + player + .unit_mut() + .set_create_mana_like_cpp(self.represented_player_base_mana_like_cpp.max(0)); + if let Some(max) = self.represented_player_max_powers_like_cpp[0] { + player.unit_mut().set_max_power(primary_power_type, max); + player.unit_mut().set_power(primary_power_type, current); + } + } + fn apply_represented_player_unit_shape_to_canonical_like_cpp(&self, player: &mut Player) { let display_id = crate::handlers::character::default_display_id( self.player_race_like_cpp(), @@ -7169,6 +7230,15 @@ impl WorldSession { ) }; + let mut powers = self.represented_player_powers_like_cpp; + let primary_power_type = + primary_power_type_for_player_class_like_cpp(self.player_class_like_cpp()); + if let Some(primary_slot) = player.unit().get_power_index(primary_power_type) + && primary_slot < MAX_POWERS_PER_CLASS + { + powers[primary_slot] = Some(player.unit().get_power(primary_power_type).max(0)); + } + snapshot = Some(PlayerSaveToDbSnapshotLikeCpp { guid, map_id, @@ -7177,7 +7247,7 @@ impl WorldSession { level: player.unit().data().level.clamp(0, i32::from(u8::MAX)) as u8, xp: player.active_data().xp.max(0) as u32, money: player.active_data().coinage, - powers: Some(player.unit().data().power), + powers, }); }); if snapshot.is_some() { @@ -7206,7 +7276,7 @@ impl WorldSession { level: self.player_level_like_cpp(), xp: self.player_xp_like_cpp(), money: self.player_gold_like_cpp(), - powers: None, + powers: self.represented_player_powers_like_cpp, }) } @@ -7228,6 +7298,7 @@ impl WorldSession { self.set_player_level_like_cpp(snapshot.level); self.set_player_xp_like_cpp(snapshot.xp); self.set_player_gold_like_cpp(snapshot.money); + self.represented_player_powers_like_cpp = snapshot.powers; Some(snapshot) } @@ -7351,17 +7422,23 @@ impl WorldSession { max: i32, base_mana: i32, ) -> bool { - self.mutate_canonical_player_like_cpp(|player| { - for raw_power in 0..=25 { - player.set_power_index(power_type_from_u8_like_cpp(raw_power), None); - } - player.set_power_index(power_type, Some(0)); - player.unit_mut().set_display_power(power_type); - player.unit_mut().set_create_mana_like_cpp(base_mana.max(0)); - player.unit_mut().set_max_power(power_type, max.max(0)); - player.unit_mut().set_power(power_type, current.max(0)); - }) - .is_some() + let synced = self + .mutate_canonical_player_like_cpp(|player| { + for raw_power in 0..=25 { + player.set_power_index(power_type_from_u8_like_cpp(raw_power), None); + } + player.set_power_index(power_type, Some(0)); + player.unit_mut().set_display_power(power_type); + player.unit_mut().set_create_mana_like_cpp(base_mana.max(0)); + player.unit_mut().set_max_power(power_type, max.max(0)); + player.unit_mut().set_power(power_type, current.max(0)); + }) + .is_some(); + if synced { + self.represented_player_base_mana_like_cpp = base_mana.max(0); + self.set_represented_player_power_slot_like_cpp(0, current, Some(max)); + } + synced } pub(crate) fn sync_canonical_player_primary_power_max_like_cpp( @@ -7370,7 +7447,7 @@ impl WorldSession { max: i32, base_mana: i32, ) -> Option<(i32, i32)> { - self.mutate_canonical_player_like_cpp(|player| { + let result = self.mutate_canonical_player_like_cpp(|player| { if player.unit().get_power_index(power_type).is_none() { player.set_power_index(power_type, Some(0)); } @@ -7382,7 +7459,12 @@ impl WorldSession { player.unit().get_power(power_type), player.unit().get_max_power(power_type), ) - }) + }); + if let Some((current, max)) = result { + self.represented_player_base_mana_like_cpp = base_mana.max(0); + self.set_represented_player_power_slot_like_cpp(0, current, Some(max)); + } + result } pub(crate) fn set_canonical_chosen_title_like_cpp( @@ -21685,7 +21767,7 @@ impl WorldSession { snapshot: &PlayerSaveToDbSnapshotLikeCpp, ) -> Option { Some(Self::build_character_powers_save_statement_like_cpp( - snapshot.powers?, + character_power_snapshot_values_like_cpp(&snapshot.powers)?, snapshot.guid.counter() as u64, )) } @@ -31731,6 +31813,34 @@ impl WorldSession { self.refresh_represented_talent_points_like_cpp(); } + pub(crate) fn set_loaded_player_powers_like_cpp( + &mut self, + powers: [i32; MAX_POWERS_PER_CLASS], + ) { + self.represented_player_powers_like_cpp = loaded_character_power_snapshot_like_cpp(powers); + } + + pub(crate) fn set_represented_player_power_slot_like_cpp( + &mut self, + slot: usize, + current: i32, + max: Option, + ) { + if slot >= MAX_POWERS_PER_CLASS { + return; + } + self.represented_player_powers_like_cpp[slot] = Some(current.max(0)); + if let Some(max) = max { + self.represented_player_max_powers_like_cpp[slot] = Some(max.max(0)); + } + } + + pub(crate) fn represented_player_power_values_like_cpp( + &self, + ) -> Option<[i32; MAX_POWERS_PER_CLASS]> { + character_power_snapshot_values_like_cpp(&self.represented_player_powers_like_cpp) + } + pub(crate) fn attach_player_controller_like_cpp( &mut self, mut controller: SessionPlayerController, @@ -98988,6 +99098,7 @@ mod tests { 10, 0, )); + session.set_loaded_player_powers_like_cpp([111, 222, 0, 0, 0, 0, 0, 0, 0, 0]); session.set_player_xp_like_cpp(1); session.set_player_gold_like_cpp(2); let _ = session.ensure_canonical_world_map_for_current_player_like_cpp(); @@ -99019,7 +99130,9 @@ mod tests { level: 42, xp: 1234, money: 5678, - powers: Some([321, 0, 0, 0, 0, 0, 0, 0, 0, 0]), + powers: loaded_character_power_snapshot_like_cpp([ + 321, 222, 0, 0, 0, 0, 0, 0, 0, 0, + ]), } ); assert_eq!( @@ -99075,7 +99188,7 @@ mod tests { assert_eq!(snapshot.level, 42); assert_eq!(snapshot.xp, 1234); assert_eq!(snapshot.money, 5678); - assert_eq!(snapshot.powers, None); + assert_eq!(snapshot.powers, empty_character_power_snapshot_like_cpp()); } #[test] @@ -99206,7 +99319,7 @@ mod tests { level: 80, xp: 0, money: 42, - powers: None, + powers: empty_character_power_snapshot_like_cpp(), }; assert!( @@ -99227,7 +99340,7 @@ mod tests { level: 80, xp: 0, money: 42, - powers: None, + powers: empty_character_power_snapshot_like_cpp(), }; let stmt = WorldSession::build_character_position_save_statement_from_snapshot_like_cpp( From d0ea708651567d727ac91f36aba6823a89952395 Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 10:13:40 +0000 Subject: [PATCH 5/6] Require Codex reviewer verdict before merge --- .github/workflows/codex-review-gate.yml | 154 ++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 .github/workflows/codex-review-gate.yml diff --git a/.github/workflows/codex-review-gate.yml b/.github/workflows/codex-review-gate.yml new file mode 100644 index 00000000..292567fd --- /dev/null +++ b/.github/workflows/codex-review-gate.yml @@ -0,0 +1,154 @@ +name: Codex Review Gate + +on: + pull_request: + branches: + - "3.4.3" + types: + - opened + - synchronize + - reopened + - ready_for_review + +permissions: + contents: read + issues: read + pull-requests: read + +jobs: + codex-review-verdict: + name: Codex reviewer verdict + runs-on: ubuntu-24.04 + steps: + - name: Require clean Codex review on current HEAD + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + CODEX_REVIEW_TIMEOUT_SECONDS: "1800" + CODEX_REVIEW_POLL_SECONDS: "30" + run: | + python3 <<'PY' + import json + import os + import re + import subprocess + import sys + import time + + repo = os.environ["REPOSITORY"] + pr_number = os.environ["PR_NUMBER"] + head_sha = os.environ["HEAD_SHA"].lower() + timeout_seconds = int(os.environ.get("CODEX_REVIEW_TIMEOUT_SECONDS", "1800")) + poll_seconds = int(os.environ.get("CODEX_REVIEW_POLL_SECONDS", "30")) + + codex_logins = {"chatgpt-codex-connector", "chatgpt-codex-connector[bot]"} + positive_markers = ( + "didn't find any major issues", + "did not find any major issues", + "no major issues", + ) + negative_markers = ( + "automated review suggestions", + "p0 badge", + "p1 badge", + "p2 badge", + "p3 badge", + ) + + def gh_json(path): + output = subprocess.check_output(["gh", "api", path], text=True) + return json.loads(output) + + def reviewed_commits(body, fallback_commit=None): + commits = [ + commit.lower() + for commit in re.findall( + r"Reviewed commit:[*\s]*`?([0-9a-fA-F]{7,40})`?", + body or "", + ) + ] + if fallback_commit: + commits.append(fallback_commit.lower()) + return commits + + def is_current_head(commit): + return bool(commit) and (head_sha.startswith(commit) or commit.startswith(head_sha)) + + def codex_items_for_current_head(): + comments = gh_json(f"repos/{repo}/issues/{pr_number}/comments?per_page=100") + reviews = gh_json(f"repos/{repo}/pulls/{pr_number}/reviews?per_page=100") + + items = [] + for item in comments: + if item.get("user", {}).get("login") not in codex_logins: + continue + body = item.get("body") or "" + if any(is_current_head(commit) for commit in reviewed_commits(body)): + items.append(("comment", body, item.get("html_url", ""))) + + for item in reviews: + if item.get("user", {}).get("login") not in codex_logins: + continue + body = item.get("body") or "" + fallback_commit = item.get("commit_id") + if any( + is_current_head(commit) + for commit in reviewed_commits(body, fallback_commit) + ): + url = item.get("html_url") or item.get("_links", {}).get("html", {}).get("href", "") + items.append(("review", body, url)) + + return items + + def verdict(): + items = codex_items_for_current_head() + positives = [] + negatives = [] + + for kind, body, url in items: + lower_body = body.lower() + if any(marker in lower_body for marker in positive_markers): + positives.append((kind, url)) + elif any(marker in lower_body for marker in negative_markers): + negatives.append((kind, url)) + + if positives: + return "pass", positives + if negatives: + return "fail", negatives + return "wait", items + + print(f"Waiting for clean Codex review on PR #{pr_number} at {head_sha[:10]}") + print("Comment '@codex review' on the PR if the review has not started.") + deadline = time.time() + timeout_seconds + + while True: + state, items = verdict() + if state == "pass": + print("Found clean Codex review for current HEAD:") + for kind, url in items: + print(f"- {kind}: {url}") + sys.exit(0) + + if state == "fail": + print("Codex left review feedback for current HEAD; address it before merge:") + for kind, url in items: + print(f"- {kind}: {url}") + sys.exit(1) + + remaining = int(deadline - time.time()) + if remaining <= 0: + print( + "Timed out waiting for a clean Codex review on the current HEAD. " + "Comment '@codex review' on the PR, then rerun this job after Codex responds." + ) + sys.exit(1) + + print( + f"No clean Codex verdict for {head_sha[:10]} yet; " + f"checking again in {poll_seconds}s ({remaining}s left)." + ) + time.sleep(poll_seconds) + PY From 8bc730c492f60f3e6d460fe75b8a28baf375db59 Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 10:37:11 +0000 Subject: [PATCH 6/6] Address Codex review gate feedback --- .github/workflows/codex-review-gate.yml | 63 ++++++++++++++++++------- crates/wow-world/src/handlers/misc.rs | 1 + crates/wow-world/src/handlers/spell.rs | 43 +++++++++++++++++ crates/wow-world/src/session.rs | 13 ++++- 4 files changed, 101 insertions(+), 19 deletions(-) diff --git a/.github/workflows/codex-review-gate.yml b/.github/workflows/codex-review-gate.yml index 292567fd..6ab9e7ec 100644 --- a/.github/workflows/codex-review-gate.yml +++ b/.github/workflows/codex-review-gate.yml @@ -61,6 +61,19 @@ jobs: output = subprocess.check_output(["gh", "api", path], text=True) return json.loads(output) + def gh_json_pages(path): + page = 1 + items = [] + while True: + separator = "&" if "?" in path else "?" + page_items = gh_json(f"{path}{separator}per_page=100&page={page}") + if not isinstance(page_items, list): + raise TypeError(f"Expected paginated list from gh api {path}") + items.extend(page_items) + if len(page_items) < 100: + return items + page += 1 + def reviewed_commits(body, fallback_commit=None): commits = [ commit.lower() @@ -77,8 +90,8 @@ jobs: return bool(commit) and (head_sha.startswith(commit) or commit.startswith(head_sha)) def codex_items_for_current_head(): - comments = gh_json(f"repos/{repo}/issues/{pr_number}/comments?per_page=100") - reviews = gh_json(f"repos/{repo}/pulls/{pr_number}/reviews?per_page=100") + comments = gh_json_pages(f"repos/{repo}/issues/{pr_number}/comments") + reviews = gh_json_pages(f"repos/{repo}/pulls/{pr_number}/reviews") items = [] for item in comments: @@ -86,7 +99,12 @@ jobs: continue body = item.get("body") or "" if any(is_current_head(commit) for commit in reviewed_commits(body)): - items.append(("comment", body, item.get("html_url", ""))) + items.append(( + "comment", + body, + item.get("html_url", ""), + item.get("created_at", ""), + )) for item in reviews: if item.get("user", {}).get("login") not in codex_logins: @@ -98,26 +116,35 @@ jobs: for commit in reviewed_commits(body, fallback_commit) ): url = item.get("html_url") or item.get("_links", {}).get("html", {}).get("href", "") - items.append(("review", body, url)) + items.append(( + "review", + body, + url, + item.get("submitted_at") or item.get("updated_at") or "", + )) return items + def classify_codex_body(body): + lower_body = body.lower() + if any(marker in lower_body for marker in negative_markers): + return "fail" + if any(marker in lower_body for marker in positive_markers): + return "pass" + return None + def verdict(): items = codex_items_for_current_head() - positives = [] - negatives = [] - - for kind, body, url in items: - lower_body = body.lower() - if any(marker in lower_body for marker in positive_markers): - positives.append((kind, url)) - elif any(marker in lower_body for marker in negative_markers): - negatives.append((kind, url)) - - if positives: - return "pass", positives - if negatives: - return "fail", negatives + classified = [] + + for kind, body, url, timestamp in items: + state = classify_codex_body(body) + if state: + classified.append((timestamp, state, kind, url)) + + if classified: + timestamp, state, kind, url = max(classified, key=lambda item: item[0]) + return state, [(kind, url)] return "wait", items print(f"Waiting for clean Codex review on PR #{pr_number} at {head_sha[:10]}") diff --git a/crates/wow-world/src/handlers/misc.rs b/crates/wow-world/src/handlers/misc.rs index 856c7b2f..5232bdf7 100644 --- a/crates/wow-world/src/handlers/misc.rs +++ b/crates/wow-world/src/handlers/misc.rs @@ -3137,6 +3137,7 @@ impl crate::session::WorldSession { cast_flags_ex: CAST_FLAG_EX_USE_TOY_SPELL_LIKE_CPP, original_cast_id: request.cast.cast_id, unit_target_battle_pet_companion_guid: None, + ..SpellCastMetadata::default() }; let mut spell_target = request.cast.target.clone(); diff --git a/crates/wow-world/src/handlers/spell.rs b/crates/wow-world/src/handlers/spell.rs index 0eb6feac..5bd96787 100644 --- a/crates/wow-world/src/handlers/spell.rs +++ b/crates/wow-world/src/handlers/spell.rs @@ -4935,6 +4935,49 @@ mod tests { ); } + #[tokio::test] + async fn timed_spell_late_power_failure_restores_global_cooldown_like_cpp() { + let (mut session, send_rx) = make_session(); + let canonical = shared_canonical_map_manager(); + let player_guid = ObjectGuid::create_player(1, 42); + let cast_id = ObjectGuid::create_world_object(HighGuid::Cast, 0, 1, 0, 0, 1, 22); + let spell_id = 13_357; + install_canonical_player(&mut session, &canonical, player_guid); + set_canonical_player_mana_like_cpp(&mut session, 149, 1000); + session.set_known_spells_like_cpp(vec![spell_id]); + session.set_spell_store(spell_store_with_mana_power_cost_like_cpp( + spell_id, 50, 10.0, + )); + let previous_last_spell_cast_time = + Some(std::time::Instant::now() - std::time::Duration::from_millis(5_000)); + session.last_spell_cast_time = previous_last_spell_cast_time; + install_active_spell_cast(&mut session, spell_id, cast_id); + if let Some(active) = session.active_spell_cast.as_mut() { + active.cast_start_time = + std::time::Instant::now() - std::time::Duration::from_millis(30_000); + active.metadata = SpellCastMetadata { + from_client: true, + original_cast_id: cast_id, + ..SpellCastMetadata::default() + }; + } + + session.tick_active_spell_cast().await; + + assert!(session.active_spell_cast.is_none()); + assert_eq!(canonical_player_mana_like_cpp(&mut session), 149); + assert_eq!( + session.last_spell_cast_time, previous_last_spell_cast_time, + "C++ failed casts do not leave a fresh successful global cooldown" + ); + let packets = drain_server_packet_bytes(&send_rx); + assert_eq!(packets.len(), 1); + assert_eq!( + cast_failed_reason_like_cpp(&packets[0]), + SpellCastResult::NoPower as i32 + ); + } + #[tokio::test] async fn queued_spell_with_mana_cost_deducts_when_executed_like_cpp() { let (mut session, send_rx) = make_session(); diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index 37aa5965..dde71724 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -3555,6 +3555,8 @@ pub struct SpellCastMetadata { pub cast_flags_ex: u32, pub original_cast_id: ObjectGuid, pub unit_target_battle_pet_companion_guid: Option, + pub restore_last_spell_cast_time_on_power_failure: bool, + pub previous_last_spell_cast_time_on_power_failure: Option, } impl Default for SpellCastMetadata { @@ -3567,6 +3569,8 @@ impl Default for SpellCastMetadata { cast_flags_ex: 0, original_cast_id: ObjectGuid::EMPTY, unit_target_battle_pet_companion_guid: None, + restore_last_spell_cast_time_on_power_failure: false, + previous_last_spell_cast_time_on_power_failure: None, } } } @@ -48133,7 +48137,10 @@ impl WorldSession { let target_data = cast_state.target_data.clone(); let cast_id = cast_state.cast_id; let spell_visual = cast_state.spell_visual.clone(); - let metadata = cast_state.metadata; + let mut metadata = cast_state.metadata; + let previous_last_spell_cast_time = self.last_spell_cast_time; + metadata.restore_last_spell_cast_time_on_power_failure = true; + metadata.previous_last_spell_cast_time_on_power_failure = previous_last_spell_cast_time; self.active_spell_cast = None; self.last_spell_cast_time = Some(Instant::now()); @@ -49153,6 +49160,9 @@ impl WorldSession { if metadata.from_client && !self.take_spell_power_like_cpp(&spell_info, cast_id, spell_id, &spell_visual) { + if metadata.restore_last_spell_cast_time_on_power_failure { + self.last_spell_cast_time = metadata.previous_last_spell_cast_time_on_power_failure; + } return Ok(()); } @@ -105062,6 +105072,7 @@ mod tests { cast_flags_ex: CAST_FLAG_EX_USE_TOY_SPELL_LIKE_CPP, original_cast_id: ObjectGuid::EMPTY, unit_target_battle_pet_companion_guid: None, + ..SpellCastMetadata::default() }, ) .await