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

NavigationRegion2D doesn't initialize its navigation_mesh correctly resulting in an error with a cell_size mismatch #83570

Closed
mrTag opened this issue Oct 18, 2023 · 2 comments · Fixed by #83568

Comments

@mrTag
Copy link
Contributor

mrTag commented Oct 18, 2023

Godot version

4.2 beta1

System information

Windows 10

Issue description

I was trying out the new 2D Navigation mesh baking features (great stuff, btw!) and immediately ran into an error after creating a NavigationRegion2D and a corresponding NavigationPolygon (all in the editor/inspector) :

modules/navigation/nav_region.cpp:128 - Navigation map synchronization error. Attempted to update a navigation region with a navigation mesh that uses a `cell_size` of 1 while assigned to a navigation map set to a `cell_size` of 0.25. The cell size for navigation maps can be changed by using the NavigationServer map_set_cell_size() function. The cell size for default navigation maps can also be changed in the ProjectSettings.

I haven't changed any of the cell_size settings anywhere and I can't even set the cell_size of the NavigationPolygon to 0.25, since the inspector for that is integer based. So I had a look at the code and the culprit seems to be in the constructor of the NavigationPolygon:

NavigationPolygon::NavigationPolygon() {
	navigation_mesh.instantiate();
}

The navigation_mesh is instantiated with its default values (cell_size being 0.25!) and the real initialization, where the cell_size is set, is then not executed, because navigation_mesh is not null:

Ref<NavigationMesh> NavigationPolygon::get_navigation_mesh() {
	MutexLock lock(navigation_mesh_generation);

	if (navigation_mesh.is_null()) {
		navigation_mesh.instantiate();
		Vector<Vector3> verts;
		{
			verts.resize(get_vertices().size());
			Vector3 *w = verts.ptrw();

			const Vector2 *r = get_vertices().ptr();

			for (int i(0); i < get_vertices().size(); i++) {
				w[i] = Vector3(r[i].x, 0.0, r[i].y);
			}
		}
		navigation_mesh->set_vertices(verts);

		for (int i(0); i < get_polygon_count(); i++) {
			navigation_mesh->add_polygon(get_polygon(i));
		}
		navigation_mesh->set_cell_size(cell_size); // Needed to not fail the cell size check on the server
	}

	return navigation_mesh;
}

So I removed the navigation_mesh instantiation from the NavigationPolygon constructor and went on with my testing without any further hitch. As far as I know navigation_mesh is only accessed through get_navigation_mesh(), where it is then instantiated lazily, so the instantiation in the constructor is not needed. I implemented and tested a whole navigation system for our game based on the NavigationRegion2D after that change and haven't noticed any problems (but I am only creating the navigation polygon at runtime via NavigationServer2D.bake_from_source_geometry_data_async, so maybe there are problems in other use cases 🤔).

I just drafted a PR: #83568

Steps to reproduce

  • Use the Godot 4.2 beta1 editor
  • Create a new project (or use an existing one)
  • Create a NavigationRegion2D node
  • In the inspector of that node create a "New NavigationPolygon"
  • Observe the "Navigation map synchronization error" in the Output tab

Minimal reproduction project

A reproduction project isn't really necessary, since the steps are so simple, but here is one anyway:
NavigationRegion2DTest.zip
(when you open the scene.tscn, you'll get the error as well)

@smix8
Copy link
Contributor

smix8 commented Oct 18, 2023

Thanks for the detailed report.

According to the error msg it is the other way around.

The navigation mesh has the correct cell size in that error.
It is the navigation map that has the incorrect cell size.

By default both "2D" navigation map and NavigationPolygon have a cell size of 1.0, or at least they should have.

@mrTag
Copy link
Contributor Author

mrTag commented Oct 18, 2023

I didn't even notice that in the error message. But it didn't line up with my experience with the code and so I had a look at the code of the error message:

ERR_PRINT_ONCE(vformat("Navigation map synchronization error. Attempted to update a navigation region with a navigation mesh that uses a `cell_size` of %s while assigned to a navigation map set to a `cell_size` of %s. The cell size for navigation maps can be changed by using the NavigationServer map_set_cell_size() function. The cell size for default navigation maps can also be changed in the ProjectSettings.", double(map->get_cell_size()), double(mesh->get_cell_size())));

and indeed: the two objects are the wrong way around in the format message! 🤦 The mesh cell size should be the first argument and the map cell size the second one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants