Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OpenAL loading of extension function that are not present. #1283

Merged
merged 2 commits into from
May 28, 2021

Conversation

NogginBops
Copy link
Member

@NogginBops NogginBops commented May 19, 2021

Purpose of this PR

Fixes an issue where checking if an OpenAL extension is present while the device and context doesn't have that extension present throws an error because it tries to load all of the extensions methods and fails.

Calling the extensions functions without it being present now gives a helpful error message.

Testing status

Manually tested checking for a made up non-existent AL extension which didn't crash.

Comments

This is not great for AOT style use cases as it generates IL at runtime. (we don't have any AOT use cases atm)
But this could be put behind if checks or static ifs if needed.

@NogginBops NogginBops added the 4.x label May 22, 2021
@NogginBops NogginBops added this to the 4.6.4 milestone May 22, 2021
Copy link
Member

@frederikja163 frederikja163 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure if i understand whats going on here. But it looks good to go

@NogginBops
Copy link
Member Author

This is doing the same type of dynamic code generation as found in #1179 . Adding that reference for completeness.

@NogginBops
Copy link
Member Author

Merging.

@NogginBops NogginBops merged commit c938983 into opentk:master May 28, 2021
@NogginBops NogginBops deleted the openal-extension-fix branch May 28, 2021 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants