Moar fixes and optimizations

src/fighter/attack.rs

@@ -7,9 +7,10 @@ use serde::{Deserialize, Serialize};
 use crate::{
     fighter::{
         Fighter, FighterAttributes, FighterFacingDirection, FighterHitboxes,
-        FighterPreviousFacingDirection, FighterPreviousTranslation, FighterTranslation,
+        FighterPreviousFacingDirection, FighterPreviousTranslation, FighterRoster,
+        FighterTranslation,
         animation::{AnimKind, FighterAnimationFrame},
-        baked_animation::{FighterBoneMatrices, FixedMat4},
+        baked_animation::{FighterBoneIndexMap, FighterBoneMatrices, FixedMat4},
         hurtbox::{FixedAffineCapsule, FixedCapsule},
         manifest::FighterManifest,
     },
@@ -131,6 +132,28 @@ pub struct AttackHitbox {
 pub struct StaleMoveQueue(pub VecDeque<AttackKind>);
 
 impl AttackHitbox {
+    pub fn validate(&self) -> Result<(), String> {
+        if self.bone.trim().is_empty() {
+            return Err("bone name is empty".into());
+        }
+        if self.radius <= FGi32::ZERO {
+            return Err("radius must be positive".into());
+        }
+        if self.damage.0 < FGi32::ZERO {
+            return Err("damage must be non-negative".into());
+        }
+        if self.knockback.0 < FGi32::ZERO {
+            return Err("knockback must be non-negative".into());
+        }
+        if self.knockback_growth < FGi32::ZERO {
+            return Err("knockback growth must be non-negative".into());
+        }
+        if self.end_frame <= self.start_frame {
+            return Err("end frame must be greater than start frame".into());
+        }
+        Ok(())
+    }
+
     /// Knockback calculation
     pub fn calculate_knockback(
         &self,
@@ -186,13 +209,15 @@ impl AttackHitbox {
 }
 
 pub fn update_active_hitbox_list(script: &FighterAttackScript, list: &mut Vec<usize>, frame: u32) {
-    *list = script
+    list.clear();
+    list.extend(
+        script
             .hitboxes
             .iter()
             .enumerate()
             .filter(|(_, hb)| hb.start_frame <= frame && hb.end_frame > frame)
-        .map(|(i, _)| i)
+            .map(|(i, _)| i),
-        .collect()
+    );
 }
 
 #[derive(Clone, Debug, PartialEq, Hash)]
@@ -213,6 +238,7 @@ pub struct FighterSolvedHitboxes(pub Vec<FighterSolvedHitbox>);
 pub fn solve_hitboxes(
     mut query: Query<(
         &FighterBoneMatrices,
+        &FighterBoneIndexMap,
         &FighterTranslation,
         &FighterPreviousTranslation,
         &FighterFacingDirection,
@@ -224,6 +250,7 @@ pub fn solve_hitboxes(
 ) {
     for (
         matrices,
+        bone_indices,
         translation,
         previous_translation,
         facing_direction,
@@ -232,23 +259,31 @@ pub fn solve_hitboxes(
         animation_frame,
     ) in &mut query
     {
-        hitboxes.active_hitboxes_solved.clear();
+        let FighterHitboxes {
-        if let Some(ref attack_script) = hitboxes.attack_script {
+            attack_script: script_handle,
+            active_hitboxes,
+            active_hitboxes_solved,
+            ..
+        } = &mut *hitboxes;
+        active_hitboxes_solved.clear();
+        if let Some(attack_script) = script_handle {
             let attack_script = attack_scripts
                 .get(attack_script)
                 .expect("Attack script should be valid");
             let fighter_trf = translation.get_3d_transform(facing_direction);
             let previous_fighter_trf = FighterTranslation(previous_translation.0)
                 .get_3d_transform(&previous_facing_direction.0);
-            let active_hitboxes = hitboxes.active_hitboxes.clone();
+            for hitbox_idx in active_hitboxes.iter().copied() {
-            for hitbox_idx in active_hitboxes {
                 let hitbox = attack_script
                     .hitboxes
                     .get(hitbox_idx)
                     .expect("Hitbox IDX should be valid");
-                let matrices = matrices.get(&hitbox.bone);
+                let matrices = bone_indices
+                    .0
+                    .get(&hitbox.bone)
+                    .and_then(|index| matrices.get_index(*index));
 
-                if let None = matrices {
+                if matrices.is_none() {
                     warn!("Bone {} not found!", hitbox.bone);
                     continue;
                 }
@@ -260,7 +295,7 @@ pub fn solve_hitboxes(
 
                 if hitbox.start_frame == animation_frame.frame {
                     // Sphere only
-                    hitboxes.active_hitboxes_solved.push(FighterSolvedHitbox {
+                    active_hitboxes_solved.push(FighterSolvedHitbox {
                         hitbox_idx,
                         capsule: FixedCapsule {
                             start: curr_hbox_pos,
@@ -271,7 +306,7 @@ pub fn solve_hitboxes(
                 } else {
                     let prev_hbox_matrix = previous_fighter_trf.mul(prev_bone_matrix);
                     let prev_hbox_pos = prev_hbox_matrix.transform_point(hitbox.offset);
-                    hitboxes.active_hitboxes_solved.push(FighterSolvedHitbox {
+                    active_hitboxes_solved.push(FighterSolvedHitbox {
                         hitbox_idx,
                         capsule: FixedCapsule {
                             start: prev_hbox_pos,
@@ -294,6 +329,7 @@ pub struct FighterSolvedHurtboxes(pub Vec<FighterSolvedHurtbox>);
 pub fn solve_hurtboxes(
     mut query: Query<(
         &FighterBoneMatrices,
+        &FighterBoneIndexMap,
         &FighterTranslation,
         &FighterFacingDirection,
         &mut FighterSolvedHurtboxes,
@@ -301,14 +337,19 @@ pub fn solve_hurtboxes(
     )>,
     manifests: Res<Assets<FighterManifest>>,
 ) {
-    for (matrices, translation, facing_direction, mut solved_hurtboxes, fighter) in &mut query {
+    for (matrices, bone_indices, translation, facing_direction, mut solved_hurtboxes, fighter) in
+        &mut query
+    {
         let manifest = manifests
             .get(&fighter.manifest)
             .expect("Manifest should be valid");
         let fighter_trf = translation.get_3d_transform(facing_direction);
         solved_hurtboxes.0.clear();
         for (hurtbox_idx, hurtbox) in manifest.hurtboxes.iter().enumerate() {
-            let bone_matrix = matrices.get_current(&hurtbox.bone);
+            let bone_matrix = bone_indices
+                .0
+                .get(&hurtbox.bone)
+                .and_then(|index| matrices.get_current_index(*index));
 
             if let Some(bone_matrix) = bone_matrix {
                 let to_global = fighter_trf.mul(bone_matrix);
@@ -333,18 +374,15 @@ pub fn solve_hurtboxes(
 }
 
 pub fn intersect_attacks_with_hurtboxes(
+    roster: Res<FighterRoster>,
     mut query_attacker: Query<(Entity, &mut FighterHitboxes, &Fighter, &Player)>,
     query_attacked: Query<(Entity, &FighterSolvedHurtboxes, &Player)>,
     attack_scripts: Res<Assets<FighterAttackScript>>,
 ) {
-    let mut attackers = query_attacker.iter_mut().collect::<Vec<_>>();
+    for &entity_hurting in &roster.0 {
-    attackers
+        let Ok((_, mut hitboxes, _, _)) = query_attacker.get_mut(entity_hurting) else {
-        .sort_by(|(_, _, _, player_a), (_, _, _, player_b)| player_a.handle.cmp(&player_b.handle));
+            continue;
-
+        };
-    let mut receivers = query_attacked.iter().collect::<Vec<_>>();
-    receivers.sort_by(|(_, _, player_a), (_, _, player_b)| player_a.handle.cmp(&player_b.handle));
-
-    for (entity_hurting, mut hitboxes, _, _) in attackers {
         if hitboxes.attack_script.is_none() {
             continue;
         }
@@ -358,35 +396,36 @@ pub fn intersect_attacks_with_hurtboxes(
 
             let attack_script = attack_script.unwrap();
 
-            for (entity_receiving, hurtboxes, _) in receivers.iter() {
+            for &entity_receiving in &roster.0 {
-                if entity_hurting == *entity_receiving {
+                if entity_hurting == entity_receiving {
                     continue;
                 }
-                if hitboxes.hit_fighters.contains(entity_receiving) {
+                if hitboxes.hit_fighters.contains(&entity_receiving) {
                     continue;
                 }
-                let mut hitting_hitboxes = vec![];
+                let Ok((_, hurtboxes, _)) = query_attacked.get(entity_receiving) else {
+                    continue;
+                };
+                let mut hitting_hitbox_idx = None;
                 for hurtbox in hurtboxes.0.iter() {
                     for solved_hitbox in hitboxes.active_hitboxes_solved.iter() {
                         if hurtbox.capsule.intersects(solved_hitbox.capsule) {
-                            hitting_hitboxes.push(solved_hitbox.hitbox_idx);
+                            let candidate = solved_hitbox.hitbox_idx;
+                            if hitting_hitbox_idx.is_none_or(|current: usize| {
+                                attack_script.hitboxes[candidate].id
+                                    < attack_script.hitboxes[current].id
+                            }) {
+                                hitting_hitbox_idx = Some(candidate);
+                            }
                         }
                     }
                 }
 
-                hitting_hitboxes.sort_by(|a, b| {
+                let Some(hitting_hitbox_idx) = hitting_hitbox_idx else {
-                    let hitbox_id_a = attack_script.hitboxes[*a].id;
-                    let hitbox_id_b = attack_script.hitboxes[*b].id;
-
-                    hitbox_id_a.cmp(&hitbox_id_b)
-                });
-
-                if hitting_hitboxes.is_empty() {
                     continue;
-                }
+                };
 
-                let hitting_hitbox_idx = hitting_hitboxes[0];
+                hitboxes.hit_fighters.push(entity_receiving);
-                hitboxes.hit_fighters.push(*entity_receiving);
                 info!("Hit by hitbox {}!!!!", hitting_hitbox_idx);
             }
         }

src/fighter/attributes.rs

@@ -45,6 +45,13 @@ pub struct FighterAttributes {
 }
 
 impl FighterAttributes {
+    pub fn validate(&self) -> Result<(), String> {
+        if self.weight <= FGi32::ZERO {
+            return Err("weight must be positive".into());
+        }
+        Ok(())
+    }
+
     pub fn get_accel_and_target_dashrun(
         &self,
         input: &FighterInputFrame,

src/fighter/baked_animation.rs

@@ -191,6 +191,15 @@ pub struct BakedFighterAnimations {
 }
 
 impl BakedFighterAnimations {
+    pub fn contains_bone(&self, bone: &str) -> bool {
+        self.bone_names.iter().any(|name| name == bone)
+    }
+
+    pub fn has_unique_bone_names(&self) -> bool {
+        let mut names = std::collections::HashSet::with_capacity(self.bone_names.len());
+        self.bone_names.iter().all(|name| names.insert(name))
+    }
+
     pub fn sample_pose(&self, frame: &FighterAnimationFrame) -> Option<&[FixedMat4]> {
         let frames = &self.clip(frame.kind)?.frames;
         let frame_index = if frame.repeat {
@@ -230,6 +239,12 @@ pub struct FighterBoneMatrices {
     previous_local_matrices: Vec<FixedMat4>,
 }
 
+/// Immutable lookup table populated from the baked skeleton. Keeping this
+/// separate from rollback-tracked matrices avoids rebuilding string searches
+/// during every simulated frame.
+#[derive(Component, Clone, Debug, Default)]
+pub struct FighterBoneIndexMap(pub HashMap<String, usize>);
+
 #[derive(Component)]
 struct BakedFighterBone {
     fighter: Entity,
@@ -256,6 +271,17 @@ impl FighterBoneMatrices {
         self.matrices.get(index).copied()
     }
 
+    pub fn get_index(&self, index: usize) -> Option<(FixedMat4, FixedMat4)> {
+        Some((
+            *self.previous_frame_matrices.get(index)?,
+            *self.matrices.get(index)?,
+        ))
+    }
+
+    pub fn get_current_index(&self, index: usize) -> Option<FixedMat4> {
+        self.matrices.get(index).copied()
+    }
+
     pub fn iter(&self) -> impl Iterator<Item = (&str, FixedMat4)> {
         self.bone_names
             .iter()
@@ -331,11 +357,12 @@ pub fn update_fighter_bone_matrices(
         &FighterAnimations,
         &Fighter,
         &mut FighterBoneMatrices,
+        &mut FighterBoneIndexMap,
     )>,
     baked_assets: Res<Assets<BakedFighterAnimations>>,
     manifests: Res<Assets<FighterManifest>>,
 ) {
-    for (frame, animations, fighter, mut matrices) in &mut fighters {
+    for (frame, animations, fighter, mut matrices, mut bone_indices) in &mut fighters {
         let baked = baked_assets
             .get(&animations.baked)
             .expect("fighter baked animation asset should be loaded before the match starts");
@@ -355,6 +382,15 @@ pub fn update_fighter_bone_matrices(
             pose,
             manifest.model_scale,
         );
+        if bone_indices.0.is_empty() {
+            bone_indices.0.extend(
+                baked
+                    .bone_names
+                    .iter()
+                    .enumerate()
+                    .map(|(index, name)| (name.clone(), index)),
+            );
+        }
     }
 }
 

src/fighter/hurtbox.rs

@@ -52,11 +52,21 @@ impl FixedAffineCapsule {
         }
         let mut simplex = [FGVec3::ZERO; 4];
         let mut simplex_len = 0;
-        for _ in 0..20 {
+        let mut supports = [FGVec3::ZERO; 20];
+        let mut support_count = 0;
+        while support_count < supports.len() {
             let point = self.support(direction) - rhs.support(-direction);
             if point.dot(direction) < FGWide::ZERO {
                 return false;
             }
+            if supports[..support_count].contains(&point) {
+                // Fixed-point quantization can make GJK revisit a support
+                // point without making progress. This is not proof of an
+                // intersection, so terminate conservatively as a miss
+                return false;
+            }
+            supports[support_count] = point;
+            support_count += 1;
             for index in (1..=simplex_len.min(3)).rev() {
                 simplex[index] = simplex[index - 1];
             }
@@ -69,9 +79,10 @@ impl FixedAffineCapsule {
                 return true;
             }
         }
-        // A capped GJK iteration means the origin remains enclosed by the
+        // Exhaustion means the fixed-point solver could not prove an
-        // current simplex to fixed-point precision, so conservatively collide.
+        // intersection. Treating it as a hit creates phantom contacts for
-        true
+        // cycling or degenerate simplexes
+        false
     }
 }
 

src/fighter/manifest.rs

@@ -34,6 +34,21 @@ pub struct FighterHurtbox {
     pub radius: FGi32,
 }
 
+impl FighterHurtbox {
+    pub fn validate(&self) -> Result<(), String> {
+        if self.bone.trim().is_empty() {
+            return Err("bone name is empty".into());
+        }
+        if self.radius <= FGi32::ZERO {
+            return Err("radius must be positive".into());
+        }
+        if self.half_length < FGi32::ZERO {
+            return Err("half-length must be non-negative".into());
+        }
+        Ok(())
+    }
+}
+
 #[derive(Clone, Asset, Deserialize, Reflect, RonAsset)]
 pub struct FighterManifest {
     pub id: FighterId,

src/fighter/mod.rs

@@ -37,7 +37,7 @@ use crate::{
             FighterAttackPlugin, FighterAttackScriptAssets, FighterSolvedHitboxes,
             FighterSolvedHurtboxes,
         },
-        baked_animation::FighterBoneMatrices,
+        baked_animation::{FighterBoneIndexMap, FighterBoneMatrices},
         manifest::FighterManifest,
         state::{FighterState, fall::FallState},
         visual::FighterAnimations,
@@ -111,6 +111,9 @@ pub struct Fighter {
     pub manifest: Handle<FighterManifest>,
 }
 
+#[derive(Resource, Clone, Debug)]
+pub struct FighterRoster(pub Vec<Entity>);
+
 impl Plugin for FighterPlugin {
     fn build(&self, app: &mut App) {
         app.add_plugins(FighterAttackPlugin)
@@ -214,6 +217,7 @@ impl FighterHitboxes {
     pub fn clear(&mut self) {
         self.attack_script = None;
         self.active_hitboxes.clear();
+        self.active_hitboxes_solved.clear();
         self.hit_fighters.clear();
     }
 }
@@ -227,8 +231,9 @@ pub fn spawn_fighter(
     animations: FighterAnimations,
     attack_scripts: FighterAttackScriptAssets,
     visual_root: WorldAssetRoot,
-) {
+) -> Entity {
-    commands.spawn((
+    commands
+        .spawn((
             Name(format!("Fighter {player_handle}").into()),
             Player {
                 handle: player_handle,
@@ -261,11 +266,13 @@ pub fn spawn_fighter(
             animation::FighterAnimationFrame::new(animation::AnimKind::Wait, true),
             (
                 baked_animation::FighterBoneMatrices::default(),
+                FighterBoneIndexMap::default(),
                 FighterSolvedHurtboxes::default(),
                 FighterSolvedHitboxes::default(),
                 FighterVisual,
                 FighterInput::default(),
                 crate::camera::FighterCameraExtents::default(),
             ),
-    ));
+        ))
+        .id()
 }

src/fighter/state/ground_attack.rs

@@ -1,4 +1,3 @@
-
 use crate::fighter::{
     attack::{AttackKind, update_active_hitbox_list},
     state::{FighterState, FighterStateImpl, ground::GroundedStateCommon, wait::WaitState},

src/match_loading.rs

@@ -11,10 +11,10 @@ use crate::{
     AppState,
     args::Args,
     fighter::{
-        FighterId,
+        FighterId, FighterRoster,
         animation::AnimKind,
         attack::{AttackKind, FighterAttackScriptAssets},
-        baked_animation::BakedFighterAnimations,
+        baked_animation::{BAKED_ANIMATION_FPS, BakedFighterAnimations},
         manifest::{FighterManifest, FighterManifestRegistry},
         spawn_fighter,
         visual::FighterAnimations,
@@ -192,6 +192,7 @@ pub fn prepare_match(
     assets: Res<MatchAssets>,
     gltfs: Res<Assets<Gltf>>,
     baked_animations: Res<Assets<BakedFighterAnimations>>,
+    attack_scripts: Res<Assets<FighterAttackScript>>,
     registry: Res<FighterManifestRegistry>,
     stage_registry: Res<StageManifestRegistry>,
     manifests: Res<Assets<FighterManifest>>,
@@ -236,6 +237,89 @@ pub fn prepare_match(
             );
             return;
         };
+        if baked.frame_rate != BAKED_ANIMATION_FPS {
+            fail_match(
+                &mut next_state,
+                &format!(
+                    "{:?} baked animation rate is {}, expected {}",
+                    character.fighter, baked.frame_rate, BAKED_ANIMATION_FPS
+                ),
+            );
+            return;
+        }
+        if !baked.has_unique_bone_names() {
+            fail_match(
+                &mut next_state,
+                &format!(
+                    "{:?} baked animation contains duplicate bone names",
+                    character.fighter
+                ),
+            );
+            return;
+        }
+        if let Err(error) = manifest.attributes.validate() {
+            fail_match(
+                &mut next_state,
+                &format!("{:?} has invalid attributes: {error}", character.fighter),
+            );
+            return;
+        }
+        for (index, hurtbox) in manifest.hurtboxes.iter().enumerate() {
+            if let Err(error) = hurtbox.validate() {
+                fail_match(
+                    &mut next_state,
+                    &format!(
+                        "{:?} hurtbox {index} is invalid: {error}",
+                        character.fighter
+                    ),
+                );
+                return;
+            }
+            if !baked.contains_bone(&hurtbox.bone) {
+                fail_match(
+                    &mut next_state,
+                    &format!(
+                        "{:?} hurtbox {index} references missing bone {:?}",
+                        character.fighter, hurtbox.bone
+                    ),
+                );
+                return;
+            }
+        }
+        for (kind, handle) in &character.attack_scripts {
+            let Some(script) = attack_scripts.get(handle) else {
+                fail_match(
+                    &mut next_state,
+                    &format!(
+                        "{:?} {kind:?} attack script is unavailable",
+                        character.fighter
+                    ),
+                );
+                return;
+            };
+            if let Err(error) = script.validate() {
+                fail_match(
+                    &mut next_state,
+                    &format!(
+                        "{:?} {kind:?} attack script is invalid: {error}",
+                        character.fighter
+                    ),
+                );
+                return;
+            }
+            for hitbox in &script.hitboxes {
+                if !baked.contains_bone(&hitbox.bone) {
+                    fail_match(
+                        &mut next_state,
+                        &format!(
+                            "{:?} {kind:?} hitbox {} references missing bone {:?}",
+                            character.fighter, hitbox.id, hitbox.bone
+                        ),
+                    );
+                    return;
+                }
+            }
+        }
         let Some(scene) = gltf.scenes.first().cloned() else {
             fail_match(&mut next_state, "a character GLTF contains no scene");
             return;
@@ -303,11 +387,12 @@ pub fn prepare_match(
         stage_manifest.camera.clone(),
     );
 
+    let mut fighter_roster = Vec::with_capacity(request.players.len());
     for (index, (player, (scene, animations, attack_scripts, manifest_handle, model_scale))) in
         request.players.iter().zip(prepared).enumerate()
     {
         let spawn_x = (index as i32 * 2 + 1 - request.players.len() as i32) * 5;
-        spawn_fighter(
+        fighter_roster.push(spawn_fighter(
             &mut commands,
             player.handle,
             FGVec2::new(FGi32::from_num(spawn_x), FGi32::lit("12.5")),
@@ -316,9 +401,10 @@ pub fn prepare_match(
             animations,
             attack_scripts,
             WorldAssetRoot(scene),
-        );
+        ));
     }
 
+    commands.insert_resource(FighterRoster(fighter_roster));
     commands.insert_resource::<Session<GGRSCfg>>(session);
     next_state.set(AppState::InMatch);
 }
@@ -352,6 +438,7 @@ pub fn cleanup_match(
     commands.remove_resource::<stage::line::StageCollision>();
     commands.remove_resource::<MatchAssets>();
     commands.remove_resource::<PendingMatch>();
+    commands.remove_resource::<FighterRoster>();
 }
 
 #[cfg(test)]

src/menus/character_select/mod.rs

@@ -1,7 +1,4 @@
-use bevy::{
+use bevy::{picking::pointer::PointerId, prelude::*};
-    picking::pointer::PointerId,
-    prelude::*,
-};
 
 use crate::{
     AppState,

src/menus/main_menu.rs

@@ -1,6 +1,4 @@
-use bevy::{
+use bevy::{prelude::*, text::FontSourceTemplate};
-    prelude::*, text::FontSourceTemplate,
-};
 
 use crate::{
     AppState,

src/menus/mod.rs

@@ -100,10 +100,8 @@ where
     S::scene(props)
 }
 
-pub fn build_menu<S>(
+pub fn build_menu<S>(existing_menu: Option<Single<Entity, With<MenuMarker>>>, commands: Commands)
-    existing_menu: Option<Single<Entity, With<MenuMarker>>>,
+where
-    commands: Commands,
-) where
     S: SceneComponent + Default + Clone + Send + Sync,
 {
     spawn_menu(

src/menus/stage_select/mod.rs

@@ -5,12 +5,8 @@ use crate::{
     game_settings::CommonAssets,
     match_loading::initiate_match,
     menus::{
-        MenuMarker,
+        MenuMarker, cursor, match_config::MenuMatchConfig, spawn_menu,
-        cursor,
+        stage_select::stage_box::StageBox, style::FGUiStyle,
-        match_config::MenuMatchConfig,
-        spawn_menu,
-        stage_select::stage_box::StageBox,
-        style::FGUiStyle,
     },
     stage::manifest::{StageManifest, StageManifestRegistry},
 };

src/scripting/mod.rs

@@ -1,5 +1,6 @@
 use bevy::prelude::*;
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
 
 use crate::{fighter::attack::AttackHitbox, math::int::FGi32};
 
@@ -17,3 +18,19 @@ pub struct FighterAttackScript {
     pub iasa_frame: u32,
     pub wont_autocancel_window: (u32, u32),
 }
+
+impl FighterAttackScript {
+    pub fn validate(&self) -> Result<(), String> {
+        let mut ids = HashSet::with_capacity(self.hitboxes.len());
+        for hitbox in &self.hitboxes {
+            hitbox.validate()?;
+            if !ids.insert(hitbox.id) {
+                return Err(format!("duplicate hitbox id {}", hitbox.id));
+            }
+        }
+        if self.wont_autocancel_window.0 > self.wont_autocancel_window.1 {
+            return Err("autocancel window end precedes its start".into());
+        }
+        Ok(())
+    }
+}

src/scripting/move_compiler.rs

@@ -17,6 +17,16 @@ struct MoveCompiler {
     cursor: u32,
 }
 
+fn rhai_u32(value: i64, parameter: &str) -> Result<u32, Box<EvalAltResult>> {
+    u32::try_from(value).map_err(|_| {
+        format!(
+            "{parameter} must be between 0 and {}, got {value}",
+            u32::MAX
+        )
+        .into()
+    })
+}
+
 impl MoveCompiler {
     fn frame(&mut self, new_frame: u32) -> Result<(), Box<EvalAltResult>> {
         if new_frame < self.cursor {
@@ -87,11 +97,7 @@ impl MoveCompiler {
 
     fn into_attack_script(mut self) -> Result<FighterAttackScript, String> {
         self.remove_all_hitboxes().map_err(|err| err.to_string())?;
-        let mut hitboxes: Vec<_> = self
+        let mut hitboxes: Vec<_> = self.hitboxes.into_values().collect();
-            .hitboxes
-            .into_iter()
-            .map(|(_, hitbox)| hitbox)
-            .collect();
         hitboxes.sort_unstable_by_key(|hitbox| hitbox.id);
         let iasa_frame = self.iasa_frame.unwrap_or(100000);
         let wont_autocancel_window = self.wont_autocancel_window.unwrap_or((0, 100000));
@@ -141,7 +147,7 @@ pub fn compile_script(text: &str) -> Result<FighterAttackScript, String> {
                 "frame",
                 move |frame: i64| -> Result<(), Box<EvalAltResult>> {
                     let mut b = mc.borrow_mut();
-                    b.frame(frame as u32)
+                    b.frame(rhai_u32(frame, "frame")?)
                 },
             );
         }
@@ -160,10 +166,10 @@ pub fn compile_script(text: &str) -> Result<FighterAttackScript, String> {
         {
             let mc = mc.clone();
             engine.register_fn(
-                "remove_all_hitboxes",
+                "remove_hitbox",
                 move |id: i64| -> Result<(), Box<EvalAltResult>> {
                     let mut b = mc.borrow_mut();
-                    b.remove_hitbox(id as u32)
+                    b.remove_hitbox(rhai_u32(id, "hitbox id")?)
                 },
             );
         }
@@ -174,17 +180,24 @@ pub fn compile_script(text: &str) -> Result<FighterAttackScript, String> {
                 "set_wont_autocancel_window",
                 move |start: i64, end: i64| -> Result<(), Box<EvalAltResult>> {
                     let mut b = mc.borrow_mut();
-                    b.set_wont_autocancel_window(start as u32, end as u32)
+                    b.set_wont_autocancel_window(
+                        rhai_u32(start, "autocancel start")?,
+                        rhai_u32(end, "autocancel end")?,
+                    )
                 },
             );
         }
 
         {
             let mc = mc.clone();
-            engine.register_fn("set_iasa_frame", move |frame: i64| {
+            engine.register_fn(
+                "set_iasa_frame",
+                move |frame: i64| -> Result<(), Box<EvalAltResult>> {
                     let mut b = mc.borrow_mut();
-                b.set_iasa_frame(frame as u32);
+                    b.set_iasa_frame(rhai_u32(frame, "IASA frame")?);
-            });
+                    Ok(())
+                },
+            );
         }
 
         {
@@ -204,7 +217,7 @@ pub fn compile_script(text: &str) -> Result<FighterAttackScript, String> {
                     let mut b = mc.borrow_mut();
 
                     // Rhai does not have first class u32 support, so we have to do this.
-                    let id = id as u32;
+                    let id = rhai_u32(id, "hitbox id")?;
 
                     if b.hitboxes.contains_key(&id) {
                         return Err(format!("Hitbox with ID {} already existed!", id).into());
@@ -248,6 +261,8 @@ pub fn compile_script(text: &str) -> Result<FighterAttackScript, String> {
 mod tests {
     use super::*;
 
+    const HITBOX: &str = "hitbox(0, \"hand_l\", damage(\"1\"), vec3(\"0\", \"0\", \"0\"), fg32(\"1\"), angle(fg32(\"0\")), knockback_normal(), knockback(\"1\"), fg32(\"1\"));";
+
     #[test]
     fn compiled_hitboxes_are_sorted_by_authored_id() {
         let script = compile_script(
@@ -280,4 +295,25 @@ mod tests {
             vec![2, 3, 1]
         );
     }
+
+    #[test]
+    fn negative_and_overflowing_frames_are_rejected() {
+        for source in ["frame(-1);", "frame(4294967296);"] {
+            let error = match compile_script(source) {
+                Ok(_) => panic!("invalid frame was accepted"),
+                Err(error) => error,
+            };
+            assert!(error.contains("frame"));
+        }
+    }
+
+    #[test]
+    fn remove_hitbox_ends_only_the_named_hitbox() {
+        let script =
+            compile_script(&format!("frame(1); {HITBOX} frame(2); remove_hitbox(0);")).unwrap();
+
+        assert_eq!(script.hitboxes.len(), 1);
+        assert_eq!(script.hitboxes[0].start_frame, 1);
+        assert_eq!(script.hitboxes[0].end_frame, 2);
+    }
 }