Fix potential softlock when closing menu in frame-by-frame (#610)

* Fix softlock

* Update menu.rs

* Update mod.rs

* Update menu.rs

* Update button_config.rs

* Unused code

src/common/button_config.rs

@@ -2,7 +2,6 @@ use std::collections::HashMap;
 
 use crate::common::*;
 use crate::input::{ControllerStyle::*, *};
-use crate::training::frame_counter;
 use crate::training::ui::menu::VANILLA_MENU_ACTIVE;
 
 use lazy_static::lazy_static;
@@ -137,8 +136,7 @@ pub fn handle_final_input_mapping(player_idx: i32, controller_struct: &mut SomeC
         let p1_controller = &mut *controller_struct.controller;
         let mut start_menu_request = false;
 
-        let menu_close_wait_frame =
+        let menu_close_wait_frame = unsafe { *menu::VISUAL_FRAME_COUNTER.data_ptr() };
-            unsafe { frame_counter::get_frame_count(menu::FRAME_COUNTER_INDEX) };
         if unsafe { MENU.menu_open_start_press == OnOff::On } {
             let start_hold_frames = &mut *START_HOLD_FRAMES.lock();
             if p1_controller.current_buttons.plus() {

src/common/menu.rs

@@ -13,18 +13,10 @@ use crate::consts::MENU_OPTIONS_PATH;
 use crate::events::{Event, EVENT_QUEUE};
 use crate::input::*;
 use crate::logging::*;
-use crate::training::frame_counter;
 
-pub static mut FRAME_COUNTER_INDEX: usize = 0;
 pub const MENU_CLOSE_WAIT_FRAMES: u32 = 15;
 pub static mut QUICK_MENU_ACTIVE: bool = false;
 
-pub fn init() {
-    unsafe {
-        FRAME_COUNTER_INDEX = frame_counter::register_counter_no_reset();
-    }
-}
-
 pub unsafe fn menu_condition() -> bool {
     button_config::combo_passes_exclusive(button_config::ButtonCombo::OpenMenu)
 }
@@ -117,6 +109,8 @@ lazy_static! {
             (RUp, 0),
         ]))
     };
+    pub static ref VISUAL_FRAME_COUNTER: Mutex<u32> = Mutex::new(0);
+    pub static ref VISUAL_FRAME_COUNTER_SHOULD_COUNT: Mutex<bool> = Mutex::new(false);
 }
 
 pub fn handle_final_input_mapping(
@@ -128,19 +122,16 @@ pub fn handle_final_input_mapping(
         if player_idx == 0 {
             let p1_controller = &mut *controller_struct.controller;
             *P1_CONTROLLER_STYLE.lock() = p1_controller.style;
-            if frame_counter::get_frame_count(FRAME_COUNTER_INDEX) > 0
+            let visual_frame_count = *VISUAL_FRAME_COUNTER.data_ptr();
-                && frame_counter::get_frame_count(FRAME_COUNTER_INDEX) < MENU_CLOSE_WAIT_FRAMES
+            if visual_frame_count > 0 && visual_frame_count < MENU_CLOSE_WAIT_FRAMES {
-            {
                 // If we just closed the menu, kill all inputs to avoid accidental presses
                 *out = MappedInputs::empty();
                 p1_controller.current_buttons = ButtonBitfield::default();
                 p1_controller.previous_buttons = ButtonBitfield::default();
                 p1_controller.just_down = ButtonBitfield::default();
                 p1_controller.just_release = ButtonBitfield::default();
-            } else if frame_counter::get_frame_count(FRAME_COUNTER_INDEX) >= MENU_CLOSE_WAIT_FRAMES
+            } else if visual_frame_count >= MENU_CLOSE_WAIT_FRAMES {
-            {
+                *VISUAL_FRAME_COUNTER_SHOULD_COUNT.lock() = false;
-                frame_counter::reset_frame_count(FRAME_COUNTER_INDEX);
-                frame_counter::stop_counting(FRAME_COUNTER_INDEX);
             }
 
             if QUICK_MENU_ACTIVE {
@@ -199,7 +190,7 @@ pub fn handle_final_input_mapping(
                         app.on_b()
                     } else {
                         // Leave menu.
-                        frame_counter::start_counting(FRAME_COUNTER_INDEX);
+                        *VISUAL_FRAME_COUNTER_SHOULD_COUNT.lock() = true;
                         QUICK_MENU_ACTIVE = false;
                         let menu_json = app.get_menu_selections();
                         set_menu_from_json(&menu_json);
@@ -211,7 +202,7 @@ pub fn handle_final_input_mapping(
                 .then(|| {
                     received_input = true;
                     // Leave menu.
-                    frame_counter::start_counting(FRAME_COUNTER_INDEX);
+                    *VISUAL_FRAME_COUNTER_SHOULD_COUNT.lock() = true;
                     QUICK_MENU_ACTIVE = false;
                     let menu_json = app.get_menu_selections();
                     set_menu_from_json(&menu_json);

src/training/frame_counter.rs

@@ -14,7 +14,7 @@ fn _register_counter(no_reset: bool) -> usize {
     }
 }
 
-pub fn register_counter_no_reset() -> usize {
+pub fn _register_counter_no_reset() -> usize {
     _register_counter(true)
 }
 

src/training/mod.rs

@@ -696,15 +696,32 @@ unsafe fn handle_final_input_mapping(
     controller_struct: &mut SomeControllerStruct,
     arg: bool,
 ) {
-    // go through the original mapping function first
+    // Order of hooks here REALLY matters. Tread lightly
+
+    // Go through the original mapping function first
     original!()(mappings, player_idx, out, controller_struct, arg);
     if !is_training_mode() {
         return;
     }
-    menu::handle_final_input_mapping(player_idx, controller_struct, out);
+
-    button_config::handle_final_input_mapping(player_idx, controller_struct);
+    // Check if we should apply hot reload configs
+    // Non-mutable pull
     dev_config::handle_final_input_mapping(player_idx, controller_struct);
+
+    // Grab menu inputs from player
+    // MUTATES controller state to kill inputs when in or closing menu
+    menu::handle_final_input_mapping(player_idx, controller_struct, out);
+
+    // Grab button input requests from player
+    // MUTATES controller state to kill start presses for menu
+    button_config::handle_final_input_mapping(player_idx, controller_struct);
+
+    // Potentially apply input delay
+    // MUTATES controller state to delay inputs
     input_delay::handle_final_input_mapping(player_idx, out);
+
+    // Potentially apply input recording, thus with delay
+    // MUTATES controller state to apply recording or playback
     input_record::handle_final_input_mapping(player_idx, out);
 }
 
@@ -800,5 +817,4 @@ pub fn training_mods() {
     tech::init();
     input_record::init();
     ui::init();
-    menu::init();
 }

src/training/ui/menu.rs

@@ -6,8 +6,9 @@ use smash::ui2d::{SmashPane, SmashTextBox};
 use training_mod_tui::gauge::GaugeState;
 use training_mod_tui::{App, AppPage, NUM_LISTS};
 
-use crate::common::menu::{self, MENU_CLOSE_WAIT_FRAMES};
+use crate::common::menu::{
-use crate::training::frame_counter;
+    MENU_CLOSE_WAIT_FRAMES, VISUAL_FRAME_COUNTER, VISUAL_FRAME_COUNTER_SHOULD_COUNT,
+};
 use crate::{common, common::menu::QUICK_MENU_ACTIVE, input::*};
 
 use super::fade_out;
@@ -352,6 +353,12 @@ unsafe fn render_slider_page(app: &App, root_pane: &Pane) {
 }
 
 pub unsafe fn draw(root_pane: &Pane) {
+    if *VISUAL_FRAME_COUNTER_SHOULD_COUNT.data_ptr() {
+        *VISUAL_FRAME_COUNTER.lock() += 1;
+    } else {
+        *VISUAL_FRAME_COUNTER.lock() = 0;
+    }
+
     // Determine if we're in the menu by seeing if the "help" footer has
     // begun moving upward. It starts at -80 and moves to 0 over 10 frames
     // in info_training_in_menu.bflan
@@ -374,7 +381,7 @@ pub unsafe fn draw(root_pane: &Pane) {
 
     let overall_parent_pane = root_pane.find_pane_by_name_recursive("TrModMenu").unwrap();
     overall_parent_pane.set_visible(true);
-    let menu_close_wait_frame = frame_counter::get_frame_count(menu::FRAME_COUNTER_INDEX);
+    let menu_close_wait_frame = *VISUAL_FRAME_COUNTER.data_ptr();
     if QUICK_MENU_ACTIVE {
         overall_parent_pane.alpha = 255;
         overall_parent_pane.global_alpha = 255;