next »

[Kylotan]

It's probably a bad idea to call DeleteAllLists in a coroutine. What is the purpose of you doing that? It just means that the lists are in a maybe-deleted-and-maybe-not state until that finishes, and you need to be careful not to access those lists until then.

When reporting a crash it's essential to point out exactly what line it crashes on, what the error says, and ideally which function the current one (and down the stack), and so on. Unity provides a full call stack in the editor so you can probably copy and paste that.

Also, your logic is very wrong in at least one place - you keep adding to the 'adjacent_nodes' list each time you expand a new node, and only clear it at the end. Normally adjacent nodes are just added to the open list for future consideration - if you need to collect them into a list first, you'd expect to empty that list after you add them to the open list.

[Vampade]

Is it crashing in the same way as before when the path is reached?

not exactly. It crashes after it reached the path when found_path and reached_dest are set to false to search the path again.

It's probably a bad idea to call DeleteAllLists in a coroutine. What is the purpose of you doing that?

ops sorry here, I tried to make it a coroutine that so I was sure it called FindPath() when it finished the execution of DeleteAllLists() but it didn't change nothing. So I made DeleteAllLists() a void but I forgot to change the calls in the update before updloading the code ^_^'

When reporting a crash it's essential to point out exactly what line it crashes on, what the error says, and ideally which function the current one (and down the stack), and so on. Unity provides a full call stack in the editor so you can probably copy and paste that.

Yes, I know but the problem, the thing that is making a lot much more harder to me and to all the good people that is helping is that it doesn't throw any error, nothing. Unity just totally crashes and I have to exit from the task manager. Anyway, it crashes in DeleteAllLists() when it clears the closed list. I tried to clear it alone and yes, when I clear the closed list it crashes and in the taskmanager there's a memory consumption of about 150/160mb.

you keep adding to the 'adjacent_nodes' list each time you expand a new node, and only clear it at the end. Normally adjacent nodes are just added to the open list for future consideration

Didn't notice that error, thank you for pointing me that out. I made this because in the foreach I thought "if I put the open list it will check also the already analyzed nodes" but apparently I didn't think good.

[quantumpotato]

I still recommend changing "DeleteAllLists" to something that more accurately describes what you're doing like, "ResetPath"..

[Vampade]

Plot twist: the game stops crashing if I put directly the adjacent nodes in the open list and I don't loop through the adjacent_nodes list in the foreach in FindPath but instead I loop the nodes of the open_list. But the path isn't correctly made, in fact only 2/3 nodes are put in the path_nodes list and the nodes don't have the parent. So I tried again and used the list adjacent_nodes as it was used in the code I posted (when I use adjacent_nodes normally the game crashes), builded the game and gave a look at the output_log and there are a lot of NullReferenceExecptions that aren't thrown in the editor here what it looks like:

Code:

NullReferenceException: Object reference not set to an instance of an object
  at AStar.BackTracePath () [0x00000] in <filename unknown>:0 
  at AStar.FindPath (UnityEngine.GameObject start_node, UnityEngine.GameObject target_node) [0x00000] in <filename unknown>:0 
  at AStar.Start () [0x00000] in <filename unknown>:0 
 
(Filename:  Line: -1)

NullReferenceException: Object reference not set to an instance of an object
  at AStar.GetNodePos (Int32 n_node) [0x00000] in <filename unknown>:0 
  at AStar.AssignFollowVars () [0x00000] in <filename unknown>:0 
  at AStar.FollowPath () [0x00000] in <filename unknown>:0 
  at AStar.Update () [0x00000] in <filename unknown>:0 

(Filename:  Line: -1)
 

then the 2nd NullReferenceException part keeps repeating, I think because it's the part in the update or something like this (?) what do you suggest guys?

UPDATE:

I still recommend changing "DeleteAllLists" to something that more accurately describes what you're doing like, "ResetPath"..

yes, you're right edited the code I posted.

I tried to build the game without using adjacent nodes but by putting the adjacent nodes directly in the open list that so it doesn't crash. Opened the output_log file and it was the same as the version that crashed (the "normal" one that uses adjacent nodes) so I think it's normal

[Kylotan]

It's weird (but not unprecedented) that Unity crashes entirely, as that's normally a bug in their code, not yours. So I'm glad that's behind you.

Regarding your new bug, you're showing us the error stack (great!) but not the new code that causes it - so I can't suggest anything.

Still, there's no magic here, and you have all the clues you need. It says 'NullReferenceException' so something on in that function is clearly null when you're not expecting it to be. Are you not seeing call stacks in the Console window, that would give you the exact line of the error? Anyway, the main culprits are:

targ_node may be null
targ_node.my_node may be null
path_nodes may be null

