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

[AssetLib] Fix crash in Web editor. #62531

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Jun 29, 2022

Add EditorAssetLibrary::is_available which always returns false in the Web editor and use it in EditorNode for detection.

Fixes #62520.
Closes #62528

@Faless
Copy link
Collaborator Author

Faless commented Jun 29, 2022

Maybe I should add the method to AssetLibraryEditorPlugin instead? CC @akien-mga

@Faless Faless force-pushed the fix/4.x_assetlib_is_available branch from a6aaa61 to d4accf9 Compare June 29, 2022 17:51
@@ -7073,13 +7073,11 @@ EditorNode::EditorNode() {

// Asset Library can't work on Web editor for now as most assets are sourced
// directly from GitHub which does not set CORS.
#ifndef JAVASCRIPT_ENABLED
Copy link
Member

Choose a reason for hiding this comment

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

You can also remove the comment here, it's sufficient to have it in is_available.

Copy link
Member

Choose a reason for hiding this comment

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

Missed that one?

Copy link
Member

@akien-mga akien-mga Jun 29, 2022

Choose a reason for hiding this comment

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

@Faless notice me senpai :P

Doesn't matter so much though, I guess it doesn't hurt to repeat it here.
Just not very consistent in wording with what the warning would then print.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh, yeah, I had missed this comment, sorry :/

@akien-mga
Copy link
Member

Maybe I should add the method to AssetLibraryEditorPlugin instead?

Good question. I never really understood the relationship between SomeEditor and SomeEditorPlugin. But yeah IIRC the former is enclosed in the latter so it might be better to have it on the plugin one.

@akien-mga
Copy link
Member

You should also update ProjectManager, see 42b4849

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 29, 2022
@Faless Faless force-pushed the fix/4.x_assetlib_is_available branch from d4accf9 to bc8be60 Compare June 29, 2022 20:53
Add EditorAssetLibrary::is_available which always returns false in the
Web editor and use it in EditorNode for detection.
@Faless Faless force-pushed the fix/4.x_assetlib_is_available branch from bc8be60 to 0e504e4 Compare June 29, 2022 23:26
@akien-mga akien-mga merged commit fd3970f into godotengine:master Jun 30, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

akien-mga commented Jun 30, 2022

Cherry-picked for 3.5.

Edit: For some reason this commit got lost? Eventually cherry-picked properly on 2022-07-08 with ad5fdcc.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 30, 2022
@Faless Faless deleted the fix/4.x_assetlib_is_available branch June 30, 2022 09:39
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.

Godot Web Editor 3.5 RC5 fully broken (since 3.5 RC 3)
2 participants