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

GetSmoothPath along straight lines returns diagonal movement #12

Closed
mhughson opened this issue Mar 29, 2018 · 12 comments
Closed

GetSmoothPath along straight lines returns diagonal movement #12

mhughson opened this issue Mar 29, 2018 · 12 comments
Assignees
Labels

Comments

@mhughson
Copy link

I have some really simple test cases that seem appear to return invalid smoothed paths.

In these screenshots, the light blue boxes are the path returned from GetSmoothPath. The agent is attempting to walk to each corner of the darker blue square.

The grid has no solid objects and a consistent cost (1.0f) across all cells in the grid.

2018-03-28 1

2018-03-28 3

With GetSmoothPath I would have expected all diagonal movement to be removed from the path.

Here is the same test but with GetPath(..., MovementPatterns.LateralOnly);

2018-03-28 6

@roy-t
Copy link
Owner

roy-t commented Mar 29, 2018

Hey @mhughson, I've been able to reproduce this bug:
capture.

It seems that if there are 3 or more tiles between the start and finish, and no obstructions, and enough space. The string pulling algorithm is only able to pull a small part of the path straight (without smoothing its a true triangle, now the top is pulled off). I will investigate more, but I think I will need to run the string pulling algorithm recursively, or make another small adjustment.

Interesting that I never saw this happen before because my test application starts in the top left corner of a grid, and due to ordering of neighbours the errors are only to the top-left direction.

@roy-t roy-t added the bug label Mar 29, 2018
@roy-t roy-t self-assigned this Mar 29, 2018
@roy-t
Copy link
Owner

roy-t commented Mar 29, 2018

I've found the problem, the string pulling is too simplistic, I've worked out a better solution on my whiteboard and I only have one programming problem to solve before I can show something, but a few quick tests showed it solved these problems :).

(What I need now is to find all the cells that are on a line from one cell to another cell, but due to floating point inaccuracies this is proving slightly more difficult than I thought and I want to keep the code as simple as possible).

I'm probably going to need to implement this: https://en.wikipedia.org/wiki/Bresenham%27s_line_algorithm :)

Sneak preview:
astar

@mhughson
Copy link
Author

Does it make sense that a diagonal move costs the same as a lateral one? They are not the same distance. In implementations I've done in the past, I believe I made a diagonal cost slightly more than a lateral move, but cheaper than 2 lateral moves. Basically I made the cost of movement equal to the distance between the 2 points.

I believe that would solve this issue without smoothing being needed, but perhaps it would introduce other oddities.

I'm probably going to need to implement this: https://en.wikipedia.org/wiki/Bresenham%27s_line_algorithm

That's actually the algorithm I used to draw the blue lines in my examples above, so we've come full circle. 😄

@roy-t
Copy link
Owner

roy-t commented Mar 30, 2018

Originally I wanted the heuristic and cost calculations to be as simple as possible. So I used Chebyshev distance for the heuristic and equal costs for the diagonal and horizontal/vertical movement. Then using the string pulling algorithm to create nicer looking paths. My reasoning for this was that the string pulling only needs to consider the cells on the path, so that doing a little extra work afterwards would speed up things, compare to making all the costSoFar and stepCost calculations more difficult.

I had quite a few samples in which the string pulling algorithm worked really well. So I thought this was the right decision. But if I see now how much more complex (its >O(n^2) in the test branch) I have to make the string pulling algorithm to smooth out this case it might have been the wrong approach.

I hope I have time this weekend to experiment a bit, and hopefully release a better version that solves your problems.

@roy-t
Copy link
Owner

roy-t commented Mar 31, 2018

@mhughson I've used your feedback to create a v2 version of the package, that does penalize diagonal movement, and I'm happy to say that it looks like it no longer needs any string pulling or other methods to make the paths look good. Please try it out and let me know how you like it, so I can close this issue :).

@mhughson
Copy link
Author

mhughson commented Apr 1, 2018

Is it possible to try via NuGet?

@roy-t
Copy link
Owner

roy-t commented Apr 1, 2018

@mhughson yes! Its the 2.0.0 version I uploaded to last night to nuget: https://www.nuget.org/packages/RoyT.AStar/ if you already have the package you should be able to see there is an update in VS :).

@mhughson
Copy link
Author

mhughson commented Apr 2, 2018

Getting a crash after after a short period:

 	mscorlib.dll!System.Collections.Generic.List<RoyT.AStar.Position>.Capacity.set(int value) Line 123	C#
 	mscorlib.dll!System.Collections.Generic.List<RoyT.AStar.Position>.EnsureCapacity(int min) Line 414	C#
 	mscorlib.dll!System.Collections.Generic.List<RoyT.AStar.Position>.Add(RoyT.AStar.Position item) Line 222	C#
 	RoyT.AStar.dll!RoyT.AStar.PathFinder.ReconstructPath(RoyT.AStar.Grid grid, RoyT.AStar.Position start, RoyT.AStar.Position end, RoyT.AStar.Position[] cameFrom)	Unknown
 	RoyT.AStar.dll!RoyT.AStar.PathFinder.FindPath(RoyT.AStar.Grid grid, RoyT.AStar.Position start, RoyT.AStar.Position end, RoyT.AStar.Offset[] movementPattern)	Unknown
 	RoyT.AStar.dll!RoyT.AStar.Grid.GetPath(RoyT.AStar.Position start, RoyT.AStar.Position end, RoyT.AStar.Offset[] movementPattern)	Unknown
