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

GDExtension: Fixed error on loading extensions #83734

Merged

Conversation

MarioLiebisch
Copy link
Contributor

Previously, before loading an extension, the editor just tried to retrieve the extension by path to test if it's been loaded already.

While this is handled gracefully, it ignored an error thrown inside GDExtensionManager::get_extension(), that would essentially still report a not yet loaded extension to the engine's log:

ERROR: Condition "!E" is true. Returning: Ref<GDExtension>()
   at: GDExtensionManager::get_extension (core\extension\gdextension_manager.cpp:165)

This change actively checks whether the extension path is known and only then proceeds to actually return the already loaded extension or loads and returns the new one otherwise.

Previously, before loading an extension, the editor just tried to
retrieve the extension by path to test if it's been loaded already.

While this is handled gracefully, it ignored an error thrown inside
`GDExtensionManager::get_extension()`, that would essentially still
report a not yet loaded extension to the engine's log:

```
ERROR: Condition "!E" is true. Returning: Ref<GDExtension>()
   at: GDExtensionManager::get_extension (core\extension\gdextension_manager.cpp:165)
```

This change actively checks whether the extension path is known and only
then proceeds to actually return the already loaded extension or loads
and returns the new one otherwise.
@MarioLiebisch MarioLiebisch requested a review from a team as a code owner October 21, 2023 15:02
@akien-mga akien-mga added this to the 4.2 milestone Oct 21, 2023
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Ah, good catch, I didn't notice that the current code generates some error spam.

I didn't test it, but the code here looks good to me. Thanks! :-)

@akien-mga akien-mga merged commit 542f6e1 into godotengine:master Oct 22, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

3 participants