From 256845ab929c782afaec00f0a1e1efa9c8f0b70e Mon Sep 17 00:00:00 2001 From: cDmonio Date: Wed, 1 Jul 2026 22:14:43 +0000 Subject: [PATCH 1/2] [05] Add represented player save transaction timer --- crates/world-server/src/main.rs | 6 + crates/wow-network/src/accept.rs | 2 + crates/wow-world/src/handlers/character.rs | 40 -- crates/wow-world/src/reputation/mgr.rs | 45 +- crates/wow-world/src/session.rs | 583 +++++++++++++++++++-- docs/migration/EXISTING-CODE-DEFECTS.md | 14 +- docs/migration/STATE.md | 4 +- 7 files changed, 605 insertions(+), 89 deletions(-) diff --git a/crates/world-server/src/main.rs b/crates/world-server/src/main.rs index abc060bb1..f01f17d04 100644 --- a/crates/world-server/src/main.rs +++ b/crates/world-server/src/main.rs @@ -4766,6 +4766,11 @@ async fn main() -> Result { 86_400, ), }, + player_save_interval_ms: world_config_u32( + &world_configs, + "CONFIG_INTERVAL_SAVE", + 15 * 60 * 1000, + ), realm_id, realm_region: active_realm.id.region, realm_battlegroup: active_realm.id.site, @@ -11847,6 +11852,7 @@ async fn create_session( session.set_chat_flood_config_like_cpp(resources.chat_flood_config); session.set_socket_timeouts_like_cpp(resources.socket_timeouts); session.set_packet_spoof_config_like_cpp(resources.packet_spoof_config); + session.set_player_save_interval_ms_like_cpp(resources.player_save_interval_ms); session.set_legacy_creature_aggro_config_like_cpp(legacy_creature_aggro_config); session.set_mmap_runtime_config_like_cpp(mmap_runtime_config); if let Some(pathfinder) = mmap_pathfinder { diff --git a/crates/wow-network/src/accept.rs b/crates/wow-network/src/accept.rs index e8d9d5949..5874310f3 100644 --- a/crates/wow-network/src/accept.rs +++ b/crates/wow-network/src/accept.rs @@ -441,6 +441,8 @@ pub struct SessionResources { pub max_overspeed_pings: u32, pub socket_timeouts: SocketTimeoutsLikeCpp, pub packet_spoof_config: PacketSpoofConfigLikeCpp, + /// C++ `CONFIG_INTERVAL_SAVE` / `PlayerSaveInterval` in milliseconds. + pub player_save_interval_ms: u32, pub realm_id: u16, /// Region from `realmlist.Region`, used in C++ `RealmHandle::GetAddress()`. pub realm_region: u8, diff --git a/crates/wow-world/src/handlers/character.rs b/crates/wow-world/src/handlers/character.rs index a8462756f..dbf5de38d 100644 --- a/crates/wow-world/src/handlers/character.rs +++ b/crates/wow-world/src/handlers/character.rs @@ -3751,46 +3751,6 @@ impl WorldSession { self.send_packet(&LogoutCancelAck); } - /// Save accumulated played time (`totaltime` + `leveltime`) back to the - /// characters database. Called on logout so time is not lost. - pub(crate) async fn save_played_time(&self) { - let guid = match self.player_guid() { - Some(g) => g, - None => return, - }; - - let char_db = match self.char_db() { - Some(db) => Arc::clone(db), - None => return, - }; - - // Compute current total values: base (from DB at login) + session elapsed. - let session_secs: u32 = self - .login_time - .map(|t| t.elapsed().as_secs() as u32) - .unwrap_or(0); - let total_time = self.total_played_time.saturating_add(session_secs); - let level_time = self.level_played_time.saturating_add(session_secs); - - let mut stmt = char_db.prepare(CharStatements::UPD_CHAR_PLAYED_TIME); - stmt.set_u32(0, total_time); - stmt.set_u32(1, level_time); - stmt.set_u32(2, guid.counter() as u32); - if let Err(e) = char_db.execute(&stmt).await { - warn!( - "Failed to save played time for guid {}: {e}", - guid.counter() - ); - } else { - info!( - "Saved played time: total={}s level={}s for guid {}", - total_time, - level_time, - guid.counter() - ); - } - } - /// Mark the current character as offline in the database. pub(crate) async fn mark_character_offline(&self) { let guid = match self.player_guid() { diff --git a/crates/wow-world/src/reputation/mgr.rs b/crates/wow-world/src/reputation/mgr.rs index 9537e387f..b41f0104c 100644 --- a/crates/wow-world/src/reputation/mgr.rs +++ b/crates/wow-world/src/reputation/mgr.rs @@ -584,12 +584,12 @@ impl ReputationMgrLikeCpp { } } - pub fn save_to_db_statement_plan_like_cpp( - &mut self, + pub fn pending_save_to_db_statement_plan_like_cpp( + &self, player_guid_counter: u64, ) -> Vec { let mut statements = Vec::new(); - for faction in self.factions.values_mut() { + for faction in self.factions.values() { if !faction.need_save { continue; } @@ -607,9 +607,24 @@ impl ReputationMgrLikeCpp { insert.set_i32(2, faction.standing); insert.set_u16(3, faction.flags.bits()); statements.push(insert); + } + statements + } - faction.need_save = false; + pub fn mark_pending_save_to_db_committed_like_cpp(&mut self) { + for faction in self.factions.values_mut() { + if faction.need_save { + faction.need_save = false; + } } + } + + pub fn save_to_db_statement_plan_like_cpp( + &mut self, + player_guid_counter: u64, + ) -> Vec { + let statements = self.pending_save_to_db_statement_plan_like_cpp(player_guid_counter); + self.mark_pending_save_to_db_committed_like_cpp(); statements } @@ -1778,6 +1793,28 @@ mod tests { assert!(!mgr.get_state(14).expect("saved state").need_save); } + #[test] + fn pending_save_to_db_statement_plan_like_cpp_does_not_clear_dirty_until_commit() { + let mut mgr = ReputationMgrLikeCpp::new_like_cpp(); + mgr.insert_state_for_test_like_cpp(FactionStateLikeCpp { + standing: 456, + flags: ReputationFlagsLikeCpp::VISIBLE, + ..FactionStateLikeCpp::new_like_cpp(86, 15, ReputationFlagsLikeCpp::VISIBLE) + }); + + let statements = mgr.pending_save_to_db_statement_plan_like_cpp(44); + + assert_eq!(statements.len(), 2); + assert!( + mgr.get_state(15).expect("pending state").need_save, + "failed Player::SaveToDB transaction must leave reputation dirty for retry" + ); + + mgr.mark_pending_save_to_db_committed_like_cpp(); + + assert!(!mgr.get_state(15).expect("committed state").need_save); + } + #[test] fn set_one_faction_reputation_like_cpp_applies_incremental_rate_and_marks_visible_dirty() { let mut faction = FactionEntry::for_test_like_cpp(86, 15); diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index 7c70d3f72..a708fa793 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -206,6 +206,7 @@ const QUEST_OBJECTIVE_MIN_REPUTATION_LIKE_CPP: u8 = 6; const QUEST_OBJECTIVE_MAX_REPUTATION_LIKE_CPP: u8 = 7; const QUEST_OBJECTIVE_MONEY_LIKE_CPP: u8 = 8; const COPPER_PER_GOLD_LIKE_CPP: u32 = 10_000; +const DEFAULT_PLAYER_SAVE_INTERVAL_MS_LIKE_CPP: u32 = 15 * 60 * 1000; const TALENT_RESET_MONTH_SECS_LIKE_CPP: u64 = 30 * 24 * 60 * 60; const QUEST_OBJECTIVE_PLAYERKILLS_LIKE_CPP: u8 = 9; const QUEST_OBJECTIVE_HAVE_CURRENCY_LIKE_CPP: u8 = 16; @@ -1167,6 +1168,15 @@ pub(crate) struct PlayerSaveToDbSnapshotLikeCpp { pub powers: CharacterPowerSnapshotLikeCpp, } +#[derive(Debug, Default)] +struct PlayerSaveToDbStatementPlanLikeCpp { + statements: Vec, + tutorials_insert_committed_like_cpp: bool, + tutorials_changed_committed_like_cpp: bool, + equipment_sets_committed_like_cpp: bool, + reputation_committed_like_cpp: bool, +} + #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum RepresentedGameObjectUseEffect { UseRejectedNoDamageImmune { @@ -4056,6 +4066,12 @@ pub struct WorldSession { pub(crate) logout_time: Option, /// Timestamp set when the player enters the world (PlayerLogin). pub(crate) login_time: Option, + /// C++ `CONFIG_INTERVAL_SAVE` / `PlayerSaveInterval` in milliseconds. + player_save_interval_ms_like_cpp: u32, + /// C++ `Player::m_nextSave` countdown in milliseconds; 0 disables autosave. + next_player_save_ms_like_cpp: u32, + /// Set by the sync update loop when the autosave countdown expires. + pending_periodic_player_save_like_cpp: bool, /// Total played time loaded from DB (seconds). pub(crate) total_played_time: u32, /// Time played at current level loaded from DB (seconds). @@ -5924,6 +5940,9 @@ impl WorldSession { time_sync_clock_delta: 0, logout_time: None, login_time: None, + player_save_interval_ms_like_cpp: DEFAULT_PLAYER_SAVE_INTERVAL_MS_LIKE_CPP, + next_player_save_ms_like_cpp: DEFAULT_PLAYER_SAVE_INTERVAL_MS_LIKE_CPP, + pending_periodic_player_save_like_cpp: false, total_played_time: 0, level_played_time: 0, player_gold: 0, @@ -17201,6 +17220,29 @@ impl WorldSession { self.packet_spoof_config_like_cpp = config; } + pub fn set_player_save_interval_ms_like_cpp(&mut self, interval_ms: u32) { + self.player_save_interval_ms_like_cpp = interval_ms; + self.reset_player_save_timer_like_cpp(); + } + + fn reset_player_save_timer_like_cpp(&mut self) { + self.next_player_save_ms_like_cpp = self.player_save_interval_ms_like_cpp; + self.pending_periodic_player_save_like_cpp = false; + } + + fn update_player_save_timer_like_cpp(&mut self, diff_ms: u32) { + if self.player_save_interval_ms_like_cpp == 0 || self.next_player_save_ms_like_cpp == 0 { + return; + } + + if diff_ms >= self.next_player_save_ms_like_cpp { + self.next_player_save_ms_like_cpp = 0; + self.pending_periodic_player_save_like_cpp = true; + } else { + self.next_player_save_ms_like_cpp -= diff_ms; + } + } + pub fn set_server_expansion_like_cpp(&mut self, expansion: u8) { self.server_expansion_like_cpp = expansion; } @@ -21822,21 +21864,29 @@ impl WorldSession { /// Save current player gold to the characters DB. pub(crate) async fn save_player_gold(&self) { - use wow_database::CharStatements; let guid = match self.player_guid() { - Some(g) => g.counter() as u32, + Some(g) => g.counter() as u64, None => return, }; let char_db = match self.char_db() { Some(db) => Arc::clone(db), None => return, }; - let mut stmt = char_db.prepare(CharStatements::UPD_CHAR_MONEY); - stmt.set_u64(0, self.player_gold_like_cpp()); - stmt.set_u32(1, guid); + let stmt = + Self::build_character_gold_save_statement_like_cpp(self.player_gold_like_cpp(), guid); let _ = char_db.execute(&stmt).await; } + fn build_character_gold_save_statement_like_cpp( + money: u64, + guid_counter: u64, + ) -> PreparedStatement { + let mut stmt = PreparedStatement::new(CharStatements::UPD_CHAR_MONEY.sql()); + stmt.set_u64(0, money); + stmt.set_u64(1, guid_counter); + stmt + } + fn build_character_health_save_statement_like_cpp( health: u32, guid_counter: u64, @@ -21924,15 +21974,28 @@ impl WorldSession { } } + fn build_character_level_xp_save_statement_like_cpp( + level: u8, + xp: u32, + guid_counter: u64, + ) -> PreparedStatement { + let mut stmt = PreparedStatement::new(CharStatements::UPD_CHAR_LEVEL.sql()); + stmt.set_u8(0, level); + stmt.set_u32(1, xp); + stmt.set_u64(2, guid_counter); + stmt + } + 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 { return; }; - let mut stmt = char_db.prepare(CharStatements::UPD_CHAR_LEVEL); - stmt.set_u8(0, self.player_level_like_cpp()); - stmt.set_u32(1, self.player_xp_like_cpp()); - stmt.set_u32(2, guid.counter() as u32); + let stmt = Self::build_character_level_xp_save_statement_like_cpp( + self.player_level_like_cpp(), + self.player_xp_like_cpp(), + guid.counter() as u64, + ); if let Err(err) = char_db.execute(&stmt).await { warn!("Failed to save level/xp for guid {}: {err}", guid.counter()); } @@ -22029,6 +22092,55 @@ impl WorldSession { ) } + fn build_character_difficulties_save_statement_like_cpp( + dungeon_difficulty_id: u32, + raid_difficulty_id: u32, + legacy_raid_difficulty_id: u32, + guid_counter: u64, + ) -> PreparedStatement { + let mut stmt = PreparedStatement::new(CharStatements::UPD_CHAR_DIFFICULTIES.sql()); + stmt.set_u32(0, dungeon_difficulty_id); + stmt.set_u32(1, raid_difficulty_id); + stmt.set_u32(2, legacy_raid_difficulty_id); + stmt.set_u64(3, guid_counter); + stmt + } + + pub(crate) fn current_played_time_values_like_cpp(&self) -> (u32, u32) { + let session_secs: u32 = self + .login_time + .map(|time| time.elapsed().as_secs() as u32) + .unwrap_or(0); + ( + self.total_played_time.saturating_add(session_secs), + self.level_played_time.saturating_add(session_secs), + ) + } + + pub(crate) fn build_character_played_time_save_statement_like_cpp( + total_time: u32, + level_time: u32, + guid_counter: u64, + ) -> PreparedStatement { + let mut stmt = PreparedStatement::new(CharStatements::UPD_CHAR_PLAYED_TIME.sql()); + stmt.set_u32(0, total_time); + stmt.set_u32(1, level_time); + stmt.set_u64(2, guid_counter); + stmt + } + + pub(crate) fn played_time_save_statement_like_cpp( + &self, + guid_counter: u64, + ) -> PreparedStatement { + let (total_time, level_time) = self.current_played_time_values_like_cpp(); + Self::build_character_played_time_save_statement_like_cpp( + total_time, + level_time, + guid_counter, + ) + } + async fn save_player_position_like_cpp(&self, snapshot: &PlayerSaveToDbSnapshotLikeCpp) { let Some(char_db) = self.char_db().map(Arc::clone) else { return; @@ -22082,7 +22194,210 @@ impl WorldSession { } } + fn current_player_save_to_db_statement_plan_like_cpp( + &mut self, + snapshot: &PlayerSaveToDbSnapshotLikeCpp, + now_unix_secs: i64, + ) -> PlayerSaveToDbStatementPlanLikeCpp { + let guid_counter = snapshot.guid.counter() as u64; + let mut plan = PlayerSaveToDbStatementPlanLikeCpp::default(); + + plan.statements.push( + Self::build_character_position_save_statement_from_snapshot_like_cpp( + snapshot, + self.player_zone_id_like_cpp as u32, + ), + ); + plan.statements + .push(Self::build_character_level_xp_save_statement_like_cpp( + self.player_level_like_cpp(), + self.player_xp_like_cpp(), + guid_counter, + )); + plan.statements + .push(Self::build_character_gold_save_statement_like_cpp( + self.player_gold_like_cpp(), + guid_counter, + )); + plan.statements + .push(Self::build_character_health_save_statement_from_snapshot_like_cpp(snapshot)); + if let Some(stmt) = + Self::build_character_powers_save_statement_from_snapshot_like_cpp(snapshot) + { + plan.statements.push(stmt); + } 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" + ); + } + plan.statements.push( + Self::build_character_talent_reset_state_save_statement_like_cpp( + self.represented_talent_reset_cost_like_cpp, + self.represented_talent_reset_time_secs_like_cpp, + guid_counter, + ), + ); + + self.sync_represented_explored_zones_from_canonical_like_cpp(); + plan.statements.push( + Self::build_character_explored_zones_save_statement_like_cpp( + self.represented_explored_zones_db_string_like_cpp(), + guid_counter, + ), + ); + + if self.player_skill_records_loaded_like_cpp() { + plan.statements + .extend(self.character_skill_save_statements_like_cpp(guid_counter)); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player skill save because character_skills were not loaded coherently" + ); + } + + plan.statements + .push(Self::build_character_difficulties_save_statement_like_cpp( + self.represented_dungeon_difficulty_id_like_cpp, + self.represented_raid_difficulty_id_like_cpp, + self.represented_legacy_raid_difficulty_id_like_cpp, + guid_counter, + )); + + if let Some(statements) = self.character_glyph_save_statements_like_cpp(guid_counter) { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player glyph save because character_glyphs was not loaded coherently" + ); + } + + if let Some(statements) = self.character_talent_save_statements_like_cpp(guid_counter) { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player talent save because character_talent was not loaded coherently" + ); + } + + if let Some(statements) = + self.character_spell_cooldown_save_statements_like_cpp(guid_counter, now_unix_secs) + { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player spell cooldown save because character_spell_cooldown was not loaded coherently" + ); + } + + if let Some(statements) = + self.character_spell_charge_save_statements_like_cpp(guid_counter, now_unix_secs) + { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player spell charge save because character_spell_charges was not loaded coherently" + ); + } + + if let Some(statements) = + self.character_action_button_save_statements_like_cpp(guid_counter) + { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented player action-button save because character_action was not loaded coherently" + ); + } + + if let Some(statements) = self.equipment_set_save_statements_like_cpp(guid_counter) { + if !statements.is_empty() { + plan.equipment_sets_committed_like_cpp = true; + plan.statements.extend(statements); + } + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented equipment-set save because character_equipmentsets/character_transmog_outfits were not loaded coherently" + ); + } + + if self.tutorials_changed_like_cpp { + if let Some(stmt) = self.tutorial_save_statement_like_cpp(self.account_id) { + plan.tutorials_insert_committed_like_cpp = !self.tutorials_loaded_from_db_like_cpp; + plan.tutorials_changed_committed_like_cpp = true; + plan.statements.push(stmt); + } else { + warn!( + account = self.account_id, + "Skipping SaveTutorialsData because tutorial data was not loaded coherently" + ); + } + } + + plan.statements + .extend(self.instance_time_restriction_save_statements_like_cpp()); + plan.statements + .push(self.played_time_save_statement_like_cpp(guid_counter)); + + let reputation_statements = self + .reputation_mgr_like_cpp() + .pending_save_to_db_statement_plan_like_cpp(guid_counter); + if !reputation_statements.is_empty() { + plan.reputation_committed_like_cpp = true; + plan.statements.extend(reputation_statements); + } + + if let Some(statements) = self.cuf_profile_save_statements_like_cpp(guid_counter) { + plan.statements.extend(statements); + } else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping represented CUF profile save because character_cuf_profiles was not loaded coherently" + ); + } + + plan + } + + fn mark_current_player_save_to_db_committed_like_cpp( + &mut self, + plan: &PlayerSaveToDbStatementPlanLikeCpp, + ) { + if plan.equipment_sets_committed_like_cpp { + self.mark_equipment_sets_saved_like_cpp(); + } + if plan.tutorials_insert_committed_like_cpp { + self.tutorials_loaded_from_db_like_cpp = true; + } + if plan.tutorials_changed_committed_like_cpp { + self.tutorials_changed_like_cpp = false; + } + if plan.reputation_committed_like_cpp { + self.reputation_mgr_like_cpp_mut() + .mark_pending_save_to_db_committed_like_cpp(); + } + } + pub(crate) async fn save_current_player_to_db_like_cpp(&mut self) { + // C++ `Player::SaveToDB` delays the next autosave for manual, code, and + // autosave callers before it appends statements. + self.reset_player_save_timer_like_cpp(); + let Some(snapshot) = self.sync_session_from_save_to_db_snapshot_like_cpp() else { warn!( account = self.account_id, @@ -22093,26 +22408,45 @@ impl WorldSession { ); return; }; - self.save_player_position_like_cpp(&snapshot).await; - self.save_player_level_xp_like_cpp().await; - self.save_player_gold().await; - self.save_player_health_like_cpp(&snapshot).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; - self.save_player_difficulties_like_cpp().await; - self.save_player_glyphs_like_cpp().await; - self.save_player_talents_like_cpp().await; - self.save_player_spell_cooldowns_like_cpp().await; - self.save_player_spell_charges_like_cpp().await; - self.save_player_action_buttons_like_cpp().await; - self.save_player_equipment_sets_like_cpp().await; - self.save_tutorials_data_like_cpp().await; - self.save_instance_time_restrictions_like_cpp().await; - self.save_played_time().await; - self.save_reputation_to_db_like_cpp().await; - self.save_cuf_profiles_like_cpp().await; + let Some(char_db) = self.char_db().map(Arc::clone) else { + warn!( + account = self.account_id, + player_guid = ?self.player_guid(), + "Skipping Player::SaveToDB represented save because character database is unavailable" + ); + return; + }; + + let mut plan = + self.current_player_save_to_db_statement_plan_like_cpp(&snapshot, unix_now()); + let statement_count = plan.statements.len(); + let mut tx = SqlTransaction::new(); + for statement in std::mem::take(&mut plan.statements) { + tx.append(statement); + } + + match char_db.commit_transaction(tx).await { + Ok(()) => { + self.mark_current_player_save_to_db_committed_like_cpp(&plan); + info!( + guid = snapshot.guid.counter(), + statement_count, + "Player::SaveToDB represented save committed in one CharacterDatabase transaction" + ); + } + Err(err) => warn!( + guid = snapshot.guid.counter(), + statement_count, "Failed to commit Player::SaveToDB represented transaction: {err}" + ), + } + } + + async fn process_pending_periodic_player_save_like_cpp(&mut self) { + if !self.pending_periodic_player_save_like_cpp || self.state != SessionState::LoggedIn { + return; + } + + self.save_current_player_to_db_like_cpp().await; } pub(crate) fn reset_represented_glyphs_like_cpp(&mut self) { @@ -23985,11 +24319,12 @@ impl WorldSession { return; }; - let mut stmt = char_db.prepare(CharStatements::UPD_CHAR_DIFFICULTIES); - stmt.set_u32(0, self.represented_dungeon_difficulty_id_like_cpp); - stmt.set_u32(1, self.represented_raid_difficulty_id_like_cpp); - stmt.set_u32(2, self.represented_legacy_raid_difficulty_id_like_cpp); - stmt.set_u32(3, guid.counter() as u32); + let stmt = Self::build_character_difficulties_save_statement_like_cpp( + self.represented_dungeon_difficulty_id_like_cpp, + self.represented_raid_difficulty_id_like_cpp, + self.represented_legacy_raid_difficulty_id_like_cpp, + guid.counter() as u64, + ); if let Err(err) = char_db.execute(&stmt).await { warn!( "Failed to save player difficulties for guid {}: {err}", @@ -24029,8 +24364,8 @@ impl WorldSession { return; }; let statements = self - .reputation_mgr_like_cpp_mut() - .save_to_db_statement_plan_like_cpp(guid.counter() as u64); + .reputation_mgr_like_cpp() + .pending_save_to_db_statement_plan_like_cpp(guid.counter() as u64); if statements.is_empty() { return; } @@ -24039,11 +24374,14 @@ impl WorldSession { for statement in statements { tx.append(statement); } - if let Err(err) = char_db.commit_transaction(tx).await { - warn!( + match char_db.commit_transaction(tx).await { + Ok(()) => self + .reputation_mgr_like_cpp_mut() + .mark_pending_save_to_db_committed_like_cpp(), + Err(err) => warn!( "Failed to save character reputation for guid {}: {err}", guid.counter() - ); + ), } } @@ -26879,6 +27217,7 @@ impl WorldSession { if self.creature_tick % 4 == 0 { self.tick_auras(); } + self.update_player_save_timer_like_cpp(diff_ms); self.represented_can_delay_teleport_like_cpp = false; self.process_represented_delayed_teleport_after_update_like_cpp(); } @@ -28554,6 +28893,8 @@ impl WorldSession { } self.dispatch_packet(pkt).await; } + + self.process_pending_periodic_player_save_like_cpp().await; } async fn process_pending_creature_kills_like_cpp(&mut self) { @@ -99674,6 +100015,170 @@ mod tests { ); } + #[test] + fn player_save_transaction_plan_orders_represented_statements_like_cpp() { + let (mut session, _, _) = make_session(); + let guid = ObjectGuid::create_player(1, 5010); + let powers = loaded_character_power_snapshot_like_cpp([321, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + let snapshot = PlayerSaveToDbSnapshotLikeCpp { + guid, + map_id: 571, + instance_id: 7, + position: Position::new(11.0, 22.0, 33.0, 1.5), + level: 70, + xp: 12_345, + money: 67_890, + health: 444, + max_health: 555, + powers, + }; + session.set_player_guid(Some(guid)); + session.set_player_level_like_cpp(70); + session.set_player_xp_like_cpp(12_345); + session.set_player_gold_like_cpp(67_890); + session.set_loaded_player_powers_like_cpp([321, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + session.set_player_skill_records_like_cpp(HashMap::from([( + 762, + RepresentedPlayerSkillLikeCpp { + skill_id: 762, + value: 75, + max: 75, + profession_slot: -1, + }, + )])); + + let plan = session.current_player_save_to_db_statement_plan_like_cpp(&snapshot, 1_000); + let sqls: Vec<&str> = plan.statements.iter().map(PreparedStatement::sql).collect(); + + assert_eq!( + &sqls[..10], + &[ + CharStatements::UPD_CHARACTER_POSITION.sql(), + CharStatements::UPD_CHAR_LEVEL.sql(), + CharStatements::UPD_CHAR_MONEY.sql(), + CharStatements::UPD_CHAR_HEALTH.sql(), + CharStatements::UPD_CHAR_POWERS.sql(), + CharStatements::UPD_CHAR_TALENT_RESET_STATE.sql(), + CharStatements::UPD_CHAR_EXPLORED_ZONES.sql(), + CharStatements::DEL_CHAR_SKILLS.sql(), + CharStatements::INS_CHAR_SKILLS.sql(), + CharStatements::UPD_CHAR_DIFFICULTIES.sql(), + ], + "C++ Player::SaveToDB appends represented character statements to one CharacterDatabaseTransaction in save order" + ); + assert_eq!( + sqls.last().copied(), + Some(CharStatements::UPD_CHAR_PLAYED_TIME.sql()) + ); + } + + #[test] + fn player_save_plan_marks_dirty_state_only_after_commit_like_cpp() { + let (mut session, _, _) = make_session(); + let guid = ObjectGuid::create_player(1, 5011); + let snapshot = PlayerSaveToDbSnapshotLikeCpp { + guid, + map_id: 571, + instance_id: 7, + position: Position::new(11.0, 22.0, 33.0, 1.5), + level: 70, + xp: 12_345, + money: 67_890, + health: 444, + max_health: 555, + powers: loaded_character_power_snapshot_like_cpp([321, 0, 0, 0, 0, 0, 0, 0, 0, 0]), + }; + session.set_player_guid(Some(guid)); + + session.tutorials_loaded_coherently_like_cpp = true; + session.tutorials_loaded_from_db_like_cpp = false; + session.tutorials_changed_like_cpp = true; + + session.mark_represented_equipment_sets_loaded_like_cpp(); + let mut changed_equipment = RepresentedEquipmentSetLikeCpp::equipment( + 9, + 0, + RepresentedEquipmentSetUpdateStateLikeCpp::Changed, + ); + changed_equipment.guid = 900; + session.insert_represented_equipment_set_like_cpp(900, changed_equipment); + + session + .reputation_mgr_like_cpp_mut() + .insert_state_for_test_like_cpp(crate::reputation::mgr::FactionStateLikeCpp { + standing: 123, + flags: ReputationFlagsLikeCpp::VISIBLE, + ..crate::reputation::mgr::FactionStateLikeCpp::new_like_cpp( + 85, + 14, + ReputationFlagsLikeCpp::VISIBLE, + ) + }); + + let plan = session.current_player_save_to_db_statement_plan_like_cpp(&snapshot, 1_000); + + assert!(plan.tutorials_changed_committed_like_cpp); + assert!(plan.equipment_sets_committed_like_cpp); + assert!(plan.reputation_committed_like_cpp); + assert!(session.tutorials_changed_like_cpp); + assert!(!session.tutorials_loaded_from_db_like_cpp); + assert_eq!( + session + .represented_equipment_set_like_cpp(900) + .expect("equipment set") + .state, + RepresentedEquipmentSetUpdateStateLikeCpp::Changed, + "failed transaction must leave equipment-set state dirty for retry" + ); + assert!( + session + .reputation_mgr_like_cpp() + .get_state(14) + .expect("reputation state") + .need_save, + "failed transaction must leave reputation state dirty for retry" + ); + + session.mark_current_player_save_to_db_committed_like_cpp(&plan); + + assert!(!session.tutorials_changed_like_cpp); + assert!(session.tutorials_loaded_from_db_like_cpp); + assert_eq!( + session + .represented_equipment_set_like_cpp(900) + .expect("equipment set") + .state, + RepresentedEquipmentSetUpdateStateLikeCpp::Unchanged + ); + assert!( + !session + .reputation_mgr_like_cpp() + .get_state(14) + .expect("reputation state") + .need_save + ); + } + + #[test] + fn player_save_timer_marks_periodic_save_due_like_cpp() { + let (mut session, _, _) = make_session(); + session.set_player_save_interval_ms_like_cpp(100); + + assert_eq!(session.next_player_save_ms_like_cpp, 100); + assert!(!session.pending_periodic_player_save_like_cpp); + + session.update_player_save_timer_like_cpp(99); + assert_eq!(session.next_player_save_ms_like_cpp, 1); + assert!(!session.pending_periodic_player_save_like_cpp); + + session.update_player_save_timer_like_cpp(1); + assert_eq!(session.next_player_save_ms_like_cpp, 0); + assert!( + session.pending_periodic_player_save_like_cpp, + "C++ Player::Update calls SaveToDB when m_nextSave expires; Rust marks the async save pending" + ); + } + #[test] fn character_position_save_uses_captured_snapshot_map_instance_like_cpp() { let guid = ObjectGuid::create_player(1, 5003); diff --git a/docs/migration/EXISTING-CODE-DEFECTS.md b/docs/migration/EXISTING-CODE-DEFECTS.md index 070480634..aa546a304 100644 --- a/docs/migration/EXISTING-CODE-DEFECTS.md +++ b/docs/migration/EXISTING-CODE-DEFECTS.md @@ -33,8 +33,11 @@ mutates DB" ≠ "computes the right result / can't lose or dupe data." slot *before* store. - [ ] **D-C6 Loot money TOCTOU → duplication.** `loot.coins` zeroed *after* distribute; two concurrent `handle_loot_money` both pay out. `handlers/loot.rs:1293-1356`. -- [ ] **D-C7 Player save has no transaction.** Serial awaits; crash mid-save leaves DB - inconsistent (gold debited, item not added; level saved, position reverted). `session.rs:21667`. +- [ ] **D-C7 Player save has incomplete transaction coverage.** Issue #17 wraps the + Rust-covered represented `Player::SaveToDB` character statements in one `SqlTransaction`, but + full C++ save parity, login/account transaction coupling, capture diff, and manual runtime QA + remain pending. Previous serial awaits could leave DB inconsistent (gold debited, item not + added; level saved, position reverted). `session.rs`. - [ ] **D-C8 Vendor buy not atomic.** Gold/currency applied to runtime before item DB commit; commit fail = paid, no item. `handlers/character.rs:10177-10292`. - [ ] **D-C9 Group full-check race.** Size checked then join without re-check → 6+ member @@ -59,8 +62,11 @@ mutates DB" ≠ "computes the right result / can't lose or dupe data." objectives. `handlers/loot.rs:6786`. - [ ] **D-H7 Auras not saved at logout.** All buffs/debuffs reset on relog. `session.rs:21656`. C++ `Player::_SaveAuras`. -- [ ] **D-H8 No periodic save + incomplete logout save.** Full inventory / mid-quest progress / - newly-learned spells may not persist; crash loses them. (Pairs with M0.4.) +- [ ] **D-H8 Periodic save represented-partial + incomplete logout save.** Issue #17 adds a + `CONFIG_INTERVAL_SAVE` / `PlayerSaveInterval` session timer for represented `Player::SaveToDB`, + but full inventory / mid-quest progress / newly-learned spells may still be outside the Rust + save surface; first-save randomization, capture diff, and manual runtime QA remain pending. + (Pairs with M0.4.) - [ ] **D-H9 Trainer skips req-skill-rank + prerequisite-spell checks.** Loaded but ignored → learn spells you shouldn't. `handlers/trainer.rs:405-463`. C++ `Trainer.cpp:195-200`. - [ ] **D-H10 Movement trusts client position.** Only NaN/map-bounds checks; no speed/teleport diff --git a/docs/migration/STATE.md b/docs/migration/STATE.md index ac0edac02..152d265b6 100644 --- a/docs/migration/STATE.md +++ b/docs/migration/STATE.md @@ -113,8 +113,8 @@ observable mutation · **ABSENT**. | `ChrClasses`, `FactionTemplate`, `CharBaseInfo` stores | **ABSENT** | class scaling hardcoded; **NPC hostility checks broken** | | `SpellPower`/`SpellCastTimes`/`SpellCooldowns` stores | **PARTIAL** | loaded into represented spell metadata for normal difficulty; full C++ modifier and runtime integration still incomplete | | DBC/DB2 store coverage | **~110 / ~325 (34%)** | `cpp-db2-stores.tsv` | -| Player periodic save | **ABSENT** | save only on logout → **crash = data loss** | -| Multi-statement save transactions | **ABSENT** | `session.rs:21667` serial awaits → partial-save races | +| Player periodic save | **REPRESENTED-PARTIAL** | session timer now uses `CONFIG_INTERVAL_SAVE` / `PlayerSaveInterval` and queues represented `Player::SaveToDB`; first-save randomization, capture diff, install/restart, and live-client QA still pending | +| Multi-statement save transactions | **REPRESENTED-PARTIAL** | represented character save now commits the Rust-covered `Player::SaveToDB` statement set in one `SqlTransaction`; full C++ save surface, login/account transaction coupling, capture diff, and manual runtime QA still pending | | Respawn DB persistence | **ABSENT** | in-memory queue; respawns lost on restart | | Quest objective auto-credit (loot item / explore) | **ABSENT** | not hooked to loot/area-trigger | From 99cbbbd3651cf6c0b0a4f0c44abe2b800db6c52a Mon Sep 17 00:00:00 2001 From: cDmonio Date: Sat, 4 Jul 2026 08:10:41 +0000 Subject: [PATCH 2/2] Fix periodic player save review gaps --- crates/world-server/src/main.rs | 10 +++++++- crates/wow-world/src/session.rs | 42 +++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/crates/world-server/src/main.rs b/crates/world-server/src/main.rs index f01f17d04..a3365c8e6 100644 --- a/crates/world-server/src/main.rs +++ b/crates/world-server/src/main.rs @@ -11925,10 +11925,18 @@ async fn create_session( info!("Session ready for account {}", account.id); // Session update loop + let mut last_session_update = Instant::now(); loop { let (count, disconnecting) = warn_about_sync_queries_scope_like_cpp(async { + let now = Instant::now(); + let diff_ms = now + .saturating_duration_since(last_session_update) + .as_millis() + .min(u128::from(u32::MAX)) as u32; + last_session_update = now; + // Process incoming packets - let count = session.update(50); + let count = session.update(diff_ms); // Dispatch pending packets (async handlers) session.process_pending().await; diff --git a/crates/wow-world/src/session.rs b/crates/wow-world/src/session.rs index a708fa793..67801fbb9 100644 --- a/crates/wow-world/src/session.rs +++ b/crates/wow-world/src/session.rs @@ -7282,9 +7282,9 @@ impl WorldSession { map_id, instance_id, position, - 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, + level: self.player_level_like_cpp(), + xp: self.player_xp_like_cpp(), + money: self.player_gold_like_cpp(), health, max_health: canonical_max_health, powers, @@ -22445,6 +22445,9 @@ impl WorldSession { if !self.pending_periodic_player_save_like_cpp || self.state != SessionState::LoggedIn { return; } + if self.pending_teleport_save_destination_like_cpp().is_some() { + return; + } self.save_current_player_to_db_like_cpp().await; } @@ -99561,7 +99564,7 @@ mod tests { } #[test] - fn logout_save_snapshot_uses_latest_session_position_and_canonical_gameplay_like_cpp() { + fn logout_save_snapshot_uses_session_money_xp_and_canonical_health_like_cpp() { let (mut session, _, _) = make_session(); let canonical = shared_canonical_map_manager(); let player_guid = ObjectGuid::create_player(1, 70); @@ -99623,9 +99626,9 @@ mod tests { map_id: 571, instance_id: 0, position: latest_session_position, - level: 42, - xp: 1234, - money: 5678, + level: 10, + xp: 1, + money: 2, health: 456, max_health: 900, powers: loaded_character_power_snapshot_like_cpp([ @@ -99637,9 +99640,9 @@ mod tests { session.player_position_like_cpp(), Some(latest_session_position) ); - assert_eq!(session.player_level_like_cpp(), 42); - assert_eq!(session.player_xp_like_cpp(), 1234); - assert_eq!(session.player_gold_like_cpp(), 5678); + assert_eq!(session.player_level_like_cpp(), 10); + assert_eq!(session.player_xp_like_cpp(), 1); + assert_eq!(session.player_gold_like_cpp(), 2); assert_eq!(session.player_health_like_cpp(), 456); } @@ -100179,6 +100182,25 @@ mod tests { ); } + #[tokio::test] + async fn periodic_player_save_defers_while_teleport_pending_like_cpp() { + let (mut session, _, _) = make_session(); + session.state = SessionState::LoggedIn; + session.set_player_save_interval_ms_like_cpp(100); + session.pending_teleport = Some((0, Position::new(10.0, 20.0, 30.0, 1.5))); + session.update_player_save_timer_like_cpp(100); + + session + .process_pending_periodic_player_save_like_cpp() + .await; + + assert!( + session.pending_periodic_player_save_like_cpp, + "autosave remains pending until the teleport handshake clears" + ); + assert_eq!(session.next_player_save_ms_like_cpp, 0); + } + #[test] fn character_position_save_uses_captured_snapshot_map_instance_like_cpp() { let guid = ObjectGuid::create_player(1, 5003);