martindevans / SupersonicSound

C# Wrapper For FMOD Studio
Other
29 stars 9 forks source link

Low Level Validity Checking #22

Closed martindevans closed 9 years ago

martindevans commented 9 years ago

Low level types can become invalid at any time, the only way to check is to check the return result of calls to see if it is FMOD_ERR_INVALID_HANDLE or FMOD_ERR_CHANNEL_STOLEN. This is problematic in SSS because these become exceptions which makes handling this case extremely ugly (all calling code must be wrapped in a try block with a handler for these exceptions).

Discussion in this thread eventually settled on a solution for this problem:

martindevans commented 9 years ago

Structs which this change will affect:

~These systems have the additional _channelstolen flag.

martindevans commented 9 years ago

I'm going to have a crack at implementing (at least partly) this on Channel right now. Just as an example of how I think this should be done.

martindevans commented 9 years ago

Implemented this for two methods in Channel (in a branch right now, see here). This was mostly writing a set of helper functions to make doing this easy, most of the remaining work will just be mechanical substitution.

martindevans commented 9 years ago

Implemented this for all of Channel and ChannelGroup, updated callback example to remove the try/catch

HakanL commented 9 years ago

:+1: I gotta check it out!

martindevans commented 9 years ago

This pattern ends up leaving some method calls rather ugly. e.g.

public void GetChannelFormat(out ChannelMask? channelMask, out int? numChannels, out SpeakerMode? sourceSpeakerMode)

If you call this you end up with three nullables, which are either all Some or all None. Once the basic conversion to this pattern is complete we could consider a further enhancement to fix this:

public ChannelFormatResult? GetChannelFormat()

This way there is only one nullable (effectively a tuple of the results).

HakanL commented 9 years ago

I like that approach with returning a ChannelFormatResult.

martindevans commented 9 years ago

Result struct work list:

~These systems have the additional _channelstolen flag.

I've implemented the new result struct system for Sound, I'll probably go back and implement it for Channel and ChannelGroup next as I guess these are the three structs you're using most. Once that's done I'd appreciate any feedback you have on the ergonomics of the new system :)

HakanL commented 9 years ago

In my application I actually only use the GetPosition on the channel and that worked really well with these changes.

martindevans commented 9 years ago

I've applied this pattern to all types except Geometry and System (LowLevel or Studio). I don't see how a handle going null for any of these is anything but an error!

Going to close this issue now