godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.56k stars 21.08k forks source link

if AudioStreamPlayer2D is instanced from thread, sometimes doesn't connect to AudioServer #77518

Open VanDeiMin opened 1 year ago

VanDeiMin commented 1 year ago

Godot version

3.5.2

System information

windows 10, intel core i7 mobile, gles2, nvidia quadro m1000m

Issue description

AudioStreamPlayer2D is a part of scene (area2D, collisionShape2D, AnimatedSprite) which is instanced from thread. From time to time it does not connect itself to AudioServer from there, which causes spamming error at queue_free():

 ERROR: Disconnecting nonexistent signal 'bus_layout_changed', slot: 714117:_bus_layout_changed.
at: _disconnect (core/object.cpp:1544) - Condition "!s->slot_map.has(target)" is true.

simple attempt to fix it by connecting it from ready() doesn't work

var _c0: int = AudioServer.connect("bus_layout_changed", $AudioStreamPlayer2D, "_bus_layout_changed") ;

Steps to reproduce

func make_star()-> void:
    stars: Array = [];
    if (thread.is_alive() == false):
        thread = Thread.new();
        omd_idx = SWARM;
        var _st: int = thread.start(self, "instance_star", stars); 
    return;

instancing is called only by thread and is wrapped by mutex:

func instance_star( stars: Array)-> Array:

    for i in range(0, 200):
    mutex.lock();
    var star_inst: Area2D = star.instance();
    mutex.unlock();
    stars.push_back(star_inst);
    continue;

return stars;

then is returned to main thread:

func _process(_delta:float)-> void: 
    if (omd_idx != -1 and thread.is_active() == true and thread.is_alive() == false):
    swarm = thread.wait_to_finish();

    for i in range(0, swarm.size()):     
        call_deferred("add_child", swarm[i]);
        continue;
    call_deferred("clear_swarm");
    omd_idx = -1;

    return;

Minimal reproduction project

"N/A"

AThousandShips commented 1 year ago

The signal is connected in the constructor, this seems incorrect, though the underlying issue is that signal connections are not thread safe

VanDeiMin commented 1 year ago

Of course it is incorect; I tried desperatly to do something to patch ad hoc with my very limited knowledge

AThousandShips commented 1 year ago

No, I mean in AudioStreamPlayer

Also this is a duplicate of #34752 I'd say

VanDeiMin commented 1 year ago

It seems to. Is it mean that instancing AudioStreamPlayer from thread is never good idea and I need to find another way to atach sound to my objects?_

AThousandShips commented 1 year ago

No, it's that there's an issue with the thread safety of signals, and unfortunately this particular class connects them on construction, not on entering the tree, like is done most of the time

You need, until that is fixed, to find another way to create the player that's not on a thread

VanDeiMin commented 1 year ago

AThousandShips Thank You so much for Your time and advices!!!!

I don't know cpp but I looked at source code and found that AudioStreamPlayer connects "bus_layout_changed" in the constructor. I looked for solution and naively thought if I can detect lack of connection on ready I could fix it ad hoc by simple using connect. Now I know it will not work. Searching for another solution;

AThousandShips commented 1 year ago

No problem! Good luck!

VanDeiMin commented 1 year ago

One more thing. Can You answer me ( or tell me where to look for it) : if AudioServer is an Object why I can’t connect it normal way? It is somehow protected because of special character of singleton? Or connecting AudioStream is somewhat special for it? My naïve attempt to connect throw “OK” error but but it doesn’t changed slot_map on Server.

AThousandShips commented 1 year ago

I couldn't tell, are you sure it wasn't connected in that case and you connected it twice?

VanDeiMin commented 1 year ago

No. I checked if my AudioStream has connection to Audio Server before . ( to be sure I’ve done it without it and had standard error) then tried simply to connect to Server’s function. Anyway Your previous answer is clear to me. _bus_layout_change is private function and seems to be not exposed to gdscript. So connect function executed with “OK” but AudioServer doesn’t do anything and didn’t put “target” to slot_map. I was curious why my naïve approach doesn’t work and You gave me an answer. Thank you and Good Luck