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

Add a more detailed error message when instantiating a scene with missing export properties #97071

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

RPicster
Copy link
Contributor

As mentioned in the macro of ERR_CONTINUE, it should only be used when there is no sensible error message.

In this case, I was searching for the cause of the ominous error in my project - which were impossible to find without further information.

With this new error message it is much easier as a game dev to understand the problem and fix it.

@AThousandShips AThousandShips changed the title Added a more detailed error message when instatiating a scene with missing export properties Add a more detailed error message when instantiating a scene with missing export properties Sep 16, 2024
@AThousandShips AThousandShips added this to the 4.4 milestone Sep 16, 2024
@RPicster
Copy link
Contributor Author

This must have been the fastest PR review I've ever seen @AThousandShips 🤘 👏👏👏

@@ -527,7 +527,7 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {

bool valid;
Array array = dnp.base->get(dnp.property, &valid);
ERR_CONTINUE(!valid);
ERR_CONTINUE_EDMSG(!valid, vformat("Failed to get property '%s' from node '%s'.", dnp.property, dnp.base->get_name()));
Copy link
Member

Choose a reason for hiding this comment

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

There's the same code in the Variant::DICTIONARY branch, would be good to use the same better message there too.

Copy link
Member

Choose a reason for hiding this comment

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

Also just to be clear, the EDMSG variant of the macro means the error will be shown in the editor toaster. Is that what you intended? (i.e. is this an error that the user can trigger from the editor and where it would make sense to show in the toaster and not just in the Output dock?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I open a separate PR for that?

Copy link
Member

Choose a reason for hiding this comment

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

No this is exactly the same change as done here, just handling Dictionary and not just Array, so it should be part of the same commit (you can edit it with git commit --amend and git push --force).

@akien-mga akien-mga added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 16, 2024
@akien-mga akien-mga merged commit 8a7555a into godotengine:master Sep 17, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 17, 2024
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.

4 participants