Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 6, 2026

  • Implement pathfinding structs and algorithms
  • Add debug visualization in editor
  • Address initial PR feedback points 1-4
  • Address PR feedback points 5-8
  • Apply review changes from previous commit
  • Address latest PR feedback:
    • Deduplicate code in BFS by using process_neighbor lambda
    • Rename bfs_find_pathbfs_find_next_edge
    • Simplify bfs_find_next_edge to accept start and target directly (removed on_node_visit callback)
    • Change cosmos_navmesh_island::cell_size to uint32_t
    • Change vec2i cell_pos arguments to vec2u for unsigned indices
    • Change vec2i cell_xy in pathfinding to vec2u
    • Simplify bounds checking to just check against size.x and size.y (no more < 0 checks needed)
Original prompt

Pathfinding

Now that we have implemented navmeshes in cosmos_navmesh.h (see PR 315), i want to implement a pathfinding algorithm using A*.

Portals/islands preprocessing

Pathfinding isn't simply go A to B. We might want to cut shortcuts using portals, or we might want to pathfind onto other islands.

Navmesh already has portals/islands information.

First determine in O(n) which islands do the source and target belong to (a point in aabb check since every island has ltrbi bound;). Might be either the same or different.

Cross-island (unknown number of islands on the path)

  • First do BFS strictly on portals information. Most of the time islands will form a full graph but support paths across more than 2 islands just in case. So navmesh_portal is a node, and its out_island_index is an edge to another island. Find shortest path from source island to target island but only in terms of the number of islands.

Then reduce the problem to the cross-island with direct connection.

Cross-island (direct connection between two islands)

  • For all portals on the source island that lead to the target island, find such that minimizes euclidean distance (in straight line) to that portal + euclid distance from exit portal to the destination. Then launch pathfinding to that portal on that source island. This is O(n).
    • I know this is not always optimal, but is a sufficient heuristic.

Same-island

  • This should already do the classic A* search.

  • However, some portals can teleport across the same island. So, for all same-island-portals check if euclidean distance to any + dist from its exit to destination is shorter than just euclidean distance to destination. This is O(n) too.

  • If there is any such portal, choose the target to instead be this portal's center tile and end the pathfinding session there. New one will be initiated later or elsewhere if necessary.

    • I know this is not always optimal, but is a sufficient heuristic.
  • In all cases, treat only 0 as walkable except tiles of a given portal if we're navigating specifically to that portal.
    0 - free
    1 - occupied

    2 - portals, identified (2 + portal_index)
    considered occupied, EXCEPT when targeting this exact portal! We target the center tile, but it's okay to walk on the others of that portal too - since this too will get us teleported just as well.

We avoid tiles of other portals when we're navigating to not get accidentally teleported.

Struct overview

Eventually, all searches reduce to a simple A* within the same island. This function should return a path.

struct cell_on_island {
	int island_index;
	pathfinding_node node;
};

struct pathfinding_path {
	int island_index;
	std::vector<pathfinding_node> nodes;
	std::optional<cell_on_island> final_portal_node;
};

struct pathfinding_node {
	vec2i cell_xy;
};

(all introspectible)
cell_xy being the x,y indices on that island

final_portal_node being the node we're meant to teleport to at the end - for informational purposes.

Suggested naming

find_islands_connection - island bfs, return just the next node to go to
find_best_portal_from_to(source_island, target_island) - should be reusable for both cross and same island search (no reason not to be reusable)

find_path_across_islands_many
find_path_across_islands_direct - eventually called by find_path_across_islands_many. uses find_best_portal_from_to
find_path_within_island - eventually called by find_path_within_island. uses find_best_portal_from_to(island_id, island_id)

return an optional to signal if path cant be found because it lies outside an island or just found its unreachable within same island search

Helper function: find_path_across_islands_many_full

This should repeatedly call the main pathfinding function (which will always eventually returns just a path within the same island), until eventually final_portal_node is empty. final_portal_node is always the source of the next search, destination being the same. This is just a helper function for visualization in the editor, i know it's not super optimal to rerun the function which does the full bfs every time etc.

Debug visualization in editor

