alessandrofama / wwise-godot-integration

Wwise Integration for the Godot Engine
Other
286 stars 14 forks source link

Using Callbacks in conjunction with a Stop Fade Time causes a crash when exiting the tree #59

Closed Maaack closed 1 year ago

Maaack commented 2 years ago

This crashes the Godot game (not the tool) with no errors in the debugger.

To reproduce, I have an AkEvent with stop_on=2 # Tree Exit stop_fade_time=5000 # 5 seconds use_callback=true callback_flag=256 # Music Sync Beat.

I dug into the code and found when ak_event.gd enters the tree, var cookie will get the reference to callback_emitter of the current AkEvent.

Calling post_event() then calls playing_id = Wwise.post_event_id_callback(event.get("Id"), callback_flag, self, cookie) and we have music that triggers the MUSIC_SYNC_BEAT callback every couple seconds.

If I then call get_tree().change_scene("...") from a scene with this setup, and a beat comes in when the old scene is gone and the music is still fading out, it crashes. I suspect the callback still triggers in Wwise, and attempts to make a function call on cookie, that doesn't exist anymore in Godot.

Hope that is helpful. Let me know if there is any more information I can provide. Our current workaround is to set the stop_fade_time=0, but this is not ideal.

Maaack commented 2 years ago

I've never written a gdnative integration, but I was wondering if we could check the callbacks for a subscriptionId that matches with the one we are trying to unsubscribe. https://github.com/alessandrofama/wwise-godot-integration/blob/main/waapiclient-gdnative/src/waapiclient_gdnative.cpp#L138

alessandrofama commented 2 years ago

Thank you for the reporting the issue and sorry for the crash. I think you are probably right and the cookie doesn't exist anymore. Earliest I can take a look at the issue is Wednesday next week.

alessandrofama commented 2 years ago

We can use FuncRef::is_valid in the callback to check whether the cookie object still exists and return early otherwise. This will prevent the crash. I have taken the liberty of printing an error message when this happens. If it's too much I can remove or tone it down in a future commit.

Updated libs to replace for you to try: win mac linux ios

Android CI seems broken right now 🤨

link to changes: https://github.com/alessandrofama/wwise-godot-integration/commit/9821b90dc44a41be4111ae378996b9490ba1389c

Maaack commented 2 years ago

Yeah, that looks like a more straightforward change. Thank you. I will give these libs a try and report back.

Maaack commented 2 years ago

I tested the linux libs with Godot 3.5.0. I still got it to crash, but with this stack trace.

 handle_crash: Program crashed with signal 11
 Engine version: Godot Engine v3.5.stable.official (991bb6ac74ac8c09d7683041b50a8ced3a2defb1)
 Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
 [1] /lib/x86_64-linux-gnu/libc.so.6(+0x43090) [0x7f591c3cd090] (??:0)
 [2] /myhomedirectory/Godot_v3.5-stable_x11.64() [0x2dc25f4] (??:0)
 [3] /myhomedirectory/Godot_v3.5-stable_x11.64() [0xbdc060] (??:0)
 [4] godot::FuncRef::is_valid() const (??:0)
 [5] godot::Wwise::eventCallback(AkCallbackType, AkCallbackInfo*) (??:0)
 [6] CAkPlayingMgr::NotifyMusic(unsigned int, AkCallbackType, AkSegmentInfo const&) (??:0)
 [7] CAkScheduledItem::NotifyMusicCallbacks(int, unsigned int, unsigned int, unsigned int, float) const (??:0)
 [8] CAkScheduleWindow::NotifyMusicCallbacks(long, unsigned int, unsigned int, unsigned int, float) const (??:0)
 [9] CAkMatrixSequencer::ProcessMusicNotifications(long, unsigned int) (??:0)
 [10] CAkMatrixSequencer::Process(long, unsigned int) (??:0)
 [11] CAkMatrixAwareCtx::ProcessEpilogue(long, unsigned int) (??:0)
 [12] CAkMusicSwitchCtx::Process(long, unsigned int, AkCutoffInfo&) (??:0)
 [13] CAkMatrixSequencer::Execute(unsigned int) (??:0)
 [14] CAkMusicRenderer::ProcessNextFrame(AK::IAkGlobalPluginContext*, AkGlobalCallbackLocation, void*) (??:0)
 [15] _CallGlobalExtensions(AkGlobalCallbackLocation) (??:0)
 [16] CAkAudioMgr::Perform(bool, bool) (??:0)
 [17] CAkAudioThread::EventMgrThreadFunc(void*) (??:0)
 [18] /lib/x86_64-linux-gnu/libpthread.so.0(+0x8609) [0x7f591c6d9609] (??:0)
alessandrofama commented 2 years ago

About 1/10 of the time it crashes on me too, weird. I guess we can`t call anything on the FuncRef because it might be released when GDNative does its interop. I'm trying to use godot_is_instance_valid now to check if the underlying object is still valid and haven't had a crash in a while. Maybe it is worth a try?

Linux libs

Maaack commented 2 years ago

I tried downloading, unzipping, and testing this library twice. Both attempts it crashed as soon as I tried to call Wwise. The editor also reports a number of errors. image

/addons/wwise/bin/linux/debug/libWwiseGDNative.so: undefined symbol: godot_is_instance_valid
 modules/gdnative/gdnative.cpp:510 - No valid library handle, can't get symbol from GDNative object
 modules/gdnative/nativescript/nativescript.cpp:1503 - No nativescript_init in "res://addons/wwise/bin/linux/debug/libWwiseGDNative.so" found
 Script does not inherit from Node: res://addons/wwise/bin/wwise-gdnative.gdns.
 modules/gdnative/gdnative.cpp:417 - No valid library handle, can't terminate GDNative object
 editor/editor_autoload_settings.cpp:496 - Condition "!info->node" is true. Continuing.
  does not have a library for the current platform.
 Script does not inherit from Node: res://addons/wwise/bin/waapi-gdnative.gdns.
 editor/editor_autoload_settings.cpp:496 - Condition "!info->node" is true. Continuing.

Switching back to the previous library and reloading the project works as expected.

alessandrofama commented 2 years ago

If you please could give this another try: https://github.com/alessandrofama/wwise-godot-integration/suites/7805292542/artifacts/329361592

otherwise I will check on a linux machine myself^^

Maaack commented 2 years ago

Thanks for the updated lib. I experienced no crashes with this build. The error reporting is pretty noisy, though. image

alessandrofama commented 1 year ago

I removed the noisy reporting in https://github.com/alessandrofama/wwise-godot-integration/commit/ac7e734d171f6a024223b81bd0656b30273c2fa7. Will be available in the release or you can grab the updated libs from CI.