Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add path query region filters #102766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

smix8
Copy link
Contributor

@smix8 smix8 commented Feb 12, 2025

Adds filter lists to exclude or include specific regions in path queries.

This is close to usability that users know from the physics queries exclude lists.

Currently the path queries run over all the enabled regions in the navigation map. The only current way to filter anything is with the navigation_layers bitmask which has its limits. This does not really work well for large maps or when users only want to consider specific regions for a path.

If a region RID is inside the exclude list all the region navmesh polygons (including navlinks that connect from or to them) will not be considered in the path query no matter what (even if in the include list).

As default if the include list is kept empty all regions are used. If a single region RID is added to the include list the path query will only consider region polygons from that list (that are also not in the exclude list).

Copy link
Contributor

@Scony Scony left a comment

Choose a reason for hiding this comment

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

LGTM, props for adding test!

modules/navigation/3d/nav_mesh_queries_3d.cpp Outdated Show resolved Hide resolved
modules/navigation/3d/nav_mesh_queries_3d.cpp Outdated Show resolved Hide resolved
modules/navigation/3d/nav_mesh_queries_3d.cpp Outdated Show resolved Hide resolved
@@ -71,6 +71,10 @@ class NavMeshQueries3D {
PathPostProcessing path_postprocessing = PathPostProcessing::PATH_POSTPROCESSING_CORRIDORFUNNEL;
bool simplify_path = false;
real_t simplify_epsilon = 0.0;
bool exclude_regions = false;
bool include_regions = false;
LocalVector<RID> excluded_regions;
Copy link
Contributor

@kiroxas kiroxas Feb 13, 2025

Choose a reason for hiding this comment

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

Is there really a way to have both of those active at the same time (include and exclude) ? Seems like we either exclude region or include region, but doing both at the same time seems ... odd ? Seems like the use case is either I want evrything except those (exclude list) or only search inside those (include list), but having both active at the same time seems error prone ? Maybe there is some case I'm not thinking of though. If this is the case, then all could be simplified, as we only need one local vector and all those previous if could become switch cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will just happen with users that both lists are used and some RID slips in both lists. That is how it always goes with users when both include and exclude options are available so better account for it.

servers/navigation/navigation_path_query_parameters_2d.cpp Outdated Show resolved Hide resolved
servers/navigation/navigation_path_query_parameters_3d.cpp Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
tests/servers/test_navigation_server_3d.h Outdated Show resolved Hide resolved
Adds filter lists to exclude or include specific regions in path queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants