From a15e392b8b61a14a84d65b587306c9f59be14ed9 Mon Sep 17 00:00:00 2001
From: Saverio Miroddi <saverio.pub2@gmail.com>
Date: Tue, 2 Aug 2022 22:21:49 +0200
Subject: [PATCH] Update the player movement clamper by using Bevy's
 SystemParam

Still not a good design, but SystemParam is much better than a helper system.
---
 src/attack.rs   |  20 +----
 src/movement.rs | 197 +++++++++++++++++++++++-------------------------
 2 files changed, 97 insertions(+), 120 deletions(-)

diff --git a/src/attack.rs b/src/attack.rs
index 1ed2a661..2901e4d9 100644
--- a/src/attack.rs
+++ b/src/attack.rs
@@ -23,13 +23,10 @@ use crate::{
         self, ATTACK_HEIGHT, ATTACK_LAYER, ATTACK_WIDTH, ITEM_BOTTLE_NAME, ITEM_HEIGHT, ITEM_LAYER,
         ITEM_WIDTH, THROW_ITEM_ROTATION_SPEED,
     },
-    enemy::SpawnLocationX,
     input::PlayerAction,
     item::item_carried_by_player,
-    metadata::{FighterMeta, GameMeta, ItemMeta, LevelMeta},
-    movement::{
-        clamp_player_movements, LeftMovementBoundary, MoveInArc, MoveInDirection, Rotate, Target,
-    },
+    metadata::{FighterMeta, ItemMeta},
+    movement::{MoveInArc, MoveInDirection, PlayerMovementClamper, Rotate, Target},
     state::State,
     ArrivedEvent, Enemy, GameState, Player, Stats,
 };
@@ -298,12 +295,9 @@ fn player_flop(
         ),
         With<Player>,
     >,
-    enemy_spawn_locations_query: Query<&SpawnLocationX>,
-    level_meta: Res<LevelMeta>,
+    player_movement_clamper: PlayerMovementClamper,
     fighter_assets: Res<Assets<FighterMeta>>,
     time: Res<Time>,
-    left_movement_boundary: Res<LeftMovementBoundary>,
-    game_meta: Res<GameMeta>,
     mut start_y: Local<Option<f32>>,
 ) {
     let players_movement = query
@@ -390,13 +384,7 @@ fn player_flop(
         )
         .collect::<Vec<_>>();
 
-    let players_movement = clamp_player_movements(
-        players_movement,
-        &enemy_spawn_locations_query,
-        &level_meta,
-        &left_movement_boundary,
-        &game_meta,
-    );
+    let players_movement = player_movement_clamper.clamp(players_movement);
 
     for ((_, _, mut transform, _, _, _, _, _), player_dir) in query.iter_mut().zip(players_movement)
     {
diff --git a/src/movement.rs b/src/movement.rs
index 05edd1f1..bc3a4f24 100644
--- a/src/movement.rs
+++ b/src/movement.rs
@@ -1,4 +1,5 @@
 use bevy::{
+    ecs::system::SystemParam,
     math::{Quat, Vec2, Vec3},
     prelude::{
         Commands, Component, Deref, DerefMut, Entity, EventWriter, Query, Res, ResMut, Transform,
@@ -41,11 +42,8 @@ pub struct Knockback {
 pub fn knockback_system(
     mut commands: Commands,
     mut query: Query<(Entity, &mut Transform, &mut Knockback, Option<&Player>)>,
-    enemy_spawn_locations_query: Query<&SpawnLocationX>,
-    level_meta: Res<LevelMeta>,
+    player_movement_clamper: PlayerMovementClamper,
     time: Res<Time>,
-    game_meta: Res<GameMeta>,
-    left_movement_boundary: Res<LeftMovementBoundary>,
 ) {
     let mut all_knockbacks = query.iter_mut().collect::<Vec<_>>();
 
@@ -88,13 +86,7 @@ pub fn knockback_system(
         })
         .collect::<Vec<_>>();
 
-    let player_dirs = clamp_player_movements(
-        player_movements,
-        &enemy_spawn_locations_query,
-        &level_meta,
-        &left_movement_boundary,
-        &game_meta,
-    );
+    let player_dirs = player_movement_clamper.clamp(player_movements);
 
     for ((_, transform, _, _), player_dir) in player_knockbacks.iter_mut().zip(player_dirs) {
         transform.translation += player_dir.unwrap().extend(0.);
@@ -112,11 +104,8 @@ pub fn player_controller(
         ),
         With<Player>,
     >,
-    enemy_spawn_locations_query: Query<&SpawnLocationX>,
-    level_meta: Res<LevelMeta>,
+    player_movement_clamper: PlayerMovementClamper,
     time: Res<Time>,
-    game_meta: Res<GameMeta>,
-    left_movement_boundary: Res<LeftMovementBoundary>,
 ) {
     // Compute the new direction vectors; can be None if the state is not (idle or running).
     //
@@ -141,13 +130,7 @@ pub fn player_controller(
         })
         .collect::<Vec<_>>();
 
-    let player_dirs = clamp_player_movements(
-        player_movements,
-        &enemy_spawn_locations_query,
-        &level_meta,
-        &left_movement_boundary,
-        &game_meta,
-    );
+    let player_dirs = player_movement_clamper.clamp(player_movements);
 
     for ((mut state, _, mut transform, mut facing, _), dir) in
         query.iter_mut().zip(player_dirs.iter())
@@ -303,104 +286,110 @@ pub fn update_left_movement_boundary(
     }
 }
 
-/// Returns the direction vectors, with X/Y clamping.
-///
-/// Not a system, but a utility method!.
-///
-/// player_movements: array of (location, direction vector).
-/// enemy_spawn_location_query: spawn locations of _alive_ enemies.
-///
-/// WATCH OUT! All players must be included, even if they don't move, in which case, pass
-/// None as direction. This is because clamping is based on the position of _all_ the
-/// players.
-/// It's possible to pass an empty array; this can happen if the system doesn't guard the case
-/// where all the players are dead; an empty array will be returned.
-pub fn clamp_player_movements(
-    mut player_movements: Vec<(Vec3, Option<Vec2>)>,
-    enemy_spawn_locations_query: &Query<&SpawnLocationX>,
-    level_meta: &LevelMeta,
-    left_movement_boundary: &LeftMovementBoundary,
-    game_meta: &GameMeta,
-) -> Vec<Option<Vec2>> {
-    // In the first pass, we check the camera stop points. If a player is moving across a stop
-    // point, all the enemies up to that point must have been defeated, in order to move.
-
-    let current_stop_point = level_meta.stop_points.iter().find(|point_x| {
-        player_movements.iter().any(|(location, dir)| {
-            if let Some(dir) = dir {
-                location.x < **point_x && **point_x <= location.x + dir.x
-            } else {
-                false
-            }
-        })
-    });
-
-    if let Some(current_stop_point) = current_stop_point {
-        let any_enemy_behind_stop_point = enemy_spawn_locations_query
-            .iter()
-            .any(|SpawnLocationX(spawn_x)| spawn_x <= current_stop_point);
+#[derive(SystemParam)]
+pub struct PlayerMovementClamper<'w, 's> {
+    enemy_spawn_locations_query: Query<'w, 's, &'static SpawnLocationX>,
+    level_meta: Res<'w, LevelMeta>,
+    game_meta: Res<'w, GameMeta>,
+    left_movement_boundary: Res<'w, LeftMovementBoundary>,
+}
 
-        if any_enemy_behind_stop_point {
-            for (location, movement) in player_movements.iter_mut() {
-                if let Some(movement) = movement.as_mut() {
-                    // Can be simplified, but it's harder to understand.
-                    if location.x + movement.x > *current_stop_point {
-                        movement.x = 0.;
+impl<'w, 's> PlayerMovementClamper<'w, 's> {
+    /// Returns the direction vectors, with X/Y clamping.
+    ///
+    /// Not a system, but a utility method!.
+    ///
+    /// player_movements: array of (location, direction vector).
+    /// enemy_spawn_location_query: spawn locations of _alive_ enemies.
+    ///
+    /// WATCH OUT! All players must be included, even if they don't move, in which case, pass
+    /// None as direction. This is because clamping is based on the position of _all_ the
+    /// players.
+    /// It's possible to pass an empty array; this can happen if the system doesn't guard the case
+    /// where all the players are dead; an empty array will be returned.
+    pub fn clamp(&self, mut player_movements: Vec<(Vec3, Option<Vec2>)>) -> Vec<Option<Vec2>> {
+        // In the first pass, we check the camera stop points. If a player is moving across a stop
+        // point, all the enemies up to that point must have been defeated, in order to move.
+
+        let current_stop_point = self.level_meta.stop_points.iter().find(|point_x| {
+            player_movements.iter().any(|(location, dir)| {
+                if let Some(dir) = dir {
+                    location.x < **point_x && **point_x <= location.x + dir.x
+                } else {
+                    false
+                }
+            })
+        });
+
+        if let Some(current_stop_point) = current_stop_point {
+            let any_enemy_behind_stop_point = self
+                .enemy_spawn_locations_query
+                .iter()
+                .any(|SpawnLocationX(spawn_x)| spawn_x <= current_stop_point);
+
+            if any_enemy_behind_stop_point {
+                for (location, movement) in player_movements.iter_mut() {
+                    if let Some(movement) = movement.as_mut() {
+                        // Can be simplified, but it's harder to understand.
+                        if location.x + movement.x > *current_stop_point {
+                            movement.x = 0.;
+                        }
                     }
                 }
             }
         }
-    }
 
-    // Then, we perform the absolute clamping (screen top/left/bottom), and we collect the data
-    // required for the relative clamping.
+        // Then, we perform the absolute clamping (screen top/left/bottom), and we collect the data
+        // required for the relative clamping.
 
-    let mut min_new_player_x = f32::MAX;
+        let mut min_new_player_x = f32::MAX;
 
-    let player_movements = player_movements
-        .iter()
-        .map(|(location, movement)| {
-            let new_movement = movement.map(|mut movement| {
-                let new_x = location.x + movement.x;
+        let player_movements = player_movements
+            .iter()
+            .map(|(location, movement)| {
+                let new_movement = movement.map(|mut movement| {
+                    let new_x = location.x + movement.x;
 
-                if new_x < left_movement_boundary.0 {
-                    movement.x = 0.;
-                }
+                    if new_x < self.left_movement_boundary.0 {
+                        movement.x = 0.;
+                    }
 
-                //Restrict player to the ground
-                let new_y = location.y + movement.y + consts::GROUND_OFFSET;
+                    //Restrict player to the ground
+                    let new_y = location.y + movement.y + consts::GROUND_OFFSET;
 
-                if new_y >= consts::MAX_Y || new_y <= consts::MIN_Y {
-                    movement.y = 0.;
-                }
+                    if new_y >= consts::MAX_Y || new_y <= consts::MIN_Y {
+                        movement.y = 0.;
+                    }
 
-                (movement, new_x)
-            });
+                    (movement, new_x)
+                });
 
-            if let Some((_, new_x)) = new_movement {
-                min_new_player_x = min_new_player_x.min(new_x);
-            } else {
-                min_new_player_x = min_new_player_x.min(location.x);
-            }
+                if let Some((_, new_x)) = new_movement {
+                    min_new_player_x = min_new_player_x.min(new_x);
+                } else {
+                    min_new_player_x = min_new_player_x.min(location.x);
+                }
 
-            (location, new_movement)
-        })
-        .collect::<Vec<_>>();
+                (location, new_movement)
+            })
+            .collect::<Vec<_>>();
 
-    // Then, we perform the clamping of the players relative to each other.
+        // Then, we perform the clamping of the players relative to each other.
 
-    let max_players_x_distance = LEFT_BOUNDARY_MAX_DISTANCE + game_meta.camera_move_right_boundary;
+        let max_players_x_distance =
+            LEFT_BOUNDARY_MAX_DISTANCE + self.game_meta.camera_move_right_boundary;
 
-    player_movements
-        .iter()
-        .map(|(_, player_movement)| {
-            player_movement.map(|(player_dir, new_player_x)| {
-                if new_player_x > min_new_player_x + max_players_x_distance {
-                    Vec2::ZERO
-                } else {
-                    player_dir
-                }
+        player_movements
+            .iter()
+            .map(|(_, player_movement)| {
+                player_movement.map(|(player_dir, new_player_x)| {
+                    if new_player_x > min_new_player_x + max_players_x_distance {
+                        Vec2::ZERO
+                    } else {
+                        player_dir
+                    }
+                })
             })
-        })
-        .collect::<Vec<_>>()
+            .collect::<Vec<_>>()
+    }
 }