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

[Main] Improve error messages with vformat #99095

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AThousandShips
Copy link
Member

@@ -315,13 +315,13 @@ void Performance::set_navigation_process_time(double p_pt) {
}

void Performance::add_custom_monitor(const StringName &p_id, const Callable &p_callable, const Vector<Variant> &p_args) {
ERR_FAIL_COND_MSG(has_custom_monitor(p_id), "Custom monitor with id '" + String(p_id) + "' already exists.");
ERR_FAIL_COND_MSG(has_custom_monitor(p_id), vformat("Custom monitor with id '%s' already exists.", String(p_id)));
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly casting to String is not needed for vformat() I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so too, as with the other batch I kept all the other details as they were, this batch is relatively small so will change them here and check, wanted to avoid any potential regression from this as some potential errors might not show up unless the error is actually triggered

@AThousandShips AThousandShips modified the milestones: 4.4, 4.x Jan 28, 2025
@AThousandShips AThousandShips force-pushed the improve_err_prints_main branch from d57bb4e to f2f2d42 Compare February 8, 2025 18:40
@AThousandShips AThousandShips force-pushed the improve_err_prints_main branch from f2f2d42 to 3edd96a Compare March 23, 2025 15:29
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.

2 participants