search

tzachshabtay / MonoAGS

AGS (Adventure Game Studio) reimagined in Mono
https://tzachshabtay.github.io/MonoAGS/
Artistic License 2.0
27 stars 8 forks source link

Implemented Duration property for IAudioClip and ISound #249

Closed ghost closed 6 years ago

ghost commented 6 years ago

Added IAudioClip.Duration and ISound.Duration properties, telling total duration of the sound in seconds.

Notes:

  1. In current implementation duration is calculated from the ISoundData when ALAudioClip is created. This calculation will be only valid if the sample rate is known at that time. AFAIK some sound files may not have such value in the header, which leaves only an option to parse whole file. But since that is a rare case, hopefully we won't need to support this too soon.
  2. Duration property is currently added to both ISound and IAudioClip directly. Then, I realized they already share ISoundProperties, but it seem to refer to sound modifications, so I am in doubts. Should I move it to ISoundProperties instead?
tzachshabtay commented 6 years ago

Regarding whether to put the duration in ISoundProperties, it might make sense, but not a big deal. If you do put it there, then also change the header doc in ISoundProperties.

Also can you add it to the ags cheat sheet doc? https://github.com/tzachshabtay/MonoAGS/blob/master/Docs/articles/ags-cheat-sheet.md It's the equivalent to Channel -> LengthMs

Also check out the CodeFactor single issue (not critical though).

ghost commented 6 years ago

Done.

ghost commented 6 years ago

I think it was a mistake to add this property to ISoundProperties. ISoundProperties is used as an argument to AudioClip.Play, which means its intention is sound setup, rather than to tell information about it. If someone implements a class for the sole purpose of passing into Play function, they will now have to make it tell "duration".

tzachshabtay commented 6 years ago

Ah, right, I agree. Do you want to submit a fix for this?

ghost commented 6 years ago

Well, I don't know what's better solution. New interface?

tzachshabtay commented 6 years ago

Seems like overkill to add an interface for a single property, I think what you did at the beginning (duplicate the property for ISound and IAudioClip) is fine for now.