>	mono8samples.exe!Mono8Samples.TopDown.simple_ai.move_to_pos(float dest_x, float dest_y) Line 232	C#

Exception (the internal list you are using has 100 million + entries...

System.OutOfMemoryException was unhandled
  HResult=-2147024882
  Message=Array dimensions exceeded supported range.
  StackTrace:
       at System.Collections.Generic.List`1.set_Capacity(Int32 value) in f:\dd\ndp\clr\src\BCL\system\collections\generic\list.cs:line 123
       at System.Collections.Generic.List`1.EnsureCapacity(Int32 min) in f:\dd\ndp\clr\src\BCL\system\collections\generic\list.cs:line 414
       at System.Collections.Generic.List`1.Add(T item) in f:\dd\ndp\clr\src\BCL\system\collections\generic\list.cs:line 222
       at RoyT.AStar.PathFinder.ReconstructPath(Grid grid, Position start, Position end, Position[] cameFrom)
       at RoyT.AStar.PathFinder.FindPath(Grid grid, Position start, Position end, Offset[] movementPattern)
       at RoyT.AStar.Grid.GetPath(Position start, Position end, Offset[] movementPattern)
       at Mono8Samples.TopDown.simple_ai.move_to_pos(Single dest_x, Single dest_y) in C:\Users\Matt\Documents\Dev\Game\mono8\Mono8Samples\TopDown\simple_ai.cs:line 232
       at Mono8Samples.TopDown.simple_ai.update() in C:\Users\Matt\Documents\Dev\Game\mono8\Mono8Samples\TopDown\simple_ai.cs:line 94
       at Mono8Samples.TopDown.TopDown._update60() in C:\Users\Matt\Documents\Dev\Game\mono8\Mono8Samples\TopDown\TopDown.cs:line 302
       at Mono8.Mono8Game`1.Update(GameTime gameTime) in C:\Users\Matt\Documents\Dev\Game\mono8\Mono8\Mono8Game.cs:line 456
       at Microsoft.Xna.Framework.Game.DoUpdate(GameTime gameTime)
       at Microsoft.Xna.Framework.Game.Tick()
       at MonoGame.Framework.WinFormsGameWindow.RunLoop()
       at Microsoft.Xna.Framework.Game.Run(GameRunBehavior runBehavior)
       at Mono8Samples.Program.Main() in C:\Users\Matt\Documents\Dev\Game\mono8\Mono8Samples\Program.cs:line 19
       at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
       at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args) in f:\dd\ndp\clr\src\BCL\system\appdomain.cs:line 1966
       at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
       at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) in f:\dd\ndp\clr\src\BCL\system\threading\executioncontext.cs:line 954
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) in f:\dd\ndp\clr\src\BCL\system\threading\executioncontext.cs:line 902
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) in f:\dd\ndp\clr\src\BCL\system\threading\executioncontext.cs:line 891
       at System.Threading.ThreadHelper.ThreadStart() in f:\dd\ndp\clr\src\BCL\system\threading\thread.cs:line 111
  InnerException: 

Here's the call that crashes and some of the values:

Position[] path = Game.map_grid.GetPath(new Position((int)(x / 8), (int)(y / 8)), new Position((int)(dest_x / 8), (int)(dest_y / 8)));

-		Game.map_grid	{RoyT.AStar.Grid}	RoyT.AStar.Grid
		DefaultCost	1	float
		DimX	128	int
		DimY	128	int
+		Weights	{float[16384]}	float[]
		dest_x	36	float
		dest_y	4	float
		path	null	RoyT.AStar.Position[]
+		this	{Mono8Samples.TopDown.simple_ai}	Mono8Samples.TopDown.simple_ai
		x	36	float
		y	7.75	float

@roy-t
Copy link
Owner

roy-t commented Apr 2, 2018

@mhughson ah, I figured it out. It happens when the start and end position are the same but are not (0,0). This is something that can never happen in the Viewer, but is of course something that can happen when the library is used. I'll update the Nuget package in a few minutes! (It might take a few hours before you can see the update). Sorry about this!

@Maxoper
Copy link

Maxoper commented Apr 2, 2018

@roy-t great work Roy ! Using this library since more than 2 months.
Wanna say thank you this way :)

@mhughson
Copy link
Author

mhughson commented Apr 3, 2018

Seems to work!

@roy-t roy-t closed this as completed Apr 4, 2018
@roy-t
Copy link
Owner

roy-t commented Apr 4, 2018

@Maxoper its truly appreciated :)

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

No branches or pull requests

3 participants