diff --git a/src/fighter/attack.rs b/src/fighter/attack.rs index 39f59d8..7d6d0c1 100644 --- a/src/fighter/attack.rs +++ b/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); 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, frame: u32) { - *list = script - .hitboxes - .iter() - .enumerate() - .filter(|(_, hb)| hb.start_frame <= frame && hb.end_frame > frame) - .map(|(i, _)| i) - .collect() + list.clear(); + list.extend( + script + .hitboxes + .iter() + .enumerate() + .filter(|(_, hb)| hb.start_frame <= frame && hb.end_frame > frame) + .map(|(i, _)| i), + ); } #[derive(Clone, Debug, PartialEq, Hash)] @@ -213,6 +238,7 @@ pub struct FighterSolvedHitboxes(pub Vec); 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(); - if let Some(ref attack_script) = hitboxes.attack_script { + let FighterHitboxes { + 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 { + for hitbox_idx in active_hitboxes.iter().copied() { 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); pub fn solve_hurtboxes( mut query: Query<( &FighterBoneMatrices, + &FighterBoneIndexMap, &FighterTranslation, &FighterFacingDirection, &mut FighterSolvedHurtboxes, @@ -301,14 +337,19 @@ pub fn solve_hurtboxes( )>, manifests: Res>, ) { - 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, mut query_attacker: Query<(Entity, &mut FighterHitboxes, &Fighter, &Player)>, query_attacked: Query<(Entity, &FighterSolvedHurtboxes, &Player)>, attack_scripts: Res>, ) { - let mut attackers = query_attacker.iter_mut().collect::>(); - attackers - .sort_by(|(_, _, _, player_a), (_, _, _, player_b)| player_a.handle.cmp(&player_b.handle)); - - let mut receivers = query_attacked.iter().collect::>(); - receivers.sort_by(|(_, _, player_a), (_, _, player_b)| player_a.handle.cmp(&player_b.handle)); - - for (entity_hurting, mut hitboxes, _, _) in attackers { + for &entity_hurting in &roster.0 { + let Ok((_, mut hitboxes, _, _)) = query_attacker.get_mut(entity_hurting) else { + continue; + }; 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() { - if entity_hurting == *entity_receiving { + for &entity_receiving in &roster.0 { + 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 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() { + let Some(hitting_hitbox_idx) = hitting_hitbox_idx else { 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); } } diff --git a/src/fighter/attributes.rs b/src/fighter/attributes.rs index 6d011b5..6985fde 100644 --- a/src/fighter/attributes.rs +++ b/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, diff --git a/src/fighter/baked_animation.rs b/src/fighter/baked_animation.rs index 0bfe9fe..4b0fc2a 100644 --- a/src/fighter/baked_animation.rs +++ b/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, } +/// 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); + #[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 { + self.matrices.get(index).copied() + } + pub fn iter(&self) -> impl Iterator { self.bone_names .iter() @@ -331,11 +357,12 @@ pub fn update_fighter_bone_matrices( &FighterAnimations, &Fighter, &mut FighterBoneMatrices, + &mut FighterBoneIndexMap, )>, baked_assets: Res>, manifests: Res>, ) { - 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)), + ); + } } } diff --git a/src/fighter/hurtbox.rs b/src/fighter/hurtbox.rs index 6f2b221..fcd6b8c 100644 --- a/src/fighter/hurtbox.rs +++ b/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 - // current simplex to fixed-point precision, so conservatively collide. - true + // Exhaustion means the fixed-point solver could not prove an + // intersection. Treating it as a hit creates phantom contacts for + // cycling or degenerate simplexes + false } } diff --git a/src/fighter/manifest.rs b/src/fighter/manifest.rs index 69376c9..76b8cec 100644 --- a/src/fighter/manifest.rs +++ b/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, diff --git a/src/fighter/mod.rs b/src/fighter/mod.rs index ae03b41..7c2edb8 100644 --- a/src/fighter/mod.rs +++ b/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, } +#[derive(Resource, Clone, Debug)] +pub struct FighterRoster(pub Vec); + 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,45 +231,48 @@ pub fn spawn_fighter( animations: FighterAnimations, attack_scripts: FighterAttackScriptAssets, visual_root: WorldAssetRoot, -) { - commands.spawn(( - Name(format!("Fighter {player_handle}").into()), - Player { - handle: player_handle, - }, - Fighter { manifest: manifest }, - FighterFacingDirection::Right, - FighterPreviousFacingDirection(FighterFacingDirection::Right), - Transform::from_scale(Vec3::splat(model_scale.to_num())), - ( - FighterECB { - vertical_half: FGi32::lit("5.0"), - horizontal_half: FGi32::lit("2.5"), +) -> Entity { + commands + .spawn(( + Name(format!("Fighter {player_handle}").into()), + Player { + handle: player_handle, }, - FighterHitboxes { - attack_script: None, - active_hitboxes: vec![], - active_hitboxes_solved: vec![], - hit_fighters: vec![], - }, - ), - ( - FighterVelocity::default(), - FighterTranslation(spawn_position), - FighterPreviousTranslation(spawn_position), - ), - visual_root, - animations, - attack_scripts, - FighterState::Fall(FallState), - animation::FighterAnimationFrame::new(animation::AnimKind::Wait, true), - ( - baked_animation::FighterBoneMatrices::default(), - FighterSolvedHurtboxes::default(), - FighterSolvedHitboxes::default(), - FighterVisual, - FighterInput::default(), - crate::camera::FighterCameraExtents::default(), - ), - )); + Fighter { manifest: manifest }, + FighterFacingDirection::Right, + FighterPreviousFacingDirection(FighterFacingDirection::Right), + Transform::from_scale(Vec3::splat(model_scale.to_num())), + ( + FighterECB { + vertical_half: FGi32::lit("5.0"), + horizontal_half: FGi32::lit("2.5"), + }, + FighterHitboxes { + attack_script: None, + active_hitboxes: vec![], + active_hitboxes_solved: vec![], + hit_fighters: vec![], + }, + ), + ( + FighterVelocity::default(), + FighterTranslation(spawn_position), + FighterPreviousTranslation(spawn_position), + ), + visual_root, + animations, + attack_scripts, + FighterState::Fall(FallState), + 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() } diff --git a/src/fighter/state/ground_attack.rs b/src/fighter/state/ground_attack.rs index 3751f0a..e78d099 100644 --- a/src/fighter/state/ground_attack.rs +++ b/src/fighter/state/ground_attack.rs @@ -1,8 +1,7 @@ - use crate::fighter::{ - attack::{AttackKind, update_active_hitbox_list}, - state::{FighterState, FighterStateImpl, ground::GroundedStateCommon, wait::WaitState}, - }; + attack::{AttackKind, update_active_hitbox_list}, + state::{FighterState, FighterStateImpl, ground::GroundedStateCommon, wait::WaitState}, +}; #[derive(Clone, Debug, Hash)] pub struct GroundAttackState { diff --git a/src/match_loading.rs b/src/match_loading.rs index d9eb507..630f4cf 100644 --- a/src/match_loading.rs +++ b/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, gltfs: Res>, baked_animations: Res>, + attack_scripts: Res>, registry: Res, stage_registry: Res, manifests: Res>, @@ -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); next_state.set(AppState::InMatch); } @@ -352,6 +438,7 @@ pub fn cleanup_match( commands.remove_resource::(); commands.remove_resource::(); commands.remove_resource::(); + commands.remove_resource::(); } #[cfg(test)] diff --git a/src/menus/character_select/mod.rs b/src/menus/character_select/mod.rs index 87f03dd..d20b911 100644 --- a/src/menus/character_select/mod.rs +++ b/src/menus/character_select/mod.rs @@ -1,7 +1,4 @@ -use bevy::{ - picking::pointer::PointerId, - prelude::*, -}; +use bevy::{picking::pointer::PointerId, prelude::*}; use crate::{ AppState, diff --git a/src/menus/main_menu.rs b/src/menus/main_menu.rs index 458974b..eeb96b0 100644 --- a/src/menus/main_menu.rs +++ b/src/menus/main_menu.rs @@ -1,6 +1,4 @@ -use bevy::{ - prelude::*, text::FontSourceTemplate, -}; +use bevy::{prelude::*, text::FontSourceTemplate}; use crate::{ AppState, diff --git a/src/menus/mod.rs b/src/menus/mod.rs index 322912d..c814473 100644 --- a/src/menus/mod.rs +++ b/src/menus/mod.rs @@ -100,10 +100,8 @@ where S::scene(props) } -pub fn build_menu( - existing_menu: Option>>, - commands: Commands, -) where +pub fn build_menu(existing_menu: Option>>, commands: Commands) +where S: SceneComponent + Default + Clone + Send + Sync, { spawn_menu( diff --git a/src/menus/stage_select/mod.rs b/src/menus/stage_select/mod.rs index 0c256da..5108a0c 100644 --- a/src/menus/stage_select/mod.rs +++ b/src/menus/stage_select/mod.rs @@ -5,12 +5,8 @@ use crate::{ game_settings::CommonAssets, match_loading::initiate_match, menus::{ - MenuMarker, - cursor, - match_config::MenuMatchConfig, - spawn_menu, - stage_select::stage_box::StageBox, - style::FGUiStyle, + MenuMarker, cursor, match_config::MenuMatchConfig, spawn_menu, + stage_select::stage_box::StageBox, style::FGUiStyle, }, stage::manifest::{StageManifest, StageManifestRegistry}, }; diff --git a/src/scripting/mod.rs b/src/scripting/mod.rs index 7ff8fa8..de83d1b 100644 --- a/src/scripting/mod.rs +++ b/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(()) + } +} diff --git a/src/scripting/move_compiler.rs b/src/scripting/move_compiler.rs index 7fba60b..474234b 100644 --- a/src/scripting/move_compiler.rs +++ b/src/scripting/move_compiler.rs @@ -17,6 +17,16 @@ struct MoveCompiler { cursor: u32, } +fn rhai_u32(value: i64, parameter: &str) -> Result> { + 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> { if new_frame < self.cursor { @@ -87,11 +97,7 @@ impl MoveCompiler { fn into_attack_script(mut self) -> Result { self.remove_all_hitboxes().map_err(|err| err.to_string())?; - let mut hitboxes: Vec<_> = self - .hitboxes - .into_iter() - .map(|(_, hitbox)| hitbox) - .collect(); + let mut hitboxes: Vec<_> = self.hitboxes.into_values().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 { "frame", move |frame: i64| -> Result<(), Box> { 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 { { let mc = mc.clone(); engine.register_fn( - "remove_all_hitboxes", + "remove_hitbox", move |id: i64| -> Result<(), Box> { 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 { "set_wont_autocancel_window", move |start: i64, end: i64| -> Result<(), Box> { 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| { - let mut b = mc.borrow_mut(); - b.set_iasa_frame(frame as u32); - }); + engine.register_fn( + "set_iasa_frame", + move |frame: i64| -> Result<(), Box> { + let mut b = mc.borrow_mut(); + b.set_iasa_frame(rhai_u32(frame, "IASA frame")?); + Ok(()) + }, + ); } { @@ -204,7 +217,7 @@ pub fn compile_script(text: &str) -> Result { 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 { 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); + } }