improve command handling

turtle-lib/src/command_behavior.rs

@@ -0,0 +1,145 @@
+//! Centralised behavioural contract for `TurtleCommand`.
+//!
+//! All knowledge about what a command does to `TurtleParams`, how long it
+//! animates, and whether it produces a drawable stroke lives here.
+//!
+//! Adding a new `TurtleCommand` variant requires editing this file (and
+//! `execute_command_side_effects` / `tessellate_command` in `execution.rs`
+//! if the variant has side effects or produces a mesh).
+
+use crate::circle_geometry::{CircleDirection, CircleGeometry};
+use crate::commands::TurtleCommand;
+use crate::general::AnimationSpeed;
+use crate::state::TurtleParams;
+use crate::tweening::normalize_angle;
+use macroquad::prelude::vec2;
+
+impl TurtleCommand {
+    /// Apply this command's effect to `params` in place.
+    ///
+    /// This is the **single source of truth** for what a command changes in
+    /// `TurtleParams`. Used by:
+    /// - `execute_command()` — instant-mode path, after side-effects return `false`
+    /// - `TweenController::calculate_target_state()` — animated-mode target computation
+    ///
+    /// Variants handled by `execute_command_side_effects` (`BeginFill`, `EndFill`,
+    /// `PenUp`, `PenDown`, `WriteText`, `Reset`) are included here so that
+    /// `calculate_target_state` can produce a correct tween target.  In the
+    /// `execute_command()` call path those variants never reach this method because
+    /// `execute_command_side_effects` returns `true` and the caller returns early —
+    /// there is no double-application.
+    pub(crate) fn apply_to_params(&self, params: &mut TurtleParams) {
+        match self {
+            TurtleCommand::Move(dist) => {
+                let dx = dist * params.heading.cos();
+                let dy = dist * params.heading.sin();
+                params.position = vec2(params.position.x + dx, params.position.y + dy);
+            }
+            TurtleCommand::Turn(angle) => {
+                params.heading = normalize_angle(params.heading + angle.to_radians());
+            }
+            TurtleCommand::Circle {
+                radius,
+                angle,
+                direction,
+                ..
+            } => {
+                let geom =
+                    CircleGeometry::new(params.position, params.heading, *radius, *direction);
+                params.position = geom.position_at_angle(angle.to_radians());
+                params.heading = normalize_angle(match direction {
+                    CircleDirection::Left => params.heading - angle.to_radians(),
+                    CircleDirection::Right => params.heading + angle.to_radians(),
+                });
+            }
+            TurtleCommand::Goto(coord) => {
+                // Y-flip: turtle graphics Y+ = up; Macroquad Y+ = down
+                params.position = vec2(coord.x, -coord.y);
+            }
+            TurtleCommand::SetHeading(heading) => {
+                params.heading = normalize_angle(*heading);
+            }
+            TurtleCommand::SetColor(color) => {
+                params.color = *color;
+            }
+            TurtleCommand::SetFillColor(color) => {
+                params.fill_color = *color;
+            }
+            TurtleCommand::SetPenWidth(width) => {
+                params.pen_width = *width;
+            }
+            TurtleCommand::SetSpeed(speed) => {
+                params.speed = *speed;
+            }
+            TurtleCommand::SetShape(shape) => {
+                params.shape = shape.clone();
+            }
+            TurtleCommand::PenUp => {
+                params.pen_down = false;
+            }
+            TurtleCommand::PenDown => {
+                params.pen_down = true;
+            }
+            TurtleCommand::ShowTurtle => {
+                params.visible = true;
+            }
+            TurtleCommand::HideTurtle => {
+                params.visible = false;
+            }
+            TurtleCommand::Reset => {
+                *params = TurtleParams::default();
+            }
+            // Fill/text commands do not change TurtleParams for tweening purposes;
+            // their effects are handled entirely by execute_command_side_effects.
+            TurtleCommand::BeginFill | TurtleCommand::EndFill | TurtleCommand::WriteText { .. } => {
+            }
+        }
+    }
+
+    /// Duration in seconds for this command's animation at the given speed.
+    ///
+    /// Returns `0.01` (minimum) for commands that have no animated component.
+    /// This is the **single source of truth**; replaces
+    /// `TweenController::calculate_duration_with_state` in `tweening.rs`.
+    pub(crate) fn animation_duration(&self, params: &TurtleParams, speed: AnimationSpeed) -> f64 {
+        let AnimationSpeed::Animated(mut spd) = speed else {
+            // Instant mode — duration is irrelevant; return the minimum so tweener
+            // infrastructure still has a valid duration if called accidentally.
+            return f64::from(0.01_f32);
+        };
+
+        // Exponential speed scaling for high values (matches original behaviour)
+        if spd > 100.0 {
+            spd *= spd / 100.0;
+        }
+
+        let base: f32 = match self {
+            TurtleCommand::Move(dist) => dist.abs() / spd,
+            TurtleCommand::Turn(angle) => angle.abs() / (spd * 1.8),
+            TurtleCommand::Circle { radius, angle, .. } => {
+                let arc_length = radius * angle.to_radians().abs();
+                arc_length / spd
+            }
+            TurtleCommand::Goto(target) => {
+                let dx = target.x - params.position.x;
+                let dy = target.y - params.position.y;
+                (dx * dx + dy * dy).sqrt() / spd
+            }
+            _ => 0.0,
+        };
+
+        f64::from(base.max(0.01))
+    }
+
+    /// Whether executing this command (when pen is down) produces a stroke or fill mesh.
+    ///
+    /// This is the **single source of truth**; replaces
+    /// `TweenController::command_creates_drawing` in `tweening.rs`.
+    #[must_use]
+    pub(crate) fn produces_drawing(&self) -> bool {
+        matches!(
+            self,
+            TurtleCommand::Move(_) | TurtleCommand::Circle { .. } | TurtleCommand::Goto(_)
+        )
+    }
+}

turtle-lib/src/execution.rs

@@ -1,6 +1,6 @@
 //! Command execution logic
 
-use crate::circle_geometry::{CircleDirection, CircleGeometry};
+use crate::circle_geometry::CircleGeometry;
 use crate::commands::TurtleCommand;
 use crate::state::{DrawCommand, Turtle, TurtleParams, TurtleWorld};
 use crate::tessellation;
@@ -197,54 +197,49 @@ pub(crate) fn record_fill_vertices_after_movement(
     }
 }
 
-/// Execute a single turtle command, updating state and adding draw commands
-#[tracing::instrument]
-#[allow(clippy::too_many_lines)]
-pub(crate) fn execute_command(command: &TurtleCommand, state: &mut Turtle) {
-    // Try to execute as side-effect-only command first
-    if execute_command_side_effects(command, state) {
-        return; // Command fully handled
+/// Tessellate a completed movement command into a [`DrawCommand`] mesh.
+///
+/// Returns `None` if the pen was up or the command does not produce a drawing.
+///
+/// `end_position` is the turtle's position after the command completed:
+/// - instant-mode: `state.params.position` after [`TurtleCommand::apply_to_params`]
+/// - animated-mode: `tween.target_params.position` when the tween finishes
+///
+/// This is the **single** tessellation site for all committed line/arc meshes.
+/// It replaces both the inline tessellation inside `execute_command` and the
+/// now-deleted `add_draw_for_completed_tween`.
+pub(crate) fn tessellate_command(
+    command: &TurtleCommand,
+    start: &TurtleParams,
+    end_position: Vec2,
+) -> Option<DrawCommand> {
+    if !start.pen_down || !command.produces_drawing() {
+        return None;
     }
 
-    // Store start state for fill vertex recording
-    let start_state = state.clone();
-
-    // Execute movement and appearance commands
     match command {
-        TurtleCommand::Move(distance) => {
-            let start = state.params.position;
-            let dx = distance * state.params.heading.cos();
-            let dy = distance * state.params.heading.sin();
-            state.params.position =
-                vec2(state.params.position.x + dx, state.params.position.y + dy);
+        TurtleCommand::Move(_) | TurtleCommand::Goto(_) => {
+            let mesh_data = tessellation::tessellate_stroke(
+                &[start.position, end_position],
+                start.color,
+                start.pen_width,
+                false,
+            )
+            .ok()?;
 
-            if state.params.pen_down {
-                // Draw line segment with round caps (caps handled by tessellate_stroke)
-                if let Ok(mesh_data) = tessellation::tessellate_stroke(
-                    &[start, state.params.position],
-                    state.params.color,
-                    state.params.pen_width,
-                    false, // not closed
-                ) {
-                    state.commands.push(DrawCommand::Mesh {
+            Some(DrawCommand::Mesh {
                 data: mesh_data,
                 source: crate::state::TurtleSource {
                     command: command.clone(),
-                            color: state.params.color,
-                            fill_color: state.params.fill_color.unwrap_or(BLACK),
-                            pen_width: state.params.pen_width,
-                            start_position: start,
-                            end_position: state.params.position,
-                            start_heading: state.params.heading,
+                    color: start.color,
+                    fill_color: start.fill_color.unwrap_or(BLACK),
+                    pen_width: start.pen_width,
+                    start_position: start.position,
+                    end_position,
+                    start_heading: start.heading,
                     contours: None,
                 },
-                    });
-                }
-            }
-        }
-
-        TurtleCommand::Turn(degrees) => {
-            state.params.heading += degrees.to_radians();
+            })
         }
 
         TurtleCommand::Circle {
@@ -253,96 +248,61 @@ pub(crate) fn execute_command(command: &TurtleCommand, state: &mut Turtle) {
             steps,
             direction,
         } => {
-            let start_heading = state.params.heading;
-            let geom =
-                CircleGeometry::new(state.params.position, start_heading, *radius, *direction);
-
-            if state.params.pen_down {
-                // Use Lyon to tessellate the arc
-                if let Ok(mesh_data) = tessellation::tessellate_arc(
+            use crate::circle_geometry::CircleGeometry;
+            let geom = CircleGeometry::new(start.position, start.heading, *radius, *direction);
+            let mesh_data = tessellation::tessellate_arc(
                 geom.center,
                 *radius,
                 geom.start_angle_from_center.to_degrees(),
                 *angle,
-                    state.params.color,
-                    state.params.pen_width,
+                start.color,
+                start.pen_width,
                 *steps,
                 *direction,
-                ) {
-                    state.commands.push(DrawCommand::Mesh {
+            )
+            .ok()?;
+
+            Some(DrawCommand::Mesh {
                 data: mesh_data,
                 source: crate::state::TurtleSource {
                     command: command.clone(),
-                            color: state.params.color,
-                            fill_color: state.params.fill_color.unwrap_or(BLACK),
-                            pen_width: state.params.pen_width,
-                            start_position: state.params.position,
-                            end_position: geom.position_at_angle(angle.to_radians()),
-                            start_heading,
+                    color: start.color,
+                    fill_color: start.fill_color.unwrap_or(BLACK),
+                    pen_width: start.pen_width,
+                    start_position: start.position,
+                    end_position,
+                    start_heading: start.heading,
                     contours: None,
                 },
-                    });
-                }
+            })
         }
 
-            // Update turtle position and heading
-            state.params.position = geom.position_at_angle(angle.to_radians());
-            state.params.heading = match direction {
-                CircleDirection::Left => start_heading - angle.to_radians(),
-                CircleDirection::Right => start_heading + angle.to_radians(),
-            };
+        // `produces_drawing()` guards entry — this arm is only reachable if
+        // `produces_drawing` and the match above diverge, which would be a bug.
+        _ => None,
+    }
+}
+
+/// Execute a single turtle command, updating state and adding draw commands
+#[tracing::instrument]
+pub(crate) fn execute_command(command: &TurtleCommand, state: &mut Turtle) {
+    // Phase 1: side effects (fills, pen contours, reset, text).
+    // Returns true if the command is fully handled — no params update or tessellation needed.
+    if execute_command_side_effects(command, state) {
+        return;
     }
 
-        TurtleCommand::Goto(coord) => {
-            let start = state.params.position;
-            // Flip Y coordinate: turtle graphics uses Y+ = up, but Macroquad uses Y+ = down
-            state.params.position = vec2(coord.x, -coord.y);
+    // Phase 2: update TurtleParams (position, heading, colour, speed, etc.)
+    let start_params = state.params.clone();
+    command.apply_to_params(&mut state.params);
 
-            if state.params.pen_down {
-                // Draw line segment with round caps
-                if let Ok(mesh_data) = tessellation::tessellate_stroke(
-                    &[start, state.params.position],
-                    state.params.color,
-                    state.params.pen_width,
-                    false, // not closed
-                ) {
-                    state.commands.push(DrawCommand::Mesh {
-                        data: mesh_data,
-                        source: crate::state::TurtleSource {
-                            command: command.clone(),
-                            color: state.params.color,
-                            fill_color: state.params.fill_color.unwrap_or(BLACK),
-                            pen_width: state.params.pen_width,
-                            start_position: start,
-                            end_position: state.params.position,
-                            start_heading: state.params.heading,
-                            contours: None,
-                        },
-                    });
-                }
-            }
-        }
+    // Phase 3: record fill vertices after movement (must follow params update)
+    record_fill_vertices_after_movement(command, &start_params, state);
 
-        // Appearance commands
-        TurtleCommand::SetColor(color) => state.params.color = *color,
-        TurtleCommand::SetFillColor(color) => state.params.fill_color = *color,
-        TurtleCommand::SetPenWidth(width) => state.params.pen_width = *width,
-        TurtleCommand::SetSpeed(speed) => state.set_speed(*speed),
-        TurtleCommand::SetShape(shape) => state.params.shape = shape.clone(),
-        TurtleCommand::SetHeading(heading) => state.params.heading = *heading,
-        TurtleCommand::ShowTurtle => state.params.visible = true,
-        TurtleCommand::HideTurtle => state.params.visible = false,
-
-        // Reset
-        TurtleCommand::Reset => {
-            state.reset();
+    // Phase 4: tessellate and persist the committed drawing
+    if let Some(draw_cmd) = tessellate_command(command, &start_params, state.params.position) {
+        state.commands.push(draw_cmd);
     }
-
-        _ => {} // Already handled by execute_command_side_effects
-    }
-
-    // Record fill vertices AFTER movement
-    record_fill_vertices_after_movement(command, &start_state.params, state);
 }
 
 /// Execute command on a specific turtle by ID
@@ -362,81 +322,6 @@ pub(crate) fn execute_command_with_id(
     }
 }
 
-/// Add drawing command for a completed tween
-pub(crate) fn add_draw_for_completed_tween(
-    command: &TurtleCommand,
-    start_state: &TurtleParams,
-    end_state: &mut TurtleParams,
-) -> Option<DrawCommand> {
-    match command {
-        TurtleCommand::Move(_) | TurtleCommand::Goto(_) => {
-            if start_state.pen_down {
-                if let Ok(mesh_data) = tessellation::tessellate_stroke(
-                    &[start_state.position, end_state.position],
-                    start_state.color,
-                    start_state.pen_width,
-                    false,
-                ) {
-                    return Some(DrawCommand::Mesh {
-                        data: mesh_data,
-                        source: crate::state::TurtleSource {
-                            command: command.clone(),
-                            color: start_state.color,
-                            fill_color: start_state.fill_color.unwrap_or(BLACK),
-                            pen_width: start_state.pen_width,
-                            start_position: start_state.position,
-                            end_position: end_state.position,
-                            start_heading: start_state.heading,
-                            contours: None,
-                        },
-                    });
-                }
-            }
-        }
-        TurtleCommand::Circle {
-            radius,
-            angle,
-            steps,
-            direction,
-        } => {
-            if start_state.pen_down {
-                let geom = CircleGeometry::new(
-                    start_state.position,
-                    start_state.heading,
-                    *radius,
-                    *direction,
-                );
-                if let Ok(mesh_data) = tessellation::tessellate_arc(
-                    geom.center,
-                    *radius,
-                    geom.start_angle_from_center.to_degrees(),
-                    *angle,
-                    start_state.color,
-                    start_state.pen_width,
-                    *steps,
-                    *direction,
-                ) {
-                    return Some(DrawCommand::Mesh {
-                        data: mesh_data,
-                        source: crate::state::TurtleSource {
-                            command: command.clone(),
-                            color: start_state.color,
-                            fill_color: start_state.fill_color.unwrap_or(BLACK),
-                            pen_width: start_state.pen_width,
-                            start_position: start_state.position,
-                            end_position: end_state.position,
-                            start_heading: start_state.heading,
-                            contours: None,
-                        },
-                    });
-                }
-            }
-        }
-        _ => (),
-    }
-    None
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

turtle-lib/src/lib.rs

@@ -48,6 +48,7 @@
 
 pub(crate) mod builders;
 pub(crate) mod circle_geometry;
+pub(crate) mod command_behavior;
 pub(crate) mod commands;
 pub(crate) mod commands_channel;
 pub(crate) mod drawing;
@@ -297,14 +298,12 @@ impl TurtleApp {
             let completed_commands = TweenController::update(turtle);
 
             // Process all completed commands and add to the turtle's commands
-            for (completed_cmd, tween_start, mut end_state) in completed_commands {
-                let draw_command = execution::add_draw_for_completed_tween(
-                    &completed_cmd,
-                    &tween_start,
-                    &mut end_state,
-                );
-                // Add the new draw commands to the turtle
-                turtle.commands.extend(draw_command);
+            for (completed_cmd, tween_start, end_state) in completed_commands {
+                if let Some(draw_cmd) =
+                    execution::tessellate_command(&completed_cmd, &tween_start, end_state.position)
+                {
+                    turtle.commands.push(draw_cmd);
+                }
             }
         }
     }

turtle-lib/src/tweening.rs

@@ -319,10 +319,7 @@ impl TweenController {
     }
 
     fn command_creates_drawing(command: &TurtleCommand) -> bool {
-        matches!(
-            command,
-            TurtleCommand::Move(_) | TurtleCommand::Circle { .. } | TurtleCommand::Goto(_)
-        )
+        command.produces_drawing()
     }
 
     fn calculate_duration_with_state(
@@ -330,110 +327,12 @@ impl TweenController {
         current: &Turtle,
         speed: AnimationSpeed,
     ) -> f64 {
-        let mut speed = speed.value();
-
-        // For high speeds, make animation even faster by scaling speed exponentially
-        if speed > 100.0 {
-            speed *= speed / 100.0;
-        }
-
-        let base_time = match command {
-            TurtleCommand::Move(dist) => dist.abs() / speed,
-            TurtleCommand::Turn(angle) => {
-                // Rotation speed: assume 180 degrees per second at speed 100
-                angle.abs() / (speed * 1.8)
-            }
-            TurtleCommand::Circle { radius, angle, .. } => {
-                let arc_length = radius * angle.to_radians().abs();
-                arc_length / speed
-            }
-            TurtleCommand::Goto(target) => {
-                // Calculate actual distance from current position to target
-                let dx = target.x - current.params.position.x;
-                let dy = target.y - current.params.position.y;
-                let distance = (dx * dx + dy * dy).sqrt();
-                distance / speed
-            }
-            _ => 0.0, // Instant commands
-        };
-        f64::from(base_time.max(0.01)) // Minimum duration
+        command.animation_duration(&current.params, speed)
     }
 
     fn calculate_target_state(current: &TurtleParams, command: &TurtleCommand) -> TurtleParams {
         let mut target = current.clone();
-
-        match command {
-            TurtleCommand::Move(dist) => {
-                let dx = dist * current.heading.cos();
-                let dy = dist * current.heading.sin();
-                target.position = vec2(current.position.x + dx, current.position.y + dy);
-            }
-            TurtleCommand::Turn(angle) => {
-                target.heading = normalize_angle(current.heading + angle.to_radians());
-            }
-            TurtleCommand::Circle {
-                radius,
-                angle,
-                direction,
-                ..
-            } => {
-                // Use helper function to calculate final position
-                target.position = calculate_circle_position(
-                    current.position,
-                    current.heading,
-                    *radius,
-                    angle.to_radians(),
-                    *direction,
-                );
-                target.heading = normalize_angle(match direction {
-                    CircleDirection::Left => current.heading - angle.to_radians(),
-                    CircleDirection::Right => current.heading + angle.to_radians(),
-                });
-            }
-            TurtleCommand::Goto(coord) => {
-                // Flip Y coordinate: turtle graphics uses Y+ = up, but Macroquad uses Y+ = down
-                target.position = vec2(coord.x, -coord.y);
-            }
-            TurtleCommand::SetHeading(heading) => {
-                target.heading = normalize_angle(*heading);
-            }
-            TurtleCommand::SetColor(color) => {
-                target.color = *color;
-            }
-            TurtleCommand::SetPenWidth(width) => {
-                target.pen_width = *width;
-            }
-            TurtleCommand::SetSpeed(speed) => {
-                target.speed = *speed;
-            }
-            TurtleCommand::SetShape(shape) => {
-                target.shape = shape.clone();
-            }
-            TurtleCommand::PenUp => {
-                target.pen_down = false;
-            }
-            TurtleCommand::PenDown => {
-                target.pen_down = true;
-            }
-            TurtleCommand::ShowTurtle => {
-                target.visible = true;
-            }
-            TurtleCommand::HideTurtle => {
-                target.visible = false;
-            }
-            TurtleCommand::SetFillColor(color) => {
-                target.fill_color = *color;
-            }
-            TurtleCommand::BeginFill | TurtleCommand::EndFill | TurtleCommand::WriteText { .. } => {
-                // Fill and text commands don't change turtle state for tweening purposes
-                // They're handled directly in execution
-            }
-            TurtleCommand::Reset => {
-                // Reset returns to default state
-                target = TurtleParams::default();
-            }
-        }
-
+        command.apply_to_params(&mut target);
         target
     }
 }
@@ -451,7 +350,7 @@ fn calculate_circle_position(
 }
 
 /// Normalize angle to range [-PI, PI] to prevent floating-point drift
-fn normalize_angle(angle: f32) -> f32 {
+pub(crate) fn normalize_angle(angle: f32) -> f32 {
     let two_pi = std::f32::consts::PI * 2.0;
     let mut normalized = angle % two_pi;