Skip to content

Fix inaccurate Frame Advantage #752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asimon-1 merged 3 commits into from
Nov 17, 2025
Merged

Fix inaccurate Frame Advantage #752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bb0df9
Refactor to fix frame advantage bug with untrue multihit moves
asimon-1 Nov 16, 2025
4a33cdb
Use infliction check instead of status check to resolve missing notif…
asimon-1 Nov 16, 2025
34aced4
Handle throws that don't trigger is_infliction
asimon-1 Nov 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 61 additions & 86 deletions src/training/combo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -13,31 +14,13 @@ use training_mod_sync::*;

static PLAYER_WAS_ACTIONABLE: RwLock<bool> = RwLock::new(false);
static CPU_WAS_ACTIONABLE: RwLock<bool> = RwLock::new(false);
static IS_COUNTING: RwLock<bool> = RwLock::new(false);

static PLAYER_FRAME_COUNTER_INDEX: LazyLock<usize> =
LazyLock::new(|| frame_counter::register_counter(frame_counter::FrameCounterType::InGame));
static CPU_FRAME_COUNTER_INDEX: LazyLock<usize> =
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 is_in_hitstun(module_accessor: *mut BattleObjectModuleAccessor) -> bool {
(*FIGHTER_STATUS_KIND_DAMAGE..*FIGHTER_STATUS_KIND_DAMAGE_FALL)
.contains(&StatusModule::status_kind(module_accessor))
}

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_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
Expand All @@ -54,12 +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 Advantage over Input Recording Playback
notifications::clear_notification("Input Recording");
notifications::clear_notification("Frame Advantage");
// 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 {
Expand Down Expand Up @@ -87,8 +69,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)
Expand All @@ -101,76 +84,68 @@ 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)))
// 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) {
// 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 AttackModule::is_infliction(
player_module_accessor,
*COLLISION_KIND_MASK_HIT | *COLLISION_KIND_MASK_SHIELD,
) || StatusModule::status_kind(player_module_accessor) == *FIGHTER_STATUS_KIND_THROW
{
// 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.
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 {
// 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,
} else if player_is_actionable && cpu_is_actionable {
if read(&IS_COUNTING) {
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
);
// Frame counters should reset before we start again, but reset them just to be safe
update_frame_advantage(frame_advantage);
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);
assign(&IS_COUNTING, false);
}
} else {
// No need to start or stop counting, one of the fighters is still not actionable
}

assign(&CPU_WAS_ACTIONABLE, cpu_is_actionable);
assign(&PLAYER_WAS_ACTIONABLE, player_is_actionable);
}
2 changes: 1 addition & 1 deletion src/training/frame_counter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading