Add TryGetExtension methods to AL & ALContext

[DmitryGolubenkov]

Summary of the PR

This PR adds TryGetExtension methods to AL and ALContext classes in Silk.NET.OpenAL

Related issues, Discord discussions, or proposals

#574

Further Comments

TryGetExtension method in AL is nearly the same as in OpenGL bindings. It was trivial to copy-paste and then compare to GetExtension to verify. The XML documentation was also taken from GL and modified.

However, GetExtension method in ALContext is different and also it doesn't use ExtensionAttribute. Also it requires a pointer to a Device to operate. So I am not sure if "copy-paste" approach would work. Should I convert ALContext.GetExtension to TryGetExtension?
Or maybe I am wrong, and it will be enough to copy existing method and import Silk.NET.Core.Attributes namespace?
Just want to be sure.

[Perksey]

The ALContext TryGetExtension method I’d expect to mirror the ones in Vk and XR i.e. it’s basically the same method but with an extra parameter. All extension-related methods bar IsExtensionPresent itself should use ExtensionAttribute. Hope this helps!

[DmitryGolubenkov]

Thanks, that helped a bit. I did some research about the codebase and also looked into documentation to better understand what is going on. And now I agree that TryGetExtension would nearly mirror the methods in Vk and XR.

Anyway, I noticed that GetExtension in ALContext requires a pointer to a device, but the parameter is not used. At least it is not obvious. Is there any reason for that?

You mentioned an extra parameter. There is no separate Instance in OpenAL wrappers, and the Device struct is empty and is used as a pointer. So I don't understand what that parameter is.

I'll make a commit so that you could check if the method is correct. Then I'll add XML documentation

[Perksey]

As requested, an initial lookover (not a full review)

Some comments:

• If you wanted to add [Obsolete("This method has been deprecated and will be removed in Silk.NET 3.0. Please use TryGetExtension instead.")] to GetExtension on both classes, that'd be much appreciated.
• If you'd like to add [Obsolete("This class is deprecated and will be removed in Silk.NET 3.0. Please use the TryGetExtension method on ALContext.")] to ALExtensionLoader again would be much appreciated

[Perksey]

This method should have a Device* parameter which you feed in as the first parameter of IsExtensionPresent. It's awful we weren't using that in GetExtension - we really should have been!

[DmitryGolubenkov]

Should XML documentation contain a remark This function doesn't check that the extension is enabled - you will get an error later on if you attempt to call an extension function from an extension that isn't loaded.? If I understand correctly, ALContext.TryGetExtension also doesn't check if it is enabled.

If yes, I think I should update the AL.TryGetExtension XML too.

[Perksey]

OpenAL doesn't have a concept of enabling extensions like Vulkan does - with Vulkan if you try to get the address to a function in an extension which is not enabled, you will get a symbol loading exception whereas OpenAL allows it and automatically grants access to all extensions available on that implementation. There is alEnable which enables the extension's functionality in some cases, but that's different to enabling the extension itself. So I would say leave that out.

[Perksey]

Thanks so much! Once again, your code is superb it's just missing a few stylistic choices we make in our codebase which could do with being implemented. All one click changes so no need to open the IDE :)