From 8bb0df921575a246de6ac82a3c1278d445bccdef Mon Sep 17 00:00:00 2001 From: asimon-1 <40246417+asimon-1@users.noreply.github.com> Date: Sun, 16 Nov 2025 08:06:28 -0500 Subject: [PATCH 1/3] Refactor to fix frame advantage bug with untrue multihit moves --- src/training/combo.rs | 144 +++++++++++++++------------------- src/training/frame_counter.rs | 2 +- 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/src/training/combo.rs b/src/training/combo.rs index 3819b7761..da0eb12ae 100644 --- a/src/training/combo.rs +++ b/src/training/combo.rs @@ -1,9 +1,10 @@ use skyline::nn::ui2d::ResColor; -use smash::app::lua_bind::{CancelModule, StatusModule, WorkModule}; +use smash::app::lua_bind::{AttackModule, CancelModule, StatusModule, WorkModule}; use smash::app::BattleObjectModuleAccessor; use smash::lib::lua_const::*; use crate::consts::Action; +use crate::info; use crate::training::frame_counter; use crate::training::ui::notifications; use crate::try_get_module_accessor; @@ -13,6 +14,7 @@ use training_mod_sync::*; static PLAYER_WAS_ACTIONABLE: RwLock = RwLock::new(false); static CPU_WAS_ACTIONABLE: RwLock = RwLock::new(false); +static IS_COUNTING: RwLock = RwLock::new(false); static PLAYER_FRAME_COUNTER_INDEX: LazyLock = LazyLock::new(|| frame_counter::register_counter(frame_counter::FrameCounterType::InGame)); @@ -21,12 +23,7 @@ static CPU_FRAME_COUNTER_INDEX: LazyLock = unsafe fn was_in_hitstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { let prev_status = StatusModule::prev_status_kind(module_accessor, 0); - (*FIGHTER_STATUS_KIND_DAMAGE..*FIGHTER_STATUS_KIND_DAMAGE_FALL).contains(&prev_status) -} - -unsafe fn is_in_hitstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { - (*FIGHTER_STATUS_KIND_DAMAGE..*FIGHTER_STATUS_KIND_DAMAGE_FALL) - .contains(&StatusModule::status_kind(module_accessor)) + (*FIGHTER_STATUS_KIND_DAMAGE..=*FIGHTER_STATUS_KIND_DAMAGE_FALL).contains(&prev_status) } unsafe fn was_in_shieldstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { @@ -34,10 +31,6 @@ unsafe fn was_in_shieldstun(module_accessor: *mut BattleObjectModuleAccessor) -> prev_status == FIGHTER_STATUS_KIND_GUARD_DAMAGE } -unsafe fn is_in_shieldstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { - StatusModule::status_kind(module_accessor) == FIGHTER_STATUS_KIND_GUARD_DAMAGE -} - unsafe fn is_actionable(module_accessor: *mut BattleObjectModuleAccessor) -> bool { [ FIGHTER_STATUS_TRANSITION_TERM_ID_CONT_ESCAPE_AIR, // Airdodge @@ -54,9 +47,8 @@ unsafe fn is_actionable(module_accessor: *mut BattleObjectModuleAccessor) -> boo fn update_frame_advantage(frame_advantage: i32) { if read(&MENU).frame_advantage == OnOff::ON { - // Prioritize Frame Advantage over Input Recording Playback - notifications::clear_notification("Input Recording"); - notifications::clear_notification("Frame Advantage"); + // Prioritize Frame Advantages over all others + notifications::clear_all_notifications(); notifications::color_notification( "Frame Advantage".to_string(), format!("{frame_advantage}"), @@ -87,8 +79,9 @@ fn update_frame_advantage(frame_advantage: i32) { pub unsafe fn once_per_frame(module_accessor: &mut BattleObjectModuleAccessor) { // Skip the CPU so we don't run twice per frame + // Also skip if the CPU is set to mash since that interferes with the frame calculation let entry_id_int = WorkModule::get_int(module_accessor, *FIGHTER_INSTANCE_WORK_ID_INT_ENTRY_ID); - if entry_id_int != (FighterId::Player as i32) { + if entry_id_int != (FighterId::Player as i32) || read(&MENU).mash_state != Action::empty() { return; } let player_module_accessor = try_get_module_accessor(FighterId::Player) @@ -101,76 +94,69 @@ pub unsafe fn once_per_frame(module_accessor: &mut BattleObjectModuleAccessor) { let cpu_is_actionable = is_actionable(cpu_module_accessor); let cpu_was_actionable = read(&CPU_WAS_ACTIONABLE); let cpu_just_actionable = !cpu_was_actionable && cpu_is_actionable; - let is_counting = frame_counter::is_counting(*PLAYER_FRAME_COUNTER_INDEX) - || frame_counter::is_counting(*CPU_FRAME_COUNTER_INDEX); - if !is_counting { - if read(&MENU).mash_state == Action::empty() - && !player_is_actionable - && !cpu_is_actionable - && (!was_in_shieldstun(cpu_module_accessor) && is_in_shieldstun(cpu_module_accessor) - || (!was_in_hitstun(cpu_module_accessor) && is_in_hitstun(cpu_module_accessor))) - { - // Start counting when: - // 1. We have no mash option selected AND - // 2. Neither fighter is currently actionable AND - // 3. Either - // a. the CPU has just entered shieldstun - // b. the CPU has just entered hitstun - // - // If a mash option is selected, this can interfere with our ability to determine when - // a character becomes actionable. So don't ever start counting if we can't reliably stop. - // - // Since our "just_actionable" checks assume that neither character is already actionable, - // we need to guard against instances where the player is already actionable by the time that - // the CPU get hit, such as if the player threw a projectile from far away. - // Otherwise our "just_actionable" checks are not valid. - // - // We also need to guard against instances where the CPU's status is in hitstun but they are actually actionable. - // I dunno, makes no sense to me either. Can trigger this edge case with PAC-MAN jab 1 against Lucas at 0%. - // This shows up as the count restarting immediately after the last one ended. + // Lock in frames + if cpu_just_actionable { + frame_counter::stop_counting(*CPU_FRAME_COUNTER_INDEX); + } + + if player_just_actionable { + frame_counter::stop_counting(*PLAYER_FRAME_COUNTER_INDEX); + } + + // DEBUG LOGGING + // if read(&IS_COUNTING) { + // info!("P0:{:x}\tC0:{:x}", StatusModule::status_kind(player_module_accessor), StatusModule::status_kind(cpu_module_accessor)); + // info!("P1:{:x}\tC1:{:x}", StatusModule::prev_status_kind(player_module_accessor,0), StatusModule::prev_status_kind(cpu_module_accessor,0)); + // if player_is_actionable && cpu_is_actionable { + // info!("!"); + // } else if !player_is_actionable && cpu_is_actionable { + // info!("-"); + // } else if player_is_actionable && !cpu_is_actionable { + // info!("+"); + // } else { + // info!("."); + // } + // } + + if !player_is_actionable && !cpu_is_actionable { + if !read(&IS_COUNTING) { + // Start counting since neither fighter is actionable frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); frame_counter::start_counting(*PLAYER_FRAME_COUNTER_INDEX); frame_counter::start_counting(*CPU_FRAME_COUNTER_INDEX); - } - } else { - // Uncomment this if you want some frame logging - // if (player_is_actionable && cpu_is_actionable) { - // info!("!"); - // } else if (!player_is_actionable && cpu_is_actionable) { - // info!("-"); - // } else if (player_is_actionable && !cpu_is_actionable) { - // info!("+"); - // } else { - // info!("."); - // } - - // Stop counting as soon as each fighter becomes actionable - if player_just_actionable { - frame_counter::stop_counting(*PLAYER_FRAME_COUNTER_INDEX); - } - - if cpu_just_actionable { - frame_counter::stop_counting(*CPU_FRAME_COUNTER_INDEX); - } - - // If we just finished counting for the second fighter, then display frame advantage - if !frame_counter::is_counting(*PLAYER_FRAME_COUNTER_INDEX) - && !frame_counter::is_counting(*CPU_FRAME_COUNTER_INDEX) - && (player_just_actionable || cpu_just_actionable) - { - update_frame_advantage( - frame_counter::get_frame_count(*CPU_FRAME_COUNTER_INDEX) as i32 - - frame_counter::get_frame_count(*PLAYER_FRAME_COUNTER_INDEX) as i32, - ); - // Frame counters should reset before we start again, but reset them just to be safe + assign(&IS_COUNTING, true); + } else if AttackModule::is_infliction( + player_module_accessor, + *COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD, + ) { + // We are already counting, but the player just landed a hit so reset the counters + // This prevents multihit moves which aren't true combos from miscounting from the first hit + // e.g. Pikachu back air on shield + info!("Resetting frame counters due to new hit"); frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); - }; - - // Store the current actionability state for next frame - assign(&PLAYER_WAS_ACTIONABLE, player_is_actionable); - assign(&CPU_WAS_ACTIONABLE, cpu_is_actionable); + frame_counter::start_counting(*PLAYER_FRAME_COUNTER_INDEX); + frame_counter::start_counting(*CPU_FRAME_COUNTER_INDEX); + } + } else if player_is_actionable && cpu_is_actionable { + if read(&IS_COUNTING) { + if was_in_shieldstun(cpu_module_accessor) || was_in_hitstun(cpu_module_accessor) { + update_frame_advantage( + frame_counter::get_frame_count(*CPU_FRAME_COUNTER_INDEX) as i32 + - frame_counter::get_frame_count(*PLAYER_FRAME_COUNTER_INDEX) as i32, + ); + frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); + frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); + } + assign(&IS_COUNTING, false); + } + } else { + // One of the fighters is still not actionable + // Keep counting until both are actionable } + + assign(&CPU_WAS_ACTIONABLE, cpu_is_actionable); + assign(&PLAYER_WAS_ACTIONABLE, player_is_actionable); } diff --git a/src/training/frame_counter.rs b/src/training/frame_counter.rs index f394a397e..25a625901 100644 --- a/src/training/frame_counter.rs +++ b/src/training/frame_counter.rs @@ -37,7 +37,7 @@ pub fn stop_counting(index: usize) { (*counters_lock)[index].should_count = false; } -pub fn is_counting(index: usize) -> bool { +pub fn _is_counting(index: usize) -> bool { let counters_lock = lock_read(&COUNTERS); (*counters_lock)[index].should_count } From 4a33cdbbcc5183259abb7a230443129a2bba1d87 Mon Sep 17 00:00:00 2001 From: asimon-1 <40246417+asimon-1@users.noreply.github.com> Date: Sun, 16 Nov 2025 09:21:54 -0500 Subject: [PATCH 2/3] Use infliction check instead of status check to resolve missing notifications --- src/training/combo.rs | 64 ++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/training/combo.rs b/src/training/combo.rs index da0eb12ae..106ee4e7d 100644 --- a/src/training/combo.rs +++ b/src/training/combo.rs @@ -1,5 +1,5 @@ use skyline::nn::ui2d::ResColor; -use smash::app::lua_bind::{AttackModule, CancelModule, StatusModule, WorkModule}; +use smash::app::lua_bind::{AttackModule, CancelModule, WorkModule}; use smash::app::BattleObjectModuleAccessor; use smash::lib::lua_const::*; @@ -21,16 +21,6 @@ static PLAYER_FRAME_COUNTER_INDEX: LazyLock = static CPU_FRAME_COUNTER_INDEX: LazyLock = LazyLock::new(|| frame_counter::register_counter(frame_counter::FrameCounterType::InGame)); -unsafe fn was_in_hitstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { - let prev_status = StatusModule::prev_status_kind(module_accessor, 0); - (*FIGHTER_STATUS_KIND_DAMAGE..=*FIGHTER_STATUS_KIND_DAMAGE_FALL).contains(&prev_status) -} - -unsafe fn was_in_shieldstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool { - let prev_status = StatusModule::prev_status_kind(module_accessor, 0); - prev_status == FIGHTER_STATUS_KIND_GUARD_DAMAGE -} - unsafe fn is_actionable(module_accessor: *mut BattleObjectModuleAccessor) -> bool { [ FIGHTER_STATUS_TRANSITION_TERM_ID_CONT_ESCAPE_AIR, // Airdodge @@ -47,11 +37,11 @@ unsafe fn is_actionable(module_accessor: *mut BattleObjectModuleAccessor) -> boo fn update_frame_advantage(frame_advantage: i32) { if read(&MENU).frame_advantage == OnOff::ON { - // Prioritize Frame Advantages over all others + // Prioritize notifications for Frame Advantage notifications::clear_all_notifications(); notifications::color_notification( "Frame Advantage".to_string(), - format!("{frame_advantage}"), + format!("{frame_advantage:+}"), 60, match frame_advantage { x if x < 0 => ResColor { @@ -106,8 +96,6 @@ pub unsafe fn once_per_frame(module_accessor: &mut BattleObjectModuleAccessor) { // DEBUG LOGGING // if read(&IS_COUNTING) { - // info!("P0:{:x}\tC0:{:x}", StatusModule::status_kind(player_module_accessor), StatusModule::status_kind(cpu_module_accessor)); - // info!("P1:{:x}\tC1:{:x}", StatusModule::prev_status_kind(player_module_accessor,0), StatusModule::prev_status_kind(cpu_module_accessor,0)); // if player_is_actionable && cpu_is_actionable { // info!("!"); // } else if !player_is_actionable && cpu_is_actionable { @@ -120,41 +108,41 @@ pub unsafe fn once_per_frame(module_accessor: &mut BattleObjectModuleAccessor) { // } if !player_is_actionable && !cpu_is_actionable { - if !read(&IS_COUNTING) { - // Start counting since neither fighter is actionable - frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); - frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); - frame_counter::start_counting(*PLAYER_FRAME_COUNTER_INDEX); - frame_counter::start_counting(*CPU_FRAME_COUNTER_INDEX); - assign(&IS_COUNTING, true); - } else if AttackModule::is_infliction( + if AttackModule::is_infliction( player_module_accessor, - *COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD, + *COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD ) { - // We are already counting, but the player just landed a hit so reset the counters - // This prevents multihit moves which aren't true combos from miscounting from the first hit - // e.g. Pikachu back air on shield - info!("Resetting frame counters due to new hit"); + if !read(&IS_COUNTING) { + // Start counting when the player lands a hit + info!("Starting frame counter"); + } else { + // Note that we want the same behavior even if we are already counting! + // This prevents multihit moves which aren't true combos from miscounting + // from the first hit (e.g. Pikachu back air on shield) + info!("Restarting frame counter"); + } + frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); frame_counter::start_counting(*PLAYER_FRAME_COUNTER_INDEX); frame_counter::start_counting(*CPU_FRAME_COUNTER_INDEX); + assign(&IS_COUNTING, true); } } else if player_is_actionable && cpu_is_actionable { if read(&IS_COUNTING) { - if was_in_shieldstun(cpu_module_accessor) || was_in_hitstun(cpu_module_accessor) { - update_frame_advantage( - frame_counter::get_frame_count(*CPU_FRAME_COUNTER_INDEX) as i32 - - frame_counter::get_frame_count(*PLAYER_FRAME_COUNTER_INDEX) as i32, - ); - frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); - frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); - } + let frame_advantage = frame_counter::get_frame_count(*CPU_FRAME_COUNTER_INDEX) as i32 + - frame_counter::get_frame_count(*PLAYER_FRAME_COUNTER_INDEX) as i32; + info!( + "Stopping frame counter, frame advantage: {}", + frame_advantage + ); + update_frame_advantage(frame_advantage); + frame_counter::reset_frame_count(*PLAYER_FRAME_COUNTER_INDEX); + frame_counter::reset_frame_count(*CPU_FRAME_COUNTER_INDEX); assign(&IS_COUNTING, false); } } else { - // One of the fighters is still not actionable - // Keep counting until both are actionable + // No need to start or stop counting, one of the fighters is still not actionable } assign(&CPU_WAS_ACTIONABLE, cpu_is_actionable); From 34aced4ebe2d12af3403f3c59c59153156bb9873 Mon Sep 17 00:00:00 2001 From: asimon-1 <40246417+asimon-1@users.noreply.github.com> Date: Sun, 16 Nov 2025 09:28:06 -0500 Subject: [PATCH 3/3] Handle throws that don't trigger is_infliction --- src/training/combo.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/training/combo.rs b/src/training/combo.rs index 106ee4e7d..57228785c 100644 --- a/src/training/combo.rs +++ b/src/training/combo.rs @@ -1,5 +1,5 @@ use skyline::nn::ui2d::ResColor; -use smash::app::lua_bind::{AttackModule, CancelModule, WorkModule}; +use smash::app::lua_bind::{AttackModule, CancelModule, StatusModule, WorkModule}; use smash::app::BattleObjectModuleAccessor; use smash::lib::lua_const::*; @@ -110,8 +110,9 @@ pub unsafe fn once_per_frame(module_accessor: &mut BattleObjectModuleAccessor) { if !player_is_actionable && !cpu_is_actionable { if AttackModule::is_infliction( player_module_accessor, - *COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD - ) { + *COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD, + ) || StatusModule::status_kind(player_module_accessor) == *FIGHTER_STATUS_KIND_THROW + { if !read(&IS_COUNTING) { // Start counting when the player lands a hit info!("Starting frame counter");