refactor[physics] :: remove duplication and add constants

.github/workflows/pull_request_template.md

@@ -1,3 +1,5 @@
+<!-- PR Title Format: type[scope] :: short imperative description (e.g., feat[physics] :: add collision detection) -->
+
 ## Summary
 <!-- Brief, high-level description of what this PR changes -->
 -

.pre-commit-config.yaml

@@ -1,19 +1,28 @@
+---
 repos:
-- repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v6.0.0
-  hooks:
-  - id: trailing-whitespace
-  - id: end-of-file-fixer
-  - id: check-yaml
-  - id: check-json
-  - id: check-merge-conflict
-  - id: check-added-large-files
-  - id: check-case-conflict
-  - id: check-executables-have-shebangs
-  - id: check-shebang-scripts-are-executable
-  - id: check-symlinks
-  - id: destroyed-symlinks
-  - id: detect-private-key
-  - id: fix-byte-order-marker
-  - id: mixed-line-ending
-  - id: requirements-txt-fixer
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v6.0.0
+    hooks:
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+      - id: check-yaml
+      - id: check-json
+      - id: check-merge-conflict
+      - id: check-added-large-files
+      - id: check-case-conflict
+      - id: check-executables-have-shebangs
+      - id: check-shebang-scripts-are-executable
+      - id: check-symlinks
+      - id: destroyed-symlinks
+      - id: detect-private-key
+      - id: fix-byte-order-marker
+      - id: mixed-line-ending
+      - id: requirements-txt-fixer
+  - repo: local
+    hooks:
+      - id: clippy
+        name: clippy
+        entry: cargo clippy
+        language: system
+        pass_filenames: false
+        files: \.rs$

.yamllint

@@ -1,48 +1,25 @@
 ---
-# YAML linting configuration
-# See: https://yamllint.readthedocs.io/en/stable/configuration.html
-
 extends: default
-
 rules:
-  # Allow longer lines for readability in CI/CD
   line-length:
     max: 120
     level: warning
-
-  # Allow empty values in mappings
   empty-values:
     forbid-in-block-mappings: false
     forbid-in-flow-mappings: false
-
-  # Allow truthy values
   truthy:
-    allowed-values:
-      - 'true'
-      - 'false'
-      - 'on'
-      - 'off'
-      - 'yes'
-      - 'no'
+    allowed-values: ['true', 'false', 'on', 'off', 'yes', 'no']
     check-keys: false
-
-  # Allow comments
   comments:
     require-starting-space: false
     ignore-shebangs: true
-
-  # Allow indentation variations for readability
   indentation:
     spaces: 2
     indent-sequences: true
     check-multi-line-strings: false
-
-  # Allow trailing spaces in empty lines
   empty-lines:
     max-start: 1
     max-end: 1
     max: 2
-
-  # Allow brackets in flow style
   brackets:
     forbid: false

CONTRIBUTING.md

@@ -78,6 +78,43 @@ git checkout -b fix/issue-number-description
 - Add examples for new features
 - Keep API documentation current
 
