Attention | Topic was automatically imported from the old Question2Answer platform. | |
Asked By | Phyron |
Hi!
I have some character nodes (area2d), which use the get_simple_path () function on a 200x200 tilemap.
I had problems, each node took time to calculate the path, and the main thread was blocking a few milliseconds, so I have tried using threads.
Now, if I put only one character, it works correctly, with 2 or 3, it takes a while to throw the error. And if for example I put 5 or more, throw error instantaneous. And the game breaks and closes.
The error message I get is:
E 0: 00: 08.518 get: FATAL: Index p_index = -1 is out of bounds
(size () = 4).
C ++ source ./core/cowdata.h:152 @ get ()
Character node code:
var speed : = 400.0
var path : = []
var thread = Thread.new()
func _ready() -> void:
randomize()
newPath()
func _unhandled_input(event: InputEvent) -> void:
if not event is InputEventMouseButton:
return
if event.button_index != BUTTON_LEFT or not event.pressed:
return
func _process(delta: float) -> void:
if path.size() != 0:
var move_distance : = speed * delta
move_along_path(move_distance)
else:
newPath()
func move_along_path(distance : float) -> void:
var last_point : = global_position
for index in range(path.size()):
var distance_to_next = last_point.distance_to(path[0])
if distance <= distance_to_next and distance >= 0.0:
global_position = last_point.linear_interpolate(path[0], distance / distance_to_next)
break
elif distance < 0.0:
global_position = path[0]
newPath()
break
distance -= distance_to_next
last_point = path[0]
path.remove(0)
func newPath():
if (thread.is_active()):
# Already working
return
print("START THREAD!")
thread.start(self, "_on_newPath", get_instance_id())
func _on_newPath(id):
print("THREAD FUNC!")
print(id)
var newPosition = Vector2(randi()%200, randi()%200)
var tile_center_pos = get_parent().get_parent().get_child(0).get_child(0).map_to_world(newPosition) #map in tree
var newPath = get_parent().get_parent().get_child(0).get_simple_path(global_position,tile_center_pos) #navigation2D node in tree
#get_parent().get_child(2).points = newPath
#newPath.remove(0)
call_deferred("_on_newPath_done")
return newPath
func _on_newPath_done():
# Wait for the thread to complete, get the returned value
var minionPath = thread.wait_to_finish()
path = minionPath
The code without thread functions worked fine, with any number of nodes. I thing that the problem is with threads, but I cant understand the error message.
I don’t know threading very well on Godot, but I think there may be an error due to access on shared resources. Maybe two or more threads try to access the same node. Read should not be a problem but maybe access enables also writing and in that case it may be a problem.
Try using mutex.lock()
and mutex.unlock()
around your getNodes
.
gioele | 2021-01-19 18:51
Thanks, but have tried using mutex but it remains the same.
I have read in another post, that we should" NOT access the tree in a thread". But I don’t understand how to calculate the Minion’s path without accessing the tilemap and navigation nodes …
I have cleaned up the code to pick up the map node before, but it didn’t work either.
First get the nodes:
onready var Map = get_parent().get_parent().get_child(0).get_child(0)
onready var Nav = get_parent().get_parent().get_child(0)
And in thread:
var tile_center_pos = Map.map_to_world(newPosition)
var newPath = Nav.get_simple_path(global_position,tile_center_pos)
Phyron | 2021-01-19 20:06
I have another question, this could also be useful.
You follow a path in _process
function. You check length to avoid following an empty path and in that case you create a new one using threads…
But do you stop processing with set_process(false)
?
Otherwise each character will start multiple threads or have unexpected behavior when path is calculated as processing will go on.
Instead try pausing process and restart once path is ready. Then you may not need threads at all.
gioele | 2021-01-19 20:24
I have tested what you say, stopping the process before calling the function that creates the path, and activating it after creating it.
Still, if I keep using threads, I get the same error, and if I don’t use them, it hangs like before, a few milliseconds every time one of the minions recalculates the path. Thanks again!
Phyron | 2021-01-19 20:44
it seems from the error you are trying to access element -1 of an array of size 4.
does the error point toward a specific line of the code?
if not, can you comment out parts of the code to pinpoint which function generate the error in particular?
Andrea | 2021-01-19 21:05
Thx Andrea,
Ok, I have tried to detect where it fails in two ways:
Debugging line by line: The game does not fail. Works fine! But it’s not a fun way to play xD
I think it will be because when going at normal speed, the threads cope with each other in some way, but I don’t know how to solve it, or if is the real problem.
Adding many logs: Normaly stop in _on_newPath function, but some times in newPath function. Never in the same lines.
Phyron | 2021-01-19 21:34
yeah, it definitely look like a problem due to thread reding/writing onto the same variable.
you said you used mutex, are you sure you used them correctly?
although it is strange, cause your thread is not writing but only reading, unless get_simple_path
and map_to_world
are somehow also writing data.
does it change if you modify them to be normal variable? like
var tile_center_pos = random_vector_variable
var newPath = random_vector2_array
or
var tile_center_pos = get_node(whatevernode).random_vector_variable
var newPath = get_node(whatevernode).random_vector2_array
Andrea | 2021-01-20 11:21
I think the problem with concurrent access is also due to the fact that since it is the scene tree every other node in code will access the same (and could read/write) so it would be necessary to mutex every possible access to it. (that I think it is impossible due to everything running in background of the engine).
So maybe the solution could be creating a local isolated representation of the map to compute paths. If the map is static there should be no problem.
gioele | 2021-01-20 11:44
Hi Andrea,
I had never used mutex, but following the official documentation I think I do it well.
At the beginning I create the mutex:
var mutex = Mutex.new()
And then I apply it here (I have also tried to apply it in other sites):
mutex.lock()
thread.start(self, "_on_newPath", get_instance_id())
mutex.unlock()
I have also set the unlock at the end of the thread
func _on_newPath_done():
# Wait for the thread to complete, get the returned value
var minionPath = thread.wait_to_finish()
path = minionPath
mutex.unlock()
It is right?
I have tried what you say, and it works!
var tile_center_pos = Map.map_to_world(newPosition)
print("map!")
var newPath = [Vector2(100,100),Vector2(550,550),Vector2(150,150),Vector2(170,170)]
So the problem is in get_simple_path. I have tried to surround only this line with mutex but it does not work either
Phyron | 2021-01-20 11:51
Hi gioele!
The project for now is very simple, only have some nodes, nav2d, tilemap, camera, and minions.
The map is generated procedurally, and minions position too. I don’t have other nodes that conect with map or Navigation2d…
I can put project in drive if you want for see the problem. It’s very small.
Phyron | 2021-01-20 11:55
what gioele said makes completely sense, and the fact that it works without get_simple_path
is a sign of confirmation.
for the mutex part, i dont think that’s the right way to do it (if i understood correctly, i’m not an expert either) you should lock it immediatly before accessing the navigation map and unlock it after you get the path.
something like
mutex.lock()
var tile_center_pos = tile_map_node.map_to_world(newPosition) #map in tree
var newPath = navigation_node.get_simple_path(global_position,tile_center_pos)
mutex.unlock()
(i think this is what gioele intended in his first comment) but it could also not be enough because the main thread can still access the nodes, making a mess. It’s a start though!
Andrea | 2021-01-20 12:30
Hey Andrea, yes, I was telling you at the end of my last comment, I already tried it with mutex on those lines, and nothing …
So to make a game of this style, the solution would be to save the map information in an array or list for example, and update it every time the map is updated, and work with it? I find it strange that godot can’t add multiple characters that move around the map to create an RTS-style game…
I also don’t understand why mutex doesn’t work in this case.
On the other hand, I could try instead of creating a Minion node for each instance, and each with the thread’s code, create a script only for thread, and extend minion node with it. In this case, all of him enter in the same thread function, and maybe mutex works. What do you think?
Phyron | 2021-01-20 12:44
i think this is not a problem with Godot, but a problem with multiple threads in general.
you can still share the simplified project, i’m really curious to see if we can find a way to make it work.
however i think you should also look for a workaround (something like what you mentioned), because having one thread for each minion is not super efficient: what happens if you have hundreds of minion on the scene? your pc will not be able to handle that amount of threads
Andrea | 2021-01-20 13:00
I understand that it is not good to create threads for 100 nodes, but I don’t know how to do it any other way, without stopping the main process.
I leave you a link to drive to download the project! It is not minimized, because it is already very small.
You will see that the map node at startup generates the map
func makeIsle():
and at the end of this funcrion add the minions:
for n in range (0,15):
The Minions is a Human.tscn node, with Character.gd script.
The game does nothing else for now
I uploaded it to Github:
GitHub - KenderM/Godot-game
Lot of thanks!
Phyron | 2021-01-20 13:19