Rework event managing and mute

[piiertho]

This refactors event nature handling using UserData of FMOD::Studio::EventInstance. This refactors callbacks handling with UserData. This modifies Fmod::muteAllEvents and Fmod::unmuteAllEvents so that it set volume to 0 and get former volume back. This adds Fmod::muteMasterBus and Fmod::unmuteMasterBus helper functions.

[piiertho]

Don't know what happen with vscode indentation...

[CedNaru]

"class Fmod : public Object {" vs "class Fmod : public Object {"

I personnaly prefer the second one (even if the standard seems to be the first, so it's the one I use now) so I have nothing against it. But I think it's weird to choose this style here when the remaining code use the first style.

[CedNaru]

Fmod::Fmod() { system = nullptr; coreSystem = nullptr; listener = nullptr; Callbacks::mut = Mutex::create(); checkErrors(FMOD::Studio::System::create(&system)); checkErrors(system->getCoreSystem(&coreSystem)); }

To create a new Mutex in the node init function seems out of place since it is a global variable, not a class member. The Fmod node might be use as a singleton in Godot but it is not mandatory. Some users might create several Fmod nodes so resetting the mutex each time it is not optimal.

We can either create the mutex elsewhere ( I don't really know where) or add an "already initialized check" to the init function.

[heraldofgargos]

Regarding the Mutex - Yes, initializing it in the constructor is probably not the best place. You could move it elsewhere. Where as to? It'll require some thinking.

The module is also meant to be used as a singleton. You can make multiple instances of it but this is not ideal at all. There is a way to "force" the singleton pattern though. I'll have a look at it later.

[CedNaru]

If you really want to enforce the fact it is a singleton, you might want to create a server instead. https://docs.godotengine.org/en/3.1/development/cpp/custom_godot_servers.html

[heraldofgargos]

If you really want to enforce the fact it is a singleton, you might want to create a server instead. https://docs.godotengine.org/en/3.1/development/cpp/custom_godot_servers.html

It can be done with a module as well. I will push some updates when I have the time. This should effectively get rid of the need to have AutoLoad scripts.

[CedNaru]

Servers are still modules but with some extra features. The real change is in the register.cpp

[CedNaru]

About the Mutex. Maybe we can create and delete it in the register.cpp, it seems to be the correct place.

[heraldofgargos]

About the Mutex. Maybe we can create and delete it in the register.cpp, it seems to be the correct place.

Yep that sounds about right.

Servers are still modules but with some extra features. The real change is in the register.cpp

I already have the changes working locally. I will push them once this PR is merged.

[CedNaru]

You mean the GameObject ? Using the UserData was the whole point of this rework. That way we can easily attach data to each event without using additionnal containers. The UserData property exist for this kind of situation. Do you prefer to just store the EventInstance directly in the EventInfo Struct and store all EventInfo in a vector ?

[CedNaru]

Something crossed my mind. We no longer handle OneShot events, but we still need to update AttachedOneShot event positions. We have to control if they have been stopped to remove them from the vector. If so, we still need a way to know if an event is unmanaged or oneShot ( with a OneShot boolean or a second vector, but this rework's goal is to get rid of those containers). Is this right or did I miss something ?

[heraldofgargos]

That sounds about right. I was initially in favour of combining these into a single vector but reflecting back the reason why I had two vectors in the first place (one for attachedOneShots and another for unmanagedEvents) was because they had to be handled differently.

[heraldofgargos]

Alright. So, merging this cleanly is going to be difficult. The PR branch has many back and forth commits that will overwrite a whole bunch of stuff in master. I will be hand picking commits from this branch and committing them to master manually. I can't afford the time right now to go through all the diffs.

[piiertho]

If you want, I can rebase it on master so that merge is easier.

[heraldofgargos]

If you want, I can rebase it on master so that merge is easier.

Sure. Can you clean up the history?

Also feel free to open another PR as the current one has sort of gone off the rails at this point.

[heraldofgargos]

Resolved with https://github.com/alexfonseka/godot-fmod-integration/pull/23 Closing.