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

Support for long range off-mesh connection support #645

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

niji-k
Copy link

@niji-k niji-k commented Jul 5, 2023

This lifts the restriction for off-mesh connections for tiled navigation meshes
where the two endpoints for a connection must be located in the same tile, or in adjacent tiles.
This is done by connection the off-mesh links AFTER all tiles have been built.
You may find more detail on: #195 (comment)

Detour only stores off mesh connection in the starting tile. This means that if you have an off mesh connection pointing from tile A to B and you have loaded tile A first it will not be able to connect the off mesh connection when tile B is loaded. This can potentially give you some inconsistent results depending on the tile load order, should you choose to go with the "find exit tile" solution.
A potential fix to that problem could be to store off mesh connections in both tiles.

This "fix" is only crudely worked on as this issue suggests; an impropmtu fix.
#586 (comment)

More specifically, the change was only made to serve as a showcase, and although this should be working as intended with building the whole tile mesh, the same may not be applied to:

  • Building the tile mesh partially, i.e. building only one tile at a time,
  • & loading the tile mesh from a binary file.

To make it work for cases above, further exploration on this method of approach should be necessary.

@illwieckz
Copy link
Contributor

I reads:

After some googling and reading through lines of code, I realized that these connections would only work if they are within a single tile, or connecting two adjacent tiles. […]
I was able to produce a working fix, by forcing the code to add connection after all tiles are fully loaded.

I seen the commit effectively disables connectExtOffMeshLinks in dtNavMesh::addTile.

So I wondered, is there any technical way to add adjacent tile offmesh links like the current code does and only postpone non-adjacent tile offmesh links? Also, would there be any benefit in doing this? I guess this would still allow adjacent offmesh links to work if not all tiles are loaded yet.

This is just a thought I share, no more.

@niji-k
Copy link
Author

niji-k commented Jul 10, 2023

That should be easy to overcome, one simply would need to create a list of all unlinked off-mesh connections, as said in the comment below:
#477 (comment)

The only reason for my commit disabling connectExtOffMeshLinks() at said position, is that the behavior of two functions connectExtOffMeshLinks() and connectGlobalOffMeshLinks() are almost identical to each other.
As for the benefits, I'm not so sure. For all I know, it's just a matter between linking the tiles during the tile loading pass, or after said pass.

Mauler125 added a commit to Mauler125/r5sdk that referenced this pull request Jul 15, 2023
Merged the fix, but without the catch upgrade.
@RGreenlees
Copy link

This is excellent, are you planning an implementation that supports the tile cache (Sample_TempObstacles) as well? I'm not sure how to integrate this into the tile cache.

@mastahg
Copy link

mastahg commented Aug 7, 2023

Some feedback on this.

1)It works

2)The documentation for the function is incorrect in my understanding, target is actually where the starting point is, and tile is the end point. It's reverse what you would expect.

3)The sample connection establishment is extremely slow for meshes with large amounts of tiles

First I moved it to be a member function of dtNavMesh, as it's something I think should be supplied and simply called by anyone using the library.

I then focused on optimizing the function call.

I altered and rearranged the checks, continuing the loop if

if (target->header->offMeshConCount == 0)
continue

this was the first major speedup as there's no point looping thru the X range if there's no offmesh connections in the target tile.

I then added a new variable to the dtMeshTile class unconnectedOffMeshCount, and during tile construction set this to the header->offMeshConCount.
This value would be decremented inside the end of connectGlobalOffMeshLinks and connectExtOffMeshLinks.

This value is also checked, at the end of dtAddtile and if it has a value > 0 then m_hasOffMesh is set to true.

This gives a huge performance benefit for meshes where there are only connections inside the same tile that we don't have to scan the entire tile range to find start/end connections.

Further possible improvements:

It maybe worthwhile to keep the neighbor checks in, as searching neighbors is faster then scanning the entire tile range.

It maybe worthwhile to import the rcVector class and add the tiles where unconnectedOffMeshCount > 0 that way we don't scan the entire tile range

It maybe faster to use a navmeshquery to find the what the end tile would be for a given off mesh connection, rather then scanning all the given tiles.

