blitz-research / monkey

Blitz Research Monkey Source
225 stars 59 forks source link

Mojo.Audio: Add GetChannelVolume(channel) and GetMusicVolume() #76

Open GWRon opened 9 years ago

GWRon commented 9 years ago

I do not know about that "If MOJO_VERSION_X ... Import mojox.audio", so I am unsure what is done there (am only using the free monkey-version).

As there is no "MaxChannels"-value stored in the base audio.monkey, I checked the available channels in GLFW and HTML5 and both had 33 channels (32 sfx and 1 music). My implementation just hooks into the volume-setters and allows for easier retrieval of current volumes.

Why? This is needed for situations in which multiple objects adjusts volumes without knowing the other manipulators. It is similar to GetAlpha() and SetAlpha().

swoolcock commented 9 years ago

I fully understand that one could circumvent/bypass the caching. But this is true for modules too (eg code in monkey).

Sure, but this is the official API, not a 3rd party module. This is also why I'm pushing for better encapsulation and I forked a feature branch for a "protected" keyword. https://github.com/blitz-research/monkey/pull/75

Following your suggestion is not possible without having access to all targets. And what happens to custom user targets?

Then it becomes something that custom user targets need to implement. I don't see any reason to use volume "caching" unless the target doesn't support a native "GetVolume".

Let's take a look at the HTML5 native implementation of SetVolume:

gxtkAudio.prototype.SetVolume=function( channel,volume ){
    var chan=this.channels[channel];

    chan.volume=volume;

    chan.waGain.gain.value=volume;
}

The correct way to create GetVolume would be to add it to the externed class definition, then do something like this:

gxtkAudio.prototype.GetVolume=function( channel ){
    var chan=this.channels[channel];
    return chan.volume=volume;
}
GWRon commented 9 years ago

I have seen that in html5... but for glfw there seems not to be a similar functionality...which made me write the caching thing.

Also this adds the hassle to know each supported target language (syntax etc.)...instead of using the abstracted monkey language.

To remind again: this pull request is a suggestion a "make aware of something missing".

If you have access to other targets...you could check if all platforms provide a kind of getter. If not i assume your suggestion is to have that cache-approach done for each target natively (which is out of my skill horizont...i even do jot know what languages are used in all official targets).