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 error message when reimporting resources with an empty scene open #80149

Merged

Conversation

aaronfranke
Copy link
Member

When reimporting, the import dock has code that will update the resource in all scenes currently opened in the editor.
If the scene is empty (like when no scene is open), this will fail and print this error message:

ERROR: Parameter "p_object" is null.
   at: _replace_resource_in_object (editor/import_dock.cpp:606)

This PR adds a check to see if the edited scene has a root node, and if not, don't try to update it.

This PR is very minor. The only behavior difference is the lack of a useless error message being printed.

@aaronfranke aaronfranke added this to the 4.x milestone Aug 2, 2023
@aaronfranke aaronfranke requested a review from a team as a code owner August 2, 2023 05:09
@YuriSizov YuriSizov requested a review from KoBeWi August 2, 2023 12:10
@aaronfranke aaronfranke force-pushed the fix-res-reimport-empty-scene branch from 3c22c11 to ff911c3 Compare September 3, 2023 16:41
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Looks good, but I'm wondering about likely. I don't remember seeing it used in a similar code.

@aaronfranke
Copy link
Member Author

@KoBeWi likely instructs the compiler to tell the CPU to guess that this is going to be true most of the time. This is a micro-optimization and not really necessary, but I think it's slightly better.

@akien-mga
Copy link
Member

akien-mga commented Sep 4, 2023

We use unlikely in ERR_FAIL_COND checks, so if this has similar expectations for how rarely it would be untrue, likely seems OK.

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Sep 4, 2023
@akien-mga akien-mga merged commit a0d21d4 into godotengine:master Sep 4, 2023
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the fix-res-reimport-empty-scene branch September 4, 2023 07:12
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