Skip to content

Add async callbacks to GLTFImportPluginContext #847

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eoineoineoin
Copy link
Member

I had a use-case when implementing an import plugin and needed to call an async function. Attempting to do this from the synchronous callbacks would either trigger exceptions due to code which should only run on the main thread or would hang Unity.

This PR extends the GLTFImportContext to provide async versions of the callbacks; for backwards-compatibility, the base implementation calls the synchronous version, so any extant implementations of the GLTFImportPluginContext should not see any API changes due to this.

For my use-case, I just needed an async OnAfterImportNode(), but saw no reason that this shouldn't be supported for each callback.

@CLAassistant
Copy link

CLAassistant commented May 1, 2025

CLA assistant check
All committers have signed the CLA.

@hybridherbst
Copy link
Collaborator

hybridherbst commented May 15, 2025

Thanks for the PR! We'll take a look.

There is already helper class to help with running async code synchronously, would it be possible for your use case to instead run your async method synchronous inside the existing OnAfterImportNode?

The helpers are here:

public static void RunSync(Func<Task> task)

@eoineoineoin
Copy link
Member Author

Thanks for looking into this; wasn't aware of that AsyncHelpers at all - looks like it does the trick and works correctly; I think that's preferable to use, rather than making the plugins async. Think we can close this PR, but wonder if there's some way we could bring attention to that util, just for developer productivity?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants