[Navigation] Create a dedicated 2D navigation server by AThousandShips · Pull Request #101504 · godotengine/godot

AThousandShips · 2025-01-13T18:45:29Z

This is ready for extensive review but still a work in progress, keeping commits separate to handle transitioning changes and deciding on some more steps in this, but it should be feature complete

What it especially needs is:

Extensive testing, I've done some basic testing of pathing but haven't tested avoidance or internal details beyond the CI and basic testing, as I don't know the details of most of these systems
Some further simplification of the 2D code, I've made a number of simplifications based on the change from 3D to 2D (like removing all use of up, and removing some cases that are distinctly 3D) but I suspect there might be some areas that still can be simplified now that it is 2D, but would need some further insight and testing
Opinions on the decisions of restructuring of performance and the moving of the original module, and if Heap should instead be put in servers/navigation
Feedback on the tests, see TODOs there, and if any specific tests needs to be added or modified
Placement and potential splitting of settings between 2D and 3D (like debug settings)

Some additions I didn't make but that could be added, either in this or a follow-up:

Add an option to disable all navigation, disabling both the modules at once
Expand the Triangle2 class to be fully featured and move to core

Otherwise this is ready for testing, I will start using this in my own projects when I get around to working on them as well

For context, currently the 2D navigation system just runs as a frontend for the 3D navigation server, this is not only inefficient and involves a lot of magic to convert it also means that all navigation is unavailable when building without 3D support

Todo:

Integrate changes from Add functions to get axis-aligned bounds of navigation regions #101505

smix8 · 2025-01-14T02:15:11Z

We need to consider if we really want to stick with the same naming and enums. There have been multiple PRs that added new properties and debug info that at least I did withdraw because the timing for compatibility breaking was a problem. Before we now repeat all that old again for 2D and cement it further we might want to reconsider that.

Agreed, open for any suggestions, would suggest keeping the old data in Performance as is

smix8 · 2025-01-14T02:21:14Z

As mentioned in the server enum we might want to reconsider the debug monitors if we are changing things now.

smix8 · 2025-01-14T03:48:11Z

Note (to self), the NavigationServer documentation needs more or less a full rewrite with this and other recent changes.
https://docs.godotengine.org/en/latest/tutorials/navigation/navigation_using_navigationservers.html

AThousandShips · 2025-01-14T09:02:36Z

Will go over the navigation docs and the optimizing for size docs when this is done

AThousandShips · 2025-01-15T15:22:12Z

Updated the agent and obstacle related code, will look at the signal code and other details as well

AThousandShips · 2025-03-13T10:53:56Z

Will try to rebase this in the next few days

AThousandShips · 2025-03-21T17:23:15Z

Successfuly rebased, going to go through some of the code to ensure it was transferred correctly but should be good to go now 🎉

smix8

We discussed this before and I am in favor of doing this split, I just have no time to spend on reviewing and testing such a big PR in all detail atm. I did a quick test and so far nothing noticeable did break. Imo it should be merged now because we will not get a less painful window to merge such a big change anytime soon again. This PR affects way too many development branches and will be a constant rebase pita and bug source if delayed. There are a few things that I might want to change but it is not important to do now, it can be done before 4.5 release.

* Add a dedicated 2D server * Create dedicated tests * Split performance metrics between 2D and 3D * Rename the 3D only server module

Repiteo

This separation makes complete sense to me. Any reservations I have with the current implementation would amount to bikeshedding, and this shouldn't be held up any longer than it already has. Great work!

Repiteo · 2025-03-30T14:07:01Z

Thanks!

AThousandShips · 2025-03-30T14:09:17Z

Thank you! Great to have this done and dusted!

Will follow up with potential performance improvements and keep things under supervision

AThousandShips · 2025-03-30T14:12:39Z

Will update the building documentation to match this the coming week

aaronfranke · 2025-03-30T22:31:26Z

+		PNAME("navigation_2d/edges_connected"),
+		PNAME("navigation_2d/edges_free"),
+		PNAME("navigation_2d/obstacles"),
+		PNAME("navigation_2d/active_maps"),


Typo, duplicate navigation_2d/active_maps and missing navigation_3d/active_maps

Thank you for spotting, will fix today

aaronfranke · 2025-03-30T22:40:57Z

I rebased PR #89409 on top of this PR, but instead of just moving the navigation servers into the navigation folder, I split the navigation folder into navigation_2d and navigation_3d. Now that the 2D and 3D navigation servers are independent, we can separate them into folders and allow compiling without them.

WhalesState · 2025-04-17T21:37:01Z

I rebased PR #89409 on top of this PR, but instead of just moving the navigation servers into the navigation folder, I split the navigation folder into navigation_2d and navigation_3d. Now that the 2D and 3D navigation servers are independent, we can separate them into folders and allow compiling without them.

Was testing it and I have found that NavigationMesh is used by 2D navigation and it requires NavigationServer3D.

AThousandShips · 2025-04-18T10:00:02Z

It is used strictly for debugging, which can be enabled in non-3D exports, please open a bug report for the issues this might cause

NavigationPolygon doesn't use any 3D features as far as I can see, you can just generate a mesh from a polygon, and that shouldn't depend on any 3D features unless used with 3D features

WhalesState · 2025-04-18T19:32:09Z

It is used strictly for debugging, which can be enabled in non-3D exports, please open a bug report for the issues this might cause

I have allowed the editor to be built with 3D disabled, and the build failed because NavigationMesh includes NavigationServer3D for visual debugging so i will try to see if there's a workaround for that since BaseMaterial3D is not initialized when 3D is disabled.

Also I have spotted this in scene_debugger.cpp that may cause issues in the future, I know it's unrelated to this PR but i just wanted to let you know about it.

#ifndef PHYSICS_2D_DISABLED
			CollisionShape3D *collision_shape = Object::cast_to<CollisionShape3D>(node_3d);
			if (collision_shape) {
				Ref<Shape3D> shape = collision_shape->get_shape();
				if (shape.is_valid()) {
					bounds = shape->get_debug_mesh()->get_aabb();
				}
			}
#endif // PHYSICS_2D_DISABLED

I really appreciate The work you are doing since i have been waiting so long for this, thanks.

AThousandShips · 2025-04-19T12:04:14Z

That doesn't sound like it comes from this PR but from:

Allow to compile templates without navigation features #104811

But enabling building the editor without 3D should include fixing such things, so it's not really related to this PR if it fails with custom code, but if things fail while building the engine itself (not a modified version) please open an issue for it, but custom changes causing issues isn't really something that's related to my work

Rindbee · 2025-07-04T01:41:01Z

+		ERR_FAIL_NULL(parser);
+
+		generator_parsers.erase(parser);
+#ifndef CLIPPER2_ENABLED


There seems to be a typo here. It seems like it should be #ifdef instead of #ifndef.

If it is it's a typo in the original code, bur worth investing

AThousandShips added enhancement topic:navigation labels Jan 13, 2025

AThousandShips added this to the 4.x milestone Jan 13, 2025

AThousandShips requested review from a team as code owners January 13, 2025 18:45

AThousandShips mentioned this pull request Jan 13, 2025

[Navigation] Restore 2D only navigation #88681

Closed

smix8 added the performance label Jan 13, 2025

smix8 reviewed Jan 14, 2025

View reviewed changes

AThousandShips force-pushed the nav_split_new branch from 543a245 to 3de09f7 Compare January 15, 2025 15:16

AThousandShips force-pushed the nav_split_new branch from 2bdade5 to 736f6a0 Compare January 17, 2025 13:43

AThousandShips force-pushed the nav_split_new branch from 736f6a0 to 0d0b7a3 Compare January 24, 2025 17:25

AThousandShips mentioned this pull request Jan 27, 2025

[Navigation] Rename classes in preparation for future restructure #102100

Merged

AThousandShips force-pushed the nav_split_new branch from 0d0b7a3 to d35a55f Compare January 27, 2025 20:21

AThousandShips force-pushed the nav_split_new branch from d35a55f to 0d48e46 Compare February 8, 2025 17:27

AThousandShips force-pushed the nav_split_new branch from 0d48e46 to aba6a0f Compare March 21, 2025 17:22

AThousandShips removed request for a team March 21, 2025 17:25

AThousandShips force-pushed the nav_split_new branch from aba6a0f to e273e49 Compare March 21, 2025 18:43

AThousandShips force-pushed the nav_split_new branch 2 times, most recently from 37320ef to c25ef7b Compare March 22, 2025 12:28

smix8 modified the milestones: 4.x, 4.5 Mar 22, 2025

AThousandShips force-pushed the nav_split_new branch 3 times, most recently from af64fb7 to 408e31f Compare March 28, 2025 17:08

smix8 approved these changes Mar 28, 2025

View reviewed changes

[Navigation] Create a dedicated 2D navigation server

5cc0539

* Add a dedicated 2D server * Create dedicated tests * Split performance metrics between 2D and 3D * Rename the 3D only server module

AThousandShips force-pushed the nav_split_new branch from 408e31f to 5cc0539 Compare March 29, 2025 11:12

Repiteo approved these changes Mar 30, 2025

View reviewed changes

Repiteo merged commit 8b2952a into godotengine:master Mar 30, 2025

AThousandShips deleted the nav_split_new branch March 30, 2025 14:08

aaronfranke reviewed Mar 30, 2025

View reviewed changes

This was referenced Mar 31, 2025

[Navigation] Fix typo in performance #104829

Merged

[Navigation] Update contributing docs to reflect 2D navigation changes godotengine/godot-docs#10821

Merged

akien-mga mentioned this pull request Mar 31, 2025

Allow to compile templates without navigation features #104811

Merged

smix8 mentioned this pull request Apr 2, 2025

TileMapLayer build-in navigation prints navigation_map.is_null() errors on builds with disable_3d=true #96791

Closed

smix8 mentioned this pull request May 13, 2025

Rename nav_2d namespace to Nav2D to match Nav3D #106329

Merged

Calinou mentioned this pull request May 27, 2025

Update Optimizing for size for Godot 4.5 godotengine/godot-docs#10968

Merged

AThousandShips mentioned this pull request Jun 18, 2025

[Navigation] Update documentation after nav split godotengine/godot-docs#11033

Merged

Rindbee reviewed Jul 4, 2025

View reviewed changes

Uh oh!

Conversation

AThousandShips commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smix8 Jan 14, 2025

Choose a reason for hiding this comment

Uh oh!

AThousandShips Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smix8 Jan 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smix8 commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AThousandShips commented Jan 14, 2025

Uh oh!

AThousandShips commented Jan 15, 2025

Uh oh!

AThousandShips commented Mar 13, 2025

Uh oh!

AThousandShips commented Mar 21, 2025

Uh oh!

smix8 left a comment

Choose a reason for hiding this comment

Uh oh!

Repiteo left a comment

Choose a reason for hiding this comment

Uh oh!

Repiteo commented Mar 30, 2025

Uh oh!

AThousandShips commented Mar 30, 2025

Uh oh!

AThousandShips commented Mar 30, 2025

Uh oh!

aaronfranke Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

AThousandShips Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

aaronfranke commented Mar 30, 2025

Uh oh!

WhalesState commented Apr 17, 2025

Uh oh!

AThousandShips commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WhalesState commented Apr 18, 2025

Uh oh!

AThousandShips commented Apr 19, 2025

Uh oh!

Rindbee Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

AThousandShips Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AThousandShips commented Jan 13, 2025 •

edited

Loading

AThousandShips Jan 14, 2025 •

edited

Loading

smix8 commented Jan 14, 2025 •

edited

Loading

AThousandShips commented Apr 18, 2025 •

edited

Loading