void dtNavMesh::ConnectGlobalOffMeshLinks()
{
	if (m_hasOffMesh)
	{
		const int tileNum = this->getMaxTiles();
		for (int y = 0; y < tileNum; ++y)
		{
			const auto target = this->getTile(y);
	
			if (!target->data)
				continue;
	
			if (target->unconnectedOffMeshCount == 0)
				continue;
	
			for (int x = 0; x < tileNum; ++x)
			{
				if (x == y) 
					continue;

				const auto tile = this->getTile(x);
				
				if (!tile->data) 
					continue;

				this->connectGlobalOffMeshLinks(tile, target);

				if (target->unconnectedOffMeshCount == 0)
					break;

			}
		}
	}
}

@mastahg
Copy link

mastahg commented Aug 7, 2023

So I played around with this a bit more

I ported over rcVectorBase and renamed to dtVectorBase etc, and during the dtnavmesh::addtile, checked if header->offMeshConCount > 0 added it to the vector.

Then a simple function


void dtNavMesh::ConnectGlobalOffMeshLinks()
{
	if (m_hasOffMesh)
	{
		for(const auto target : tilesWithOffmeshConnections)
		{
			GlobalOffMeshLinks(target);
		}

}

and

smashed together the logic from dtNavMeshQuery::queryPolygons and the previous function create the following:

void dtNavMesh::GlobalOffMeshLinks(dtMeshTile* target)
{
	if (!target) 
		return;

	for (int i = 0; i < target->header->offMeshConCount; ++i)
	{
		dtOffMeshConnection* targetCon = &target->offMeshCons[i];

		dtPoly* targetPoly = &target->polys[targetCon->poly];
		// Skip off-mesh connections which start location could not be connected at all.
		if (targetPoly->firstLink == DT_NULL_LINK)
			continue;

		const float ext[3] = { targetCon->rad, target->header->walkableClimb, targetCon->rad };


		float bmin[3], bmax[3];
		dtVsub(bmin, &targetCon->pos[3], ext);
		dtVadd(bmax, &targetCon->pos[3], ext);

		// Find tiles the query touches.
		int minx, miny, maxx, maxy;
		this->calcTileLoc(bmin, &minx, &miny);
		this->calcTileLoc(bmax, &maxx, &maxy);

		static const int MAX_NEIS = 32;
		dtMeshTile* neis[MAX_NEIS];

		for (int y = miny; y <= maxy; ++y)
		{
			for (int x = minx; x <= maxx; ++x)
			{
				const int nneis = this->getTilesAt(x, y, neis, MAX_NEIS);
				for (int j = 0; j < nneis; ++j)
				{
					auto tile = neis[j];
					// Find polygon to connect to.
					const float* p = &targetCon->pos[3];
					float nearestPt[3];
					dtPolyRef ref = findNearestPolyInTile(tile, p, ext, nearestPt);
					if (!ref)
						continue;
					// findNearestPoly may return too optimistic results, further check to make sure. 
					if (dtSqr(nearestPt[0] - p[0]) + dtSqr(nearestPt[2] - p[2]) > dtSqr(targetCon->rad))
						continue;
					// Make sure the location is on current mesh.
					float* v = &target->verts[targetPoly->verts[1] * 3];
					dtVcopy(v, nearestPt);

					// Link off-mesh connection to target poly.
					unsigned int idx = allocLink(target);
					if (idx != DT_NULL_LINK)
					{
						dtLink* link = &target->links[idx];
						link->ref = ref;
						link->edge = (unsigned char)1;
						link->side = 0xff;
						link->bmin = link->bmax = 0;
						// Add to linked list.
						link->next = targetPoly->firstLink;
						targetPoly->firstLink = idx;
					}

					// Link target poly to off-mesh connection.
					if (targetCon->flags & DT_OFFMESH_CON_BIDIR)
					{
						unsigned int tidx = allocLink(tile);
						if (tidx != DT_NULL_LINK)
						{
							const unsigned short landPolyIdx = (unsigned short)decodePolyIdPoly(ref);
							dtPoly* landPoly = &tile->polys[landPolyIdx];
							dtLink* link = &tile->links[tidx];
							link->ref = getPolyRefBase(target) | (dtPolyRef)(targetCon->poly);
							link->edge = 0xff;
							link->side = (unsigned char)(0xff);
							link->bmin = link->bmax = 0;
							// Add to linked list.
							link->next = landPoly->firstLink;
							landPoly->firstLink = tidx;
						}
					}
				}
			}
		}
	}
}

this new method is exceptionally fast, two orders of magnitude faster for meshes with large tile counts, and handles all connections just fine so I even went and disabled the self tile check inside addtile, so that all offmesh connections are handled thru this one function.

Concerns that are outside my usecase:
Removing tiles and then readding.

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.

None yet

5 participants