+## Pull Request Title Format
+
+Use the following format for pull request titles:
+
+```
+<type>[<scope>] :: <short, imperative description>
+```
+
+### Breakdown
+
+* **`<type>`** — what kind of change this is
+  Common values:
+  * `feat` – new functionality
+  * `fix` – bug or build fix
+  * `refactor` – internal restructuring, no behavior change
+  * `perf` – performance improvement
+  * `docs` – documentation only
+  * `chore` – tooling, CI, deps
+  * `test` – tests only
+
+* **`<scope>`** — affected area (keep it short, lowercase)
+  Examples: `physics`, `engine`, `ci`, `ui`
+
+* **`::`** — visual separator
+
+* **`<short description>`**
+  * Imperative mood ("add", "fix", "improve", "refactor")
+  * ≤ 60–70 characters if possible
+  * No period at the end
+
+### Examples
+
+* `feat[physics] :: improve numerical stability`
+* `fix[engine] :: resolve collision detection bug`
+* `refactor[ci] :: update pre-commit hooks`
+* `chore[deps] :: upgrade rust dependencies`
+
 ## Commit Message Format
 
 We use [Conventional Commits](https://conventionalcommits.org/):

docs/Examples.md

@@ -75,30 +75,32 @@ fn main() {
 ## Integration with External Systems
 
 ```rust
-use simulation_engine::physics::PhysicsEngine;
+use simulation_engine::physics::{PhysicsEngine, Object};
 use nalgebra::Vector2;
 
 // Example: Reading data from a file or network
-fn load_objects_from_data(data: Vec<(f32, f32, f32, f32, f32)>) -> Vec<Object> {
+fn load_objects_from_data(data: Vec<(f32, f32, f32, f32, f32)>) -> Result<Vec<Object>, String> {
     data.into_iter()
         .map(|(x, y, vx, vy, mass)| {
             Object::new(Vector2::new(x, y), Vector2::new(vx, vy), mass)
         })
         .collect()
 }
 
-fn main() {
+fn main() -> Result<(), String> {
     let mut engine = PhysicsEngine::new();
 
     // Load objects from external source
     let data = vec![(0.0, 0.0, 1.0, 0.0, 1.0), (10.0, 0.0, -1.0, 0.0, 1.5)];
-    let objects = load_objects_from_data(data);
+    let objects = load_objects_from_data(data)?;
 
     for obj in objects {
-        engine.add_object(obj.position, obj.velocity, obj.mass);
+        engine.add_object(obj.position, obj.velocity, obj.mass)?;
     }
 
     // Simulate
-    engine.simulate(0.1);
+    engine.simulate(0.1)?;
+
+    Ok(())
 }
 ```

engine/constraints/physics_constraints.rs

@@ -59,8 +59,11 @@ impl PhysicsConstraint {
                 *position = *fixed_pos;
                 *velocity = Vector2::zeros();
             }
-            _ => {
-                // Other constraint types not yet implemented
+            Constraint::Distance { .. } => {
+                unimplemented!("Distance constraint not yet implemented");
+            }
+            Constraint::Axis { .. } => {
+                unimplemented!("Axis constraint not yet implemented");
             }
         }
     }

engine/main.rs

@@ -1,14 +1,12 @@
 extern crate env_logger;
 extern crate log;
+extern crate simulation_engine;
 
 use serde::Deserialize;
 use std::sync::Arc;
 use warp::Filter;
 
-mod managers;
-mod physics;
-
-use managers::simulation_manager::SimulationManager;
+use simulation_engine::managers::simulation_manager::SimulationManager;
 
 #[derive(Deserialize)]
 pub struct SimulationParams {

engine/numerics/constants.rs

@@ -19,3 +19,12 @@ pub const EARTH_GRAVITY: f32 = 9.81;
 /// Time step epsilon for floating point comparisons
 #[allow(dead_code)]
 pub const TIME_STEP_EPSILON: f32 = 1e-7;
+
+/// Perfectly elastic restitution coefficient (no energy loss on collision)
+pub const PERFECTLY_ELASTIC_RESTITUTION: f32 = 1.0;
+
+/// Damping factor applied when objects bounce off walls
+pub const WALL_BOUNCE_DAMPING_FACTOR: f32 = 0.8;
+
+/// Maximum coordinate value for position clamping to prevent numerical issues
+pub const MAX_COORDINATE_VALUE: f32 = 1e6;

engine/numerics/utilities.rs

@@ -1,6 +1,7 @@
+//! Numerical utilities and helper functions
+//! Provides safe mathematical operations and validation functions
+
 use crate::numerics::constants::*;
-/// Numerical utilities and helper functions
-/// Provides safe mathematical operations and validation functions
 use nalgebra::Vector2;
 
 /// Clamps a value between min and max

engine/physics/physics_engine.rs

@@ -1,48 +1,6 @@
+use crate::numerics::*;
 use nalgebra::Vector2;
 
-// TODO: These are duplicated from the numerics module.
-// The proper solution is to import from crate::numerics::*,
-// but there's currently a module resolution issue preventing this.
-// Once resolved, these inline implementations should be removed
-// and replaced with: use crate::numerics::*;
-const MIN_DISTANCE_EPSILON: f32 = 1e-6;
-const MAX_VELOCITY: f32 = 1000.0;
-
-fn clamp<T: PartialOrd>(value: T, min: T, max: T) -> T {
-    if value < min {
-        min
-    } else if value > max {
-        max
-    } else {
-        value
-    }
-}
-
-fn safe_divide(numerator: f32, denominator: f32, default: f32) -> f32 {
-    if denominator.abs() < MIN_DISTANCE_EPSILON {
-        default
-    } else {
-        numerator / denominator
-    }
-}
-
-fn clamp_velocity(velocity: &mut Vector2<f32>) {
-    velocity.x = clamp(velocity.x, -MAX_VELOCITY, MAX_VELOCITY);
-    velocity.y = clamp(velocity.y, -MAX_VELOCITY, MAX_VELOCITY);
-}
-
-fn validate_vector(vec: &Vector2<f32>) -> Result<(), String> {
-    if !vec.x.is_finite() || !vec.y.is_finite() {
-        Err(format!("Vector contains non-finite values: {:?}", vec))
-    } else {
-        Ok(())
-    }
-}
-
-fn approx_eq(a: f32, b: f32, epsilon: f32) -> bool {
-    (a - b).abs() < epsilon
-}
-
 #[derive(Clone, Debug)]
 pub struct Object {
     pub position: Vector2<f32>,
@@ -53,28 +11,9 @@ pub struct Object {
 
 impl Object {
     pub fn new(position: Vector2<f32>, velocity: Vector2<f32>, mass: f32) -> Result<Self, String> {
-        // Temporary inline validation
-        if mass < 1e-3 {
-            return Err(format!("Mass {} is below minimum threshold {}", mass, 1e-3));
-        }
-        if mass > 1e6 {
-            return Err(format!("Mass {} exceeds maximum threshold {}", mass, 1e6));
-        }
-        if !mass.is_finite() {
-            return Err("Mass must be finite".to_string());
-        }
-        if !position.x.is_finite() || !position.y.is_finite() {
-            return Err(format!(
-                "Position contains non-finite values: {:?}",
-                position
-            ));
-        }
-        if !velocity.x.is_finite() || !velocity.y.is_finite() {
-            return Err(format!(
-                "Velocity contains non-finite values: {:?}",
-                velocity
-            ));
-        }
+        let mass = validate_mass(mass)?;
+        validate_vector(&position)?;
+        validate_vector(&velocity)?;
 
         Ok(Object {
             position,
@@ -103,8 +42,8 @@ impl PhysicsEngine {
         let default_object = Object::new(Vector2::new(0.0, 0.0), Vector2::new(0.0, 0.0), 1.0)?;
 
         Ok(PhysicsEngine {
-            objects: vec![default_object],     // default object
-            gravity: Vector2::new(0.0, -9.81), // downward acceleration
+            objects: vec![default_object],              // default object
+            gravity: Vector2::new(0.0, -EARTH_GRAVITY), // downward acceleration
             bounds: (Vector2::new(-100.0, -100.0), Vector2::new(100.0, 100.0)),
         })
     }
@@ -185,8 +124,8 @@ impl PhysicsEngine {
         // Elastic collision impulse calculation
         let m1 = self.objects[i].mass;
         let m2 = self.objects[j].mass;
-        let restitution = 1.0; // perfectly elastic
-        let impulse_scalar = -(1.0 + restitution) * velocity_along_normal * (m1 * m2) / (m1 + m2);
+        let restitution = PERFECTLY_ELASTIC_RESTITUTION;
+        let impulse_scalar = -(1.0 + restitution) * velocity_along_normal / (1.0 / m1 + 1.0 / m2);
         let impulse = impulse_scalar * normal;
 
         // Calculate separation to prevent overlap
@@ -251,17 +190,21 @@ impl PhysicsEngine {
             obj.position.x = clamp(obj.position.x, self.bounds.0.x, self.bounds.1.x);
             obj.position.y = clamp(obj.position.y, self.bounds.0.y, self.bounds.1.y);
 
-            // Bounce off walls with damping
-            if approx_eq(obj.position.x, self.bounds.0.x, MIN_DISTANCE_EPSILON)
-                || approx_eq(obj.position.x, self.bounds.1.x, MIN_DISTANCE_EPSILON)
+            // Bounce off walls with damping only if moving towards the boundary
+            if (approx_eq(obj.position.x, self.bounds.0.x, MIN_DISTANCE_EPSILON)
+                && obj.velocity.x < 0.0)
+                || (approx_eq(obj.position.x, self.bounds.1.x, MIN_DISTANCE_EPSILON)
+                    && obj.velocity.x > 0.0)
             {
-                obj.velocity.x = -obj.velocity.x * 0.8;
+                obj.velocity.x = -obj.velocity.x * WALL_BOUNCE_DAMPING_FACTOR;
                 bounced = true;
             }
-            if approx_eq(obj.position.y, self.bounds.0.y, MIN_DISTANCE_EPSILON)
-                || approx_eq(obj.position.y, self.bounds.1.y, MIN_DISTANCE_EPSILON)
+            if (approx_eq(obj.position.y, self.bounds.0.y, MIN_DISTANCE_EPSILON)
+                && obj.velocity.y < 0.0)
+                || (approx_eq(obj.position.y, self.bounds.1.y, MIN_DISTANCE_EPSILON)
+                    && obj.velocity.y > 0.0)
             {
-                obj.velocity.y = -obj.velocity.y * 0.8;
+                obj.velocity.y = -obj.velocity.y * WALL_BOUNCE_DAMPING_FACTOR;
                 bounced = true;
             }
 
@@ -276,8 +219,8 @@ impl PhysicsEngine {
         for obj in &mut self.objects {
             clamp_velocity(&mut obj.velocity);
             // Ensure position stays within reasonable bounds (expand if needed)
-            obj.position.x = clamp(obj.position.x, -1e6, 1e6);
-            obj.position.y = clamp(obj.position.y, -1e6, 1e6);
+            obj.position.x = clamp(obj.position.x, -MAX_COORDINATE_VALUE, MAX_COORDINATE_VALUE);
+            obj.position.y = clamp(obj.position.y, -MAX_COORDINATE_VALUE, MAX_COORDINATE_VALUE);
         }
     }
 
@@ -302,7 +245,7 @@ mod tests {
         let time_step = 0.1;
         engine.simulate(time_step).unwrap();
         // After one step, velocity.y should be gravity.y * time_step
-        let expected_velocity_y = -9.81 * time_step;
+        let expected_velocity_y = -EARTH_GRAVITY * time_step;
         assert!((engine.objects[0].velocity.y - expected_velocity_y).abs() < 0.01);
         // Position.y should be velocity.y * time_step
         let expected_position_y = expected_velocity_y * time_step;