Game broken using threads

Question

Hi!

I have some character nodes (area2d), which use the getsimplepath () 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.

Comments

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.

---

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!

---

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?

---

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

---

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:
https://github.com/KenderM/Godot-game

Lot of thanks!

Answer 1

i think i understood what you were doing wrong with the mutex: you were (probably) creating a mutex for each character, instead of creating a single shared one.
if each character has his own mutex variable, when reaching the mutex.lock() line they will always proceed (no other character will ever lock their own mutex) genereting the cuncurrent access error.

what you want to do is creating a common one in a parent node: each character need to look at this mutex and lock/unlock before proceeding with the map access.
For example, in the Demo main node

var mutex
func _ready():
        mutex=Mutex.new()

and in the character

func _on_newPath(id):
    var newPosition = Vector2(randi()%200, randi()%200)
    get_node("/root/Demo").mutex.lock()
    var tile_center_pos = Map.map_to_world(newPosition)
    var newPath = Nav.get_simple_path(global_position,tile_center_pos) #navigation2D node in tree
    get_node("/root/Demo").mutex.unlock()
    call_deferred("_on_newPath_done")
    return newPath

i tried up to 200 character and it works, although the majority will remain still as my pc can manage only 12 threads at a time (considered the average path lenght and the time to calculate it, it is equivalent to roughly 20 character moving while the other were waiting for a free thread to start calculating their path)

Comments

i just realize that this code structure is more similar to a single thread that calculate the path of each character one after the other (there are multiple threads, but each one have to wait for the path calculation of the previous one).

to fully use each thread at everytime without queueing them, you probably have to save multiple representation of the map as suggested previously, and let each thread access only one of them (eg: 12 threads means 12 map representation)

---

It works!
Thank you very much for getting rid of this headache! :)

I can't process more than 12 +- either. But as in the future, when they arrive at the destination, they will be quiet for a while (good time for get the new path). Or I can even recalculate the next path while still walking if necessary. With a few tricks like that, I'm sure I can get a few more minions to work :)

Thanks again! I owe you a beer.