Possible bug in DeserializeGraphsAdditive

Hi Aaron,

I’m running version 4.2.6 in Unity 2018.2.20f1.

I’ve encountered what appears to be a bug when using DeserializeGraphsAdditive to append graphs to my scene at run-time, unless of course I’m understanding it’s purpose incorrectly.

If for example I have two navmeshes that I would like to load in my scene at run-time, I’d call the following code for each of the two navmesh .bytes files to pull them from my Resources folder and into the Astar Path instance:

ResourceRequest resourceLoadRequest = Resources.LoadAsync(path);
		yield return resourceLoadRequest; // wait until ready
		var textAsset = (TextAsset)resourceLoadRequest.asset;
		if (textAsset != null)
		{
			AstarPath.active.data.DeserializeGraphsAdditive(textAsset.bytes);  
		}

The problem I’m encountering when doing this is the second call to DeserializeGraphsAdditive somehow causes the first loaded navmesh graph to duplicate. I would expect there to be a total of 2 elements in the AstarPath.active.data.graphs array, however, I have 3. The first two are duplicates of the first loaded graph and the third is the correct single instance of the graph that was loaded on the second call to DeserializeGraphsAdditive .

I’ve reworked my navmesh cutting sample project from my last forum post to reproduce this issue for you available in the following unity package.

Please note that my intention is to append graphs incrementally at runtime so that each time I call DeserializeGraphsAdditive the graph is appended to AstarPath.active.data.graphs without clearing or duplicating any existing loaded graphs, so hopefully I’m not confused about the intended functionality of DeserializeGraphsAdditive.

Thanks in advance!
Todd

Hi

Note that if you have any graph in the scene when the game is started, DeserializeGraphsAdditive will still additively load whatever you had in the file. So if your first graph was in the scene, you will both have it from the start and then also add it later, thus a duplicate.

You might want to clear all graphs before you begin loading. Easiest way might be to not use additive loading for the first graph that you load.

1 Like

I apparently forgot to add that to my sample, but here is what I’m actually doing in game:

	private void Awake()
	{
		mSelf = this;

		// this is necessary to clear any navmesh that may have been loaded at design time due to clicking the Generate Navmesh button
		UnloadAll();
	}

	public static void UnloadAll()
	{
		if (AstarPath.active != null && AstarPath.active.data != null)
		{
			var graphs = AstarPath.active.data.graphs;
			for(int i = 0; i < graphs.Length; i++)
			{
				AstarPath.active.data.RemoveGraph(graphs[i]);
			}
			AstarPath.active.data = new AstarData();
		}
	}

I’m assuming this is sufficient to clear any graphs I had in my scene at design time. That said, I’m still trying to understand if DeserializeGraphsAdditive can facilitate my need here. I’m making a game in which it is necessary to stream specific navmeshes in and out at run time, depending on triggers the player passes through. In order to remove a specific graph from the currently loaded navmeshes, AstarPath.active.data.RemoveGraph appears to facilitate that, however what I also need is the inverse of that, in which I can append a single graph which will add itself to the loaded graphs, but not cause duplication in the currently loaded graphs. Is that not the intended purpose of DeserializeGraphsAdditive?

Hi

You are correct in that this is exactly how DeserializeGraphsAdditive is intended to work.

There is a possible race condition in that script though. Since your UnloadAll code runs during Awake, and the AstarPath deserialization code also runs during Awake it is possible that your unloading code runs first before the AstarPath code has even been initialized and thus it would just do nothing (AstarPath.active would return null). I would suggest moving it to run during Start.

Also, I would not recommend setting AstarPath.active.data to a new AstarData object, that might break things. Clearing the graphs exactly like you do should be sufficient. Though I would suggest doing something like

if (graphs[i] != null) ...RemoveGraph(graphs[i]);

as elements in the graph array are allowed to be null.

It may help to know that DeserializeGraphs is implemented literally as a method that first clears all graphs and then calls DeserializeGraphsAdditive.
image

Thanks for your feedback on clearing the graphs on start.

So for the record I’ve finally pinpointed the cause of what I thought was a bug in your code, however, as you implied it was a bug in mine… The problem was not the fact that I wasn’t clearing the graphs at run-time within the scope of Awake or Start before calling DeserializeGraphsAdditive. Rather, it was that I wasn’t clearing the graphs before programatically calling Scan() via my CreateNavmeshFunction.


	public static bool CreateNavMesh(MeshFilter meshFilter, string name)
	{
		string path = string.Format("Assets/Resources/{0}.bytes", name);

		Mesh tempMesh = new Mesh
		{
			name = name,
			vertices = meshFilter.sharedMesh.vertices.Select(x => meshFilter.transform.TransformPoint(x)).ToArray(), // apply world position to mesh in order to ensure navmesh loads properly at runtime  
			triangles = meshFilter.sharedMesh.triangles,
			normals = meshFilter.sharedMesh.normals,
			uv = meshFilter.sharedMesh.uv,
			tangents = meshFilter.sharedMesh.tangents
		};

		if (AstarPath.active.data == null) AstarPath.active.data = new AstarData();

		AstarPath.active.data.FindGraphTypes(); // forces the internal code to populate the graphTypes array to avoid an intermittent object ref exception in AddGraph


//NOTE: here was the problem, AstarPath.active.data.graphs already had the last graph I'd scanned in memory, so when AstarPath.active.Scan is called below, I'd end up with a .bytes file containing multiple graphs instead of the one I was intending to scan to file.
		var graph = AstarPath.active.data.AddGraph(typeof(NavMeshGraph)) as NavMeshGraph;
		graph.sourceMesh = tempMesh;
		graph.name = name;

		AstarPath.active.Scan();

		// important - clear the mesh after Scan and before SerializeGraphs to prevent hang that occurs loading this asset at runtime.
		// see http://forum.arongranberg.com/t/astarpath-active-data-deserializegraphs-textasset-bytes-very-slow/5886/3 for more info.
		graph.sourceMesh = null;

		// guid's are not automatically generated, so assign them here before saving to file
		foreach (var g in AstarPath.active.data.graphs)
		{
			if (g != null && g.guid == Pathfinding.Util.Guid.zero)
			{
				g.guid = Pathfinding.Util.Guid.NewGuid();
			}
		}

		var serializationSettings = Pathfinding.Serialization.SerializeSettings.Settings;
		serializationSettings.nodes = true;
		uint checksum;
		var bytes = AstarPath.active.data.SerializeGraphs(serializationSettings, out checksum);
		System.IO.File.WriteAllBytes(path, bytes);

		AssetDatabase.Refresh();

		EditorUtility.DisplayDialog("Complete", string.Format("The Navmesh was successfully saved to '{0}'", path), "Ok");
		return true;
	}

Thanks for the feedback, problem solved!

Hi

Ah I see. Nice that you found the issue!

Your code uses some parts of the internal API that is usually not used by user code. If you are running this in the unity editor outside of play mode I would recommend that you use the method call AstarPath.FindAstarPath() (as mentioned here: https://arongranberg.com/astar/docs/editormode.html).
Then you can skip creating the AstarData class yourself and calling FindGraphTypes.
It will also work more consistently as right now it is not guaranteed that the AstarPath.active property is actually set (it will only be set if the AstarPath inspector has been visible).

Note that this bug has been fixed in a recent update. So if you are using a recent version you should not need this anymore.

Awesome, I’ll upgrade to the latest and use FindAstarPath from now on, thanks again!

1 Like