From 9eecd3e49d0864c53f625660b6a1d9f57c81e598 Mon Sep 17 00:00:00 2001 From: popertots Date: Mon, 6 Apr 2026 00:22:17 +0100 Subject: [PATCH] pathfinding --- .sisyphus/plans/job_pathfinding_system.md | 493 ++++++++++++++++++++++ 1 file changed, 493 insertions(+) create mode 100644 .sisyphus/plans/job_pathfinding_system.md diff --git a/.sisyphus/plans/job_pathfinding_system.md b/.sisyphus/plans/job_pathfinding_system.md new file mode 100644 index 0000000..63a6508 --- /dev/null +++ b/.sisyphus/plans/job_pathfinding_system.md @@ -0,0 +1,493 @@ +# Job Pathfinding System Implementation + +## Overview + +You need to implement a new asynchronous pathfinding system for the job assignment in the dorf game. This replaces the broken use of provisional pathfinding in the job queue. + +## Current System (BROKEN) + +### Files Involved +- `src/entities/tasks/job_queue.rs` - Job queue management +- `src/entities/tasks/job_assignment.rs` - Assigns jobs to dorfs +- `src/entities/shared_systems/pathfinding.rs` - Existing pathfinding infrastructure +- `src/entities/tasks/components.rs` - Task definitions + +### What Currently Happens (WRONG) + +1. **job_assignment.rs** calls `job_queue.claim_job_at()` or `pop_best_pathfinding()` +2. These functions call `calculate_provisional_path()` from pathfinding.rs +3. This uses a hardcoded node limit (1024, now 4096) to check if a path "exists" +4. If pathfinding hits the limit, it returns "no path found" and the job goes UNCLAIMED +5. This blocks ALL job assignments for far-away dorfs + +### Why This Is Wrong + +`calculate_provisional_path()` is NOT designed for reachability checking! It is designed to give entities something to walk toward IMMEDIATELY while async full pathfinding runs in the background. + +From pathfinding.rs comments: +- Provisional paths are for tier 2 (2-4 chunks away) and tier 3 (>4 chunks away) +- When node_limit is hit, it returns a PARTIAL path to the closest node found +- It is NOT meant to determine if a path exists +- It is NOT meant to compare multiple candidate paths + +The current code misunderstands: returning empty from provisional DOES NOT mean "unreachable" - it just means the quick search didn't finish. The actual pathfinding system handles all this automatically when the dorf moves. + +### The Failure Mode + +- Dorf at position (500, -500) tries to get job at position (0, 0) +- Provisional pathfinding searches 4096 nodes, doesn't reach goal +- Returns "no path found" +- Job stays Unclaimed forever +- Dorf never gets work +- Game stalls + +## What You Need To Build + +### 1. New Traversal Distance Function + +In `pathfinding.rs`, create: + +```rust +/// Calculate traversal distance between two points using full A*. +/// Returns Ok(distance_in_tiles) or Err(()) if unreachable. +/// This is different from provisional pathfinding - it actually searches until it finds the goal. +/// Uses PATHFINDER_MAX_NODES (15,000) as the limit. +pub fn calculate_traversal_distance( + tilemap: &TileMap, + start: IVec3, + goal: IVec3, +) -> Result +``` + +Key differences from provisional: +- Uses full A* (calculate_path_with_scratchpad), not provisional +- Returns actual distance (g_score), not path +- Uses 15,000 node limit (PATHFINDER_MAX_NODES) +- Should NOT fail on long paths unless truly unreachable + +**Optimization (Early Exit):** When batch calculating, track the shortest path found so far. If current g_score exceeds this, terminate early: + +```rust +let mut best_distance = i32::MAX; +// When checking dorf N: +// If g_score > best_distance, return Err(()) - can't beat current best +``` + +### 2. Batch Calculation Function + +```rust +/// Calculate traversal distances for multiple dorf-to-target pairs. +/// Takes: Vec of (job_id, dorf_entity, dorf_pos, target_pos) +/// Returns: Vec of (job_id, dorf_entity, distance) for reachable pairs +pub fn batch_calculate_traversals( + tilemap: &TileMap, + requests: Vec<(JobId, Entity, IVec3, IVec3)>, +) -> Vec<(JobId, Entity, i32)> +``` + +### Note on Scratchpad + +The `pathfinding.rs` uses a thread-local `SCRATCHPAD` (AStarScratchpad). Since our batch runs synchronously on the main thread (within the FixedUpdate system), we can safely use the existing scratchpad. If you move to async threads later, you'll need to create per-thread scratchpads. + +### Additional: Update has_fell_tree Helper + +After updating JobState, also update the helper method in job_queue.rs: + +```rust +/// Check if any FellTree job exists (in any active state) +pub fn has_fell_tree(&self) -> bool { + self.jobs.iter().any(|entry| { + if !entry.kind.is_fell_tree() { + return false; + } + matches!( + entry.state, + JobState::Unclaimed | JobState::Calculating(_) | JobState::Claimed(_) + ) + }) +} +``` + +This is used by demo.rs to check if a new tree job should be spawned. + +### 3. Update JobState Enum (job_queue.rs) + +```rust +#[derive(Debug, Clone, PartialEq)] +pub enum JobState { + /// No dorf has taken this job. + Unclaimed, + /// Paths are being calculated - dorfs in this vec are locked + Calculating(Vec), // Entity IDs of dorfs being considered + /// A dorf has claimed this job and is executing it. + Claimed(Entity), + /// Pathfinding failed too many times - requires manual review or world change + Suspended, + /// All tasks for this job are done. Pending removal. + Complete, +} +``` + +**IMPORTANT:** Add a helper method to JobQueue to check if a dorf is locked: + +```rust +impl JobQueue { + /// Check if a dorf is currently in 'Calculating' state for any job + pub fn is_dorf_locked(&self, dorf: Entity) -> bool { + self.jobs.iter().any(|entry| { + if let JobState::Calculating(dorfs) = &entry.state { + dorfs.contains(&dorf) + } else { + false + } + }) + } + + /// Get all dorfs that are locked (in Calculating or Claimed state) + pub fn get_locked_dorfs(&self) -> Vec { + let mut locked = Vec::new(); + for entry in &self.jobs { + match &entry.state { + JobState::Calculating(dorfs) => locked.extend(dorfs.iter().cloned()), + JobState::Claimed(dorf) => locked.push(*dorf), + _ => {} + } + } + locked + } +} +``` + +When finding candidate dorfs for a new job, filter out any dorfs that appear in `get_locked_dorfs()`. + +### 4. Add approach_target and retry_count to Job Entry + +```rust +struct Entry { + kind: JobKind, + state: JobState, + id: JobId, + approach_target: Option, // Pre-computed target for assigned dorf + retry_count: u32, // How many times pathfinding has failed +} +``` + +The retry_count is used to determine the expanding scope: +- 0: Try 5 dorfs +- 1-3: Try 20 dorfs +- 4+: Try all dorfs (200 max) + +### 5. Add Config (config.toml) + +```toml +[job_assignment] +max_dorfs_per_job = 5 +``` + +And in `config.rs`: +```rust +#[derive(Debug, Deserialize, Clone, Resource)] +pub struct GameConfig { + // ... existing fields ... + #[serde(default)] + pub job_assignment: JobAssignmentSettings, +} + +#[derive(Debug, Deserialize, Clone, Default)] +pub struct JobAssignmentSettings { + #[serde(default = "default_max_dorfs")] + pub max_dorfs_per_job: u32, +} + +const fn default_max_dorfs() -> u32 { 5 } +``` + +### 6. Expanding Dorf Scope (CRITICAL!) + +The initial batch of 5 dorfs might not all be able to reach the job (island problem). Implement EXPANDING SCOPE: + +```rust +const DEFAULT_DORFS: u32 = 5; +const EXPANDED_DORFS: u32 = 20; +const MAX_DORFS: u32 = 200; // All dorfs + +enum DorfScope { + Default, // Start with 5 + Expanded, // Expand to 20 if Default fails + Max, // Expand to all if Expanded fails +} + +impl JobQueue { + pub fn get_scope_for_job(&self, job_idx: usize) -> u32 { + // Track how many times each job has been retried + // Or store retry count in Entry struct + match self.jobs[job_idx].retry_count { + 0 => DEFAULT_DORFS, + 1..=3 => EXPANDED_DORFS, + _ => MAX_DORFS, + } + } +} +``` + +**Algorithm:** +``` +1. Try with DEFAULT_DORFS (5) closest dorfs +2. If ALL fail to find path: + - Increment retry count + - Expand scope to EXPANDED_DORFS (20) +3. If still all fail: + - Expand scope to MAX_DORFS (all available) +4. If ALL dorfs fail → job stays Unclaimed, will retry next frame +``` + +This handles: +- Islands: eventually checks ALL dorfs +- Performance: most jobs resolve with 5 dorfs +- Fairness: no single job hogs all pathfinding + +### 7. New System: job_pathfinding_system + +Create new file `src/entities/tasks/job_pathfinding.rs`: + +```rust +/// System that runs every FixedUpdate +/// For each Unclaimed job not currently Calculating: +/// - Determine scope (5, 20, or all) +/// - Find N closest dorfs (simple Manhattan distance) +/// - Set job to Calculating with dorf list +/// - Calculate traversal distances +/// - Assign to dorf with shortest distance +/// - Store approach_target in job Entry +/// If ALL dorfs in scope fail, expand scope and retry next tick +pub fn job_pathfinding_system( + mut job_queue: ResMut, + config: Res, + tilemap: Res, + dorf_query: Query<(Entity, &Transform), With>, +) { + // Implementation details below... +} +``` + +### 8. Throttling (CRITICAL!) + +To prevent CPU starvation when player designates many jobs: + +```rust +const MAX_JOBS_PER_TICK: usize = 5; + +pub fn job_pathfinding_system(...) { + // Get list of Unclaimed jobs + let mut unclaimed: Vec = job_queue.iter_unclaimed() + .map(|(idx, _)| idx) + .collect(); + + // Sort by priority (existing logic) + unclaimed.sort_by(|&a, &b| { + job_queue.jobs[a].kind.priority() + .cmp(&job_queue.jobs[b].kind.priority()) + .reverse() + }); + + // Only process up to MAX_JOBS_PER_TICK per frame + for job_idx in unclaimed.into_iter().take(MAX_JOBS_PER_TICK) { + // ... processing logic ... + } +} +``` + +This prevents 1000+ pathfinding calls when player marks forest of 50 trees. + +### 7. Logic for Each Job Type + +**FellTree:** +``` +1. Get trunk_pos from job +2. Find ALL standable tiles adjacent to trunk (call find_all_standable_adjacent) +3. Find N closest dorfs to trunk (simple distance) +4. For each dorf: + - For each approach_tile: + - calculate_traversal_distance(dorf_pos, approach_tile) + - Track shortest path per dorf +5. Pick dorf + approach_tile with overall shortest path +6. Store approach_target in Entry +``` + +**HaulCargo:** +``` +1. Get cargo_pos from job +2. Find N closest dorfs to cargo_pos (simple distance) +3. For each dorf: + - calculate_traversal_distance(dorf_pos, cargo_pos) +4. Pick dorf with shortest path +5. Store cargo_pos as approach_target +``` + +### 8. Event for Job Assignment + +When job is assigned, fire an event or directly push task to dorf: + +```rust +/// Called when a job is successfully assigned +fn assign_job_to_dorf( + job_queue: &mut JobQueue, + job_idx: usize, + dorf_entity: Entity, + approach_target: IVec3, + // ... query to get dorf components ... +) { + // Set job state to Claimed + job_queue.jobs[job_idx].state = JobState::Claimed(dorf_entity); + job_queue.jobs[job_idx].approach_target = Some(approach_target); + + // Get the job kind to create appropriate task + match &job_queue.jobs[job_idx].kind { + JobKind::FellTree { trunk_pos } => { + // Create ChopTree with approach_target, not trunk_pos! + let task = Task::ChopTree { + job_id, + trunk_pos: *trunk_pos, // original trunk for chopping + chop_ticks: 120, + step: ChopStep::MovingToTree { + approach: Some(approach_target), // pre-computed! + }, + }; + // Push to dorf's queue, set state to Pending + } + JobKind::HaulCargo { cargo_entity, cargo_pos, dest } => { + // Create HaulCargo with pre-computed approach + let task = Task::HaulCargo { + job_id, + cargo_entity: *cargo_entity, + cargo_pos: *cargo_pos, + dest: *dest, + step: HaulStep::MovingToCargo { + approach: Some(approach_target), + }, + }; + // Push to dorf's queue, set state to Pending + } + } +} +``` + +### 9. Simplify job_assignment.rs + +After implementing the above, job_assignment.rs should: +- NO LONGER call calculate_provisional_path +- NO LONGER call pop_best_pathfinding or claim_job_at +- Simply handle fallback: if dorf has no task and no job pending, give them Idle task +- Trust the new job_pathfinding system for actual job assignment + +## CRITICAL: This Is NOT A Flat World + +READ `src/world/generation/terrain.rs` before implementing! + +The world has: +- Perlin noise-based heightmaps +- Multiple Z-levels (not just z=0) +- Caves and voids +- Terrain blobs that create organic shapes + +A tree at position (32, 0, 16) might have its trunk at z=16 while the nearest standable tile could be at z=0, z=16, z=32, or anywhere depending on terrain. + +The `find_all_standable_adjacent` function in job_queue.rs already handles this - it searches for standable tiles. Your pathfinding must use IVec3 (x, y, Z) correctly and not assume z=0. + +Common mistakes to avoid: +- Assuming tree trunk z == approach tile z +- Using 2D distance only +- Assuming flat floor at z=0 +- Not checking is_standable() properly + +## CRITICAL EDGE CASES (GOTCHAS) + +### 1. The "Island" Problem (Infinite Calculation Loops) + +**Scenario:** A tree is on an unreachable floating island or behind a solid wall. +**Bug:** System picks 5 closest dorfs -> calculates full paths -> all 5 fail -> job resets to Unclaimed -> next tick, picks 5 closest dorfs again. Infinite CPU drain. +**Solution:** The expanding scope handles this - eventually checks ALL dorfs. Additionally, after MAX retries, mark job as `Failed` or `Suspended` so it stops retrying. + +### 2. State Mutation During Calculation + +**Scenario:** Tick 100: Dorf A is Idle. Async pathfinding starts. Tick 105: Dorf A gets attacked/derped. Tick 110: Async task finishes, assigns job to Dorf A. +**Bug:** Dorf A gets job while doing something else, queue out of sync. +**Solution:** When async task returns the winning dorf, verify: +- TaskState is NOT Active (not already working) +- Has no cargo (if HaulCargo job) + +Note: Movement during calculation is negligible (~16 tiles/second) so we don't check position. + +```rust +// TODO(future): Check if dorf is still alive before assigning +// Currently no death system implemented, but when added: +// - Query dorf entity exists before assigning +// - If dorf died, assign to runner-up +``` + +### 3. Race Condition: Multiple Jobs Assigning Same Dorf + +**Scenario:** Job A and Job B both pick Dorf A as best candidate. Both assign. +**Bug:** First assignment gets overwritten, job lost. +**Solution:** The `Calculating` state locks dorfs: +- When job enters `Calculating(Vec)`, those dorfs are locked +- Other jobs in `Unclaimed` state skip these dorfs when finding candidates +- Only when job completes (Claimed -> Complete) are dorfs unlocked + +### 4. Cargo Position Changes During Haul + +**Scenario:** HaulCargo job created for log at position X. While dorf is walking, another event moves the log. +**Bug:** Dorf walks to old position, finds nothing. +**Solution:** When dorf arrives at cargo_pos, verify cargo_entity still exists and is at expected position. If not, fail task and re-queue as new job. + +## Summary of Changes + +| File | Change | +|------|--------| +| `pathfinding.rs` | Add `calculate_traversal_distance()` and `batch_calculate_traversals()` | +| `job_queue.rs` | Add `Calculating` state, `approach_target` field | +| `job_pathfinding.rs` | NEW - main pathfinding system | +| `job_assignment.rs` | Remove pathfinding calls, keep fallback idle assignment | +| `config.toml` | Add `[job_assignment]` section | +| `config.rs` | Add JobAssignmentSettings | +| `components.rs` | May need to ensure ChopStep::MovingToTree::approach is set correctly | + +## Expected Behavior After Fix + +1. Job appears in queue (Unclaimed) +2. job_pathfinding_system picks it up +3. Finds 5 closest dorfs, sets job to Calculating +4. Calculates actual traversal distances (not provisional!) +5. Assigns to best dorf, stores approach_target +6. Pushes task to dorf's queue +7. Executor runs - dorf walks to approach_target using NORMAL pathfinding +8. Dorf chops/hauls as expected + +The key difference: we use FULL pathfinding for job ASSIGNMENT, not provisional. The actual dorf movement still uses the tiered pathfinding system as before. + +## Demo System Update + +In `src/entities/tasks/demo.rs`, modify the tree spawning logic: + +```rust +pub fn demo_system( + mut job_queue: ResMut, + // ... existing params ... +) { + // Check if we have ANY FellTree jobs (Claimed, Unclaimed, or Calculating) + let has_any_fell_tree = job_queue.iter() + .any(|(_, _, state)| { + matches!(state, JobState::Claimed(_) | JobState::Unclaimed | JobState::Calculating(_)) + }); + + // Only spawn new job if NO FellTree jobs exist at all + // Suspended jobs are ignored - they stay suspended forever + if !has_any_fell_tree { + if let Some(trunk_pos) = find_tree_nearest_origin(&tilemap, &chunk_map, &tree_parts) { + job_queue.push(JobKind::FellTree { trunk_pos }); + } + } +} +``` + +This ensures there's always exactly ONE active tree-chopping job in the queue at a time. If previous jobs become Suspended (unreachable), they stay suspended and don't block new jobs.