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 diagnostic dialog for missing variables #6573

Merged
merged 15 commits into from
May 20, 2024
Merged

Add a diagnostic dialog for missing variables #6573

merged 15 commits into from
May 20, 2024

Conversation

D8H
Copy link
Collaborator

@D8H D8H commented May 16, 2024

Changes

  • Show a diagnostic report when a preview is launched and there are missing:
    • scene variables
    • object variables
    • behaviors

Comment on lines +250 to +253
RaiseError(gd::ExpressionParserError::ErrorType::MissingBehavior,
_("This behavior is not attached to this object."),
function.behaviorNameLocation,
/*isFatal=*/false, function.behaviorName, function.objectName);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, we have the behavior name but not the expected type. It would be hard to build an understandable message so it's not forwarded to the report.
I think it's fine because behaviors instructions will likely be used too.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok!

return gd::make_unique<ExpressionParserDiagnostic>();
return std::unique_ptr<ExpressionParserError>(nullptr);
Copy link
Collaborator Author

@D8H D8H May 20, 2024

Choose a reason for hiding this comment

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

This change is not just a refactoring. It was wrongly replacing the null pointer with a default constructed instance.
When removing the base class with IsError() { return false }, it returned true which causes bugs.
IsError() no longer exists, only null is checked.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the fix 👍 Much better

@D8H D8H marked this pull request as ready for review May 20, 2024 08:55
@D8H D8H merged commit 8d6e0b3 into unify-variable May 20, 2024
4 of 6 checks passed
@D8H D8H deleted the diagnostic branch May 20, 2024 12:11
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.

None yet

2 participants