Where the editor visualizes a navmesh (it already does) add another color for visualizing the path nodes.
Overlay the path nodes of a given color (default to green (0,255,0,220)) on top of the navmesh ones, so you can do it in separate step after showing navmesh.

Have another point_marker_type: DEBUG_PATHFINDING_START and DEBUG_PATHFINDING_END. Assign them this icon:
result.id = I::EDITOR_ICON_SPAWN;
result.col = green;
Iterate over these point markers here and for every DEBUG_PATHFINDING_START launch find_path_across_islands_many_full for every DEBUG_PATHFINDING_END (it's quadratic i know). Then just draw the squares that constitute the path. Draw EDITOR_TOOL_PLAIN_ARROW in full white on top of them designating the direction at the center of each tile (scaled with zoom so just like the colore...


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 6, 2026 17:05
Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Copilot AI changed the title [WIP] Add A* pathfinding algorithm with cross-island support Implement A* pathfinding with cross-island portal support Jan 6, 2026
Copilot AI requested a review from geneotech January 6, 2026 17:11
@geneotech
Copy link
Member

@copilot

  1. Brother, when generating a navmesh, save its position within a portal struct.
vec2i in_cell_pos = vec2i::zero;

this makes find_portal_center_cell completely unnecessary.

  1. I forgot to mention the only allowed directions are left/right/up/down.

  2. Have this pathfinder in a separate cpp file. have a header too with just declarations. Add to CMakeLists.txt

  3. instead of returning -1 where there is a failure, return std::optionalstd::size_t. Conveys intent better. Add this rule to AGENTS.md.

  4. have cosmos_pathfinding.h in common_state, a separate one for the pathfinding structs.

  5. world_distance use vec2 math. Add a rule for this in AGENTS.md:
    "Whenever performing 2D vector math, check if there's a function like this in augs/math/vec2.h", and give this example. Here we can just use (a - b).length()

  6. same with
    if (pos_i.x >= bound.l && pos_i.x < bound.r &&
    pos_i.y >= bound.t && pos_i.y < bound.b) {
    return static_cast(i);
    }
    use ltrb bound math - hover function. add a rule to AGENTS.md.

  7. it would be great if we had new headers

augs/algorithm/a_star.hpp
augs/algorithm/bfs.hpp

these would be heavily-templatized pathfinding functions that only contain the pathfinding algorithm logic, and heavily accept lambdas for setting "visited" state of a node, getting visited state, getting neighbors of a node through for_each_neighbor, and for a* the distance heuristic, and for whatever else is needed. You can assume all weights/costs are uniform so there's no need to getting/passing these anywhere.
So e.g. for_each_neighbor would be a lambda accepting a lambda called with identificators which then can be passed as is to get/set visited state or also the distance heuristic. The id would probably here be the cell index x,y, and it should be transparent to these algorithms, they just pass ids around whatever that id is. Also remember to operate on centers of cells for the distance heuristic but it's probably like this already.

… std::optional

Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 6, 2026

@copilot

  1. Brother, when generating a navmesh, save its position within a portal struct.
    ...

Addressed all 4 points in commit 6ec0fb5:

  1. Added in_cell_pos to navmesh_portal struct and updated navmesh_generation.hpp to populate it during generation. Removed find_portal_center_cell function.
  2. Changed from 8-directional to 4-directional movement (up/down/left/right only).
  3. Split pathfinding into pathfinding.h (declarations) and pathfinding.cpp (implementations), added to CMakeLists.txt.
  4. Changed all functions returning -1 for failure to use std::optional instead.

@geneotech
Copy link
Member

@copilot

You ignored the other points 5-9:

  1. have cosmos_pathfinding.h in common_state, a separate one for the pathfinding structs.

  2. world_distance use vec2 math. Add a rule for this in AGENTS.md:
    "Whenever performing 2D vector math, check if there's a function like this in augs/math/vec2.h", and give this example. Here we can just use (a - b).length()

  3. same with
    if (pos_i.x >= bound.l && pos_i.x < bound.r &&
    pos_i.y >= bound.t && pos_i.y < bound.b) {
    return static_cast(i);
    }
    use ltrb bound math - hover function. add a rule to AGENTS.md.

  4. it would be great if we had new headers

augs/algorithm/a_star.hpp
augs/algorithm/bfs.hpp

these would be heavily-templatized pathfinding functions that only contain the pathfinding algorithm logic, and heavily accept lambdas for setting "visited" state of a node, getting visited state, getting neighbors of a node through for_each_neighbor, and for a* the distance heuristic, and for whatever else is needed. You can assume all weights/costs are uniform so there's no need to getting/passing these anywhere.
So e.g. for_each_neighbor would be a lambda accepting a lambda called with identificators which then can be passed as is to get/set visited state or also the distance heuristic. The id would probably here be the cell index x,y, and it should be transparent to these algorithms, they just pass ids around whatever that id is. Also remember to operate on centers of cells for the distance heuristic but it's probably like this already.

Copilot AI and others added 2 commits January 6, 2026 18:02
…hms, use vec2 math

Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 6, 2026

@copilot

You ignored the other points 5-9:
...

Addressed all points 5-8 in commits 6125cbe and cbe7ccc:

  1. Created game/common_state/cosmos_pathfinding.h for the pathfinding structs.
  2. Changed world_distance to use (a - b).length() and added rule to AGENTS.md.
  3. Changed point-in-rect checks to use bound.hover(pos) and added rule to AGENTS.md.
  4. Created templated augs/algorithm/a_star.hpp and augs/algorithm/bfs.hpp that accept lambdas for:
    • get_visited / set_visited for node visited state
    • for_each_neighbor for iterating neighbors
    • heuristic (A* only) for distance estimation
    • on_node_visit (BFS) for early termination
    • All operations use transparent node IDs (vec2i for cells, int for islands)

Copy link
Member

@geneotech geneotech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Apply changes according to my review.

*/
for (std::size_t i = 0; i < path.nodes.size(); ++i) {
const auto& node = path.nodes[i];
const auto world_x = island.bound.l + node.cell_xy.x * cell_size;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use island.bound.lt() to get a vector (add such a function to rects if it doesnt exist)
should be same as

 basic_vec2<T> get_position() const {
	return basic_vec2<T>(l, t);
}

add equivalents for other corners like rb() etc, and specify to favor these in agents.md again instead of using x, exprs y separately. Also specify there you can use vec2::square(cell_size) instead of vec2(cell_size, cell_size).

const auto world_y = island.bound.t + node.cell_xy.y * cell_size;

const auto screen_lt = on_screen(vec2(static_cast<float>(world_x), static_cast<float>(world_y)));
const auto screen_rb = on_screen(vec2(static_cast<float>(world_x + cell_size), static_cast<float>(world_y + cell_size)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be then reduced to a single vector expression again and above too, also say in agents.md you can cast between vec2 and vec2i freely

if (i + 1 < path.nodes.size()) {
const auto& next_node = path.nodes[i + 1];
const auto dir = vec2(
static_cast<float>(next_node.cell_xy.x - node.cell_xy.x),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly fix using one vector expression


const auto center_world = vec2(
static_cast<float>(world_x + cell_size / 2),
static_cast<float>(world_y + cell_size / 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly fix using one vector expression

/*
Compute rotation angle from direction.
*/
const auto angle = std::atan2(dir.y, dir.x) * RAD_TO_DEG<float>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use vec2::degrees. to fix this for good, search through vec2.h and rects.h and add the most obvious functions to the AGENTS.md in a list with a oneline descrptions like

  • vec2 ltrb::lt() - left-top
  • vec2 ltrb::rb() - right bottom
  • float vec2::degrees() - get degrees of a vector
    you get the idea

list like 15-25 the most obvious functions that would often be used.

/*
Reverse to get start -> target order.
*/
for (auto it = path_cells.rbegin(); it != path_cells.rend(); ++it) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use for (auto it : reverse(path_cells)) (it's a wrapper in reversion_wrapper.h) add a rule for this in AGENTS.md


/*
Parent tracking for path reconstruction.
Store parent as (y * size.x + x + 1), where +1 avoids confusion with 0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead use -1 as "unset" value, and 0 should be the first cell already. this simplifies the calculations later.

};

auto set_parent = [&](const vec2i child, const vec2i parent_cell) {
parent[cell_index(child)] = parent_cell.y * size.x + parent_cell.x + 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not reuse cell_index here? Also actually implement cell_index inside cosmos_navmesh_island and use that in the lambda cell_index.

Store parent as (y * size.x + x + 1), where +1 avoids confusion with 0.
*/
std::vector<int> parent(grid_size, 0);
std::vector<float> g_costs(grid_size, std::numeric_limits<float>::max());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these to be reused in the pathfinding context

SetGCost&& set_g_cost
) {
using node_type = astar_queue_node<Id>;
std::priority_queue<node_type, std::vector<node_type>, std::greater<node_type>> open_set;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

store this and bfs's queue in the pathfinding context, pass the single queues as the references to the algorithms. clear upon calling.
make a using astar_queue_type and using bfs_queue_type in augs namespace so i can declare them easily in the struct e.g.

augs::astar_queue_type astar_queue;

…llback_result

Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 6, 2026

@copilot Apply changes according to my review.

Applied all review changes in commit ce31543:

Rects/Vec2 changes:

  • Added lt(), rb(), rt(), lb() shortcuts to ltrb
  • Use vector expressions: vec2(island.bound.lt()) + vec2(cell_xy) * cell_size
  • Use vec2::degrees() instead of atan2 * RAD_TO_DEG
  • Use vec2::square(side) for square vectors

Type changes:

  • Changed all index types from int to std::size_t with std::optional
  • Use size.area() instead of size.x * size.y

Algorithm changes:

  • Added augs:: namespace to BFS/A* functions
  • Use callback_result::CONTINUE/ABORT in callbacks
  • Store queues (astar_queue, bfs_queue) in pathfinding_context
  • Store cells_parent and cells_g_costs in pathfinding_context
  • Use -1 as unset parent value (0 is valid first cell)

Other:

  • find_path_within_island now takes island reference instead of index
  • Added cell_index() to cosmos_navmesh_island
  • Use reverse() wrapper for reverse iteration
  • Updated AGENTS.md with common vec2/ltrb functions list

Copilot AI requested a review from geneotech January 6, 2026 22:34
@geneotech
Copy link
Member

@copilot

  • please deduplicate the code in bfs_find_path for the first node and the rest of the nodes, it's awfully duplicated.
  • rename bfs_find_path -> bfs_find_next_edge
  • at this point dont accept on_node_visit, just pass
    source_island_index,
    target_island_index
    no need to complicate this further.
  • i would like cosmos_navmesh_island::cell_size to be uint32_t. As well as all:
  • vec2i cell_pos arguments in navmesh logic to be vec2u since they're about indices. Chan
  • vec2i cell_xy in pathfinding as well to vec2u
    once you do this you should look for and reduce logic like
    if (cell_pos.x < 0 || cell_pos.y < 0 || cell_pos.x >= size.x ||
    cell_pos.y >= size.y) {
    return;
    }
    to just checking against size.
    you will have to use uint32_t instead of ints in some loops to avoid different sign warnings.

…code

Co-authored-by: geneotech <3588717+geneotech@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 6, 2026

@copilot

  • please deduplicate the code in bfs_find_path for the first node and the rest of the nodes, it's awfully duplicated.
    ...

Applied all changes in commit ade1e17:

BFS changes:

  • Renamed bfs_find_pathbfs_find_next_edge
  • Simplified to accept start and target directly instead of on_node_visit callback
  • Deduplicated code using process_neighbor lambda (first node and rest now share same logic)

Type changes:

  • cosmos_navmesh_island::cell_size: intuint32_t
  • navmesh_portal::in_cell_pos/out_cell_pos: vec2ivec2u
  • pathfinding_node::cell_xy: vec2ivec2u
  • editor_arena_settings::navmesh_cell_size: intuint32_t
  • All cell position arguments changed to vec2u

Simplified bounds checking:

  • Removed < 0 checks (not needed for unsigned types)
  • Now just checks cell_pos.x >= size.x || cell_pos.y >= size.y

@geneotech geneotech marked this pull request as ready for review January 7, 2026 01:05
@geneotech geneotech merged commit aceb380 into master Jan 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants