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

Make 2D navigation mesh baking parse all TileMapLayers #85856

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

smix8
Copy link
Contributor

@smix8 smix8 commented Dec 6, 2023

Makes 2D navigation mesh baking parse all TileMapLayers.

This lifts the limitation of the 2D navigation mesh baking only parsing the first TileMapLayer, due to popular demand.

Fixes #85798 and all the other billion linked issues of the same kind.

Note that this change is intended to enable workflows where users have their parsed polygons on another TileMapLayer than the first. The 2D navigation mesh baking still treats the Tilemap data as if it is done with a single TileMapLayer, it only now parses data from all TileMapLayers, picks the first valid data for a cell found, and combines them all together.

The parsing process will try to block the most common user errors by ignoring all TileMap cell keys that already had a navigation mesh or collision shape that fits the collision mask parsed and print a warning instead.

  • Having navigation mesh for the same cell on 2 or more TileMapLayers is an error.
  • Having collision polygons with the parsed collision mask for the same cell on 2 or more TileMapLayers is an error.

For navigation polygons the build and placement rules are no surface overlap at all is allowed on the same navigation map. The only thing that can "overlap" between 2 navigation meshes are unique pairs of edges in order to merge them.

For collision polygons the build and placement rules are no overlap if it has a physics layer bit that is parsed by the NavigationPolygon collision mask. This means if users have 2 TileMapLayers with collision polygons placed in the same cell, 1 intended for the navigation mesh parsing and 1 intended for e.g. gameplay the gameplay TileMapLayer should not have a matching bit on the collision layer with the NavigationPolygon parse collision mask, else it will be treated as duplicates with errors and rejected cell data.

@smix8 smix8 added this to the 4.3 milestone Dec 6, 2023
@smix8 smix8 added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 6, 2023
@smix8 smix8 requested a review from groud December 6, 2023 16:53
@smix8 smix8 force-pushed the tilemap_layers_parse branch from 5efbede to 14d67d4 Compare December 15, 2023 03:32
@InfiniteCactusDev
Copy link

InfiniteCactusDev commented Dec 16, 2023

Just out of interest. Why aren't you first combining (union in Clipper) all the layer-polygons before using them for navmeshbaking? Functionally this would give the desired result and it should solve these problems as well, right?

@smix8 smix8 force-pushed the tilemap_layers_parse branch from 14d67d4 to e633be4 Compare February 27, 2024 16:40
@smix8 smix8 force-pushed the tilemap_layers_parse branch from e633be4 to aae7009 Compare April 4, 2024 07:59
@smix8
Copy link
Contributor Author

smix8 commented Apr 4, 2024

Added a max error print counter to not flood the debugger when the Tilemap has many overlapping cells.

Instead after this cap it prints just once the total error count. I think 10 error prints is a reasonable cap.

Makes 2D navigation mesh baking parse all TileMapLayers.
@smix8 smix8 force-pushed the tilemap_layers_parse branch from aae7009 to d6ddeec Compare April 4, 2024 08:45
@smix8 smix8 removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Apr 4, 2024
@akien-mga akien-mga merged commit 7744d59 into godotengine:master Apr 5, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@874808039
Copy link

Thanks, Bug this new TilemapLayer node is not working(#89179 new TilemapLayer node cannot yet cooperate with NavigationRegion2D and cannot bake navigation polygons.Can this function be synchronized on the new node?

@xynanlee
Copy link

xynanlee commented May 2, 2024

Does this PR solves the long standing TileMap navigation meshes "cutout" issue? I tried TileMapLayer a bit and it seems not the case.

@874808039
Copy link

Does this PR solves the long standing TileMap navigation meshes "cutout" issue? I tried TileMapLayer a bit and it seems not the case.

If you click to bake, will it be successful?
image

@xynanlee
Copy link

xynanlee commented May 2, 2024

Does this PR solves the long standing TileMap navigation meshes "cutout" issue? I tried TileMapLayer a bit and it seems not the case.

If you click to bake, will it be successful? image

I'm not using NavigationRegion2D at all. There has been a long standing issue where tilemap navigation layers unable to be "cut out" by physics layers in other tilemap layers. Looking at this PR, I'm getting the impression of 1 TileMapLayer with navigation layer can parse multiple TileMapLayers with physic layers to do the "cutout". I tried it on myside and it doesn't seemed to be the case.

@LeifintheWind
Copy link

LeifintheWind commented May 31, 2024

Does this (or other related PRs) include changes for tilemap Physics Layers?

At the moment in 4.2 it appears as though no matter the Collision Masks parsed by the NavigationPolygon, only tilemap Physics Layer 0 is used.

As an example, I currently have two collision layers used in my tilemap: one for preventing ground navigation (mask layer 1), one for preventing flying (mask layer 2).

Physics Layer 0 uses Collision Mask 1 (only prevent walking)
Physics Layer 1 uses Collision Masks 1 and 2 (prevent both walking and flying)
Physics Layer 2 uses Collision Mask 2 (only prevent flying)

When attempting to bake the nav polygon using Collision Mask 1 it seems to only bake according to that first Physics Layer, and ignores the others whether or not they include Collision Mask 1.

If that's not changed, then to create a NavigationPolygon for walking, it's easy enough to just remove Physics Layer 1 and recreate all the necessary collision polygons in the TileSet with Physics Layer 0. But to create a NavigationPolygon for flying I would need to copy the whole tilemap and change it to have a Physics Layer 0 using Collision Mask 2. When creating large maps that have a mix of Masks and Layers, that seems like it could get confusing and messy real quick. Exponentially so with any more than two Collision Masks used.

@RuinRui
Copy link

RuinRui commented Jul 13, 2024

I checked many navigation issues in the official forums, and the conclusion is that the 2D navigation layer is just a series of polygons, without the concept of layers, including the physics layer。
so even if it is solved, it is only by cutting off all merged physical layers before merging the navigation?
This fix only fixes the problem that only the first navigation layer on multiple tilemap layers is effective, and it is achieved by merging the navigation polygons in all navigation layers on the tilemap. This is my understanding.

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.

NavigationRegion2D doesn't take into account tiles with collision on other layers besides the first one
8 participants