Path_nodes looks like it's never null, so it's probably one of the other two - and you should be checking the state of those in Start(). (And probably not calling FindPath in Start - that doesn't sound like initialisation to me.)

[Vampade]

Regarding your new bug, you're showing us the error stack (great!) but not the new code that causes it - so I can't suggest anything.

I simply changed adjacent_nodes with open_list in the code, instead of using adjacent_nodes I directly put everything in the open_list.

It says 'NullReferenceException' so something on in that function is clearly null when you're not expecting it to be. Are you not seeing call stacks in the Console window, that would give you the exact line of the error?

Yes I know that there are errors but in the log file it says the errors are at line -1 as you saw, what does it mean?
The console window in Unity doesn't show anything, no errors.

Anyway thanks for your input I'll see where it brings me 

[Vampade]

Hey guys, I admit that I made a really messed up AI so I decided to rewrite some of its critical parts.

I modified the part that checks the adjacent nodes, it's not a list in the pathfinding script anymore but each node stores its adjacent nodes and the pathfinding accesses this list in the node when he needs it, so no more huge lists of 100+ adjacent nodes.
To find them the node simply casts rays in every directions and if they hit nodes they collect them in a list.

Now the nodes of a path have both a child node and a parent node (obviously if one is the first of a path it won't have a parent, same for the last that won't have a child). I made it because I want to make the follow path system that instead of choosing the next node by seeing its index it goes to the child node, it's more "elegant" I think, and also because at the middle of the path the enemy stops going on and starts moving between the node he reached and the previous one.

Here you can see the changes I made: http://pastebin.com/C9y292SM

[Kylotan]

Several issues I see:

1) You check to see if the open list contains 'my_node' when you start a search. That shouldn't be necessary because your open list needs to be empty. If it's not empty then you're not doing A*.

2) Similarly you shouldn't need to check if 'my_node' is on the closed list or not before adding it. It shouldn't be there, because if it was, you wouldn't have got this node from the open list in the first place.

3) Same with the check inside BackTracePath. If the same node has somehow ended up in your path twice then you have big problems that merely skipping that node isn't going to fix.

4) Why bother returning unwalkable adjacent nodes? If it can't be crossed, then it doesn't need to exist as a node at all.

5) Each node storing which nodes are adjacent is good, as long as you're calculating those in advance. However this shouldn't have anything to do with "huge lists of 100+ adjacent nodes" - that's just because you were passing data around via a shared list that you weren't clearing. You could have kept the old system and just returned that list as a temporary, instead of storing it at the Monobehaviour level. (It's usually a good idea to reduce the amount of shared state.)

6) You don't need 'if (closed_list.Contains(target_node))' to know if you've found a path - just check if my_node is target_node. There's no other way that the target node could get onto the closed list unless you just added it 2 lines up.

7) You need to return from FindPath once you find the path - don't keep running the rest of the search, as in the best case it's a big waste of CPU time, and in the worst case it's going to trash the path you already calculated. Just bail out early.

[Vampade]

Several issues I see:

1) You check to see if the open list contains 'my_node' when you start a search. That shouldn't be necessary because your open list needs to be empty. If it's not empty then you're not doing A*.

2) Similarly you shouldn't need to check if 'my_node' is on the closed list or not before adding it. It shouldn't be there, because if it was, you wouldn't have got this node from the open list in the first place.

3) Same with the check inside BackTracePath. If the same node has somehow ended up in your path twice then you have big problems that merely skipping that node isn't going to fix.

5) Each node storing which nodes are adjacent is good, as long as you're calculating those in advance. However this shouldn't have anything to do with "huge lists of 100+ adjacent nodes" - that's just because you were passing data around via a shared list that you weren't clearing. You could have kept the old system and just returned that list as a temporary, instead of storing it at the Monobehaviour level. (It's usually a good idea to reduce the amount of shared state.)

6) You don't need 'if (closed_list.Contains(target_node))' to know if you've found a path - just check if my_node is target_node. There's no other way that the target node could get onto the closed list unless you just added it 2 lines up.

Thank you very much for your feedback. I always check if a list contains or doesn't contains a node because if I didn't do that unity would add a node at each frame and there would be a lot of copies of the same nodes in the lists. Instead for the adjacent nodes list, yes it was a mistake in the logic of the code that could be solved anyway I think that storing once on each node its adjacent nodes is the best way.

4) Why bother returning unwalkable adjacent nodes? If it can't be crossed, then it doesn't need to exist as a node at all.

The player every time presses space makes a "jump" (moves towards the cursor for a certain distance quickly) and spawns a line that goes from a point where the space key was pressed to the point where he finished the jump. The purpose of the lines is to be an obstacle for the enemies and after some time they disappear, that's why the node always has to check if he's walkable.

Now I'm working on the follow path system and on the path recalculation. Thank you all for your huge help

[Kylotan]

If you're re-running the path algorithm when you already have nodes in the open and closed lists then you're doing something wrong. It would probably be best to solve that problem rather than hack around it by going through a whole unnecessary iteration of A* where you don't add nodes. Whenever you call FindPath both open and closed lists should be entirely empty.

[Vampade]

Hey guys, I passed to Aron Granberg's A* Pathfinding Project and in a couple of days I made more than I made in 3 weeks with my AI. I want to focus more on the creative part of the game and get ASAP a playable prototype, I stuck on it for too much time. I feel a bit like a coward because instead of solving the problem I just decided to solve everything with someone else's work but my system has too much flaws and it would require much more time. I'll surely retry making it when I'll be a more skilled. Anyway thank you very much guys for your great support and patience