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 arg count checks in SceneDebugger #79655

Conversation

kleonc
Copy link
Member

@kleonc kleonc commented Jul 19, 2023

In #58929 args passed to SceneDebugger::parse_message were changed to no longer be fixed size. However:

  • For p_msg == "live_node_call" case the error was removed, instead of modified to handle variadic arg count.
  • For p_msg == "live_res_call" case the error was unchanged, instead of modified to handle variadic arg count.

This PR fixes error checking for both cases.

Fixes #79616.

@kleonc kleonc added bug topic:editor cherrypick:4.0 cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 19, 2023
@kleonc kleonc added this to the 4.2 milestone Jul 19, 2023
@kleonc kleonc requested a review from a team as a code owner July 19, 2023 11:19
@kleonc kleonc force-pushed the scene-dubugger-remove-outdated-error-check branch from d30d13f to 3273b81 Compare July 19, 2023 11:24
@kleonc kleonc changed the title Remove outdated arg count check in SceneDebugger Fix outdated arg count check in SceneDebugger Jul 19, 2023
@kleonc
Copy link
Member Author

kleonc commented Jul 19, 2023

LocalVector<Variant> args;
LocalVector<Variant *> argptrs;
args.resize(p_args.size() - 2);
argptrs.resize(args.size());
for (uint32_t i = 0; i < args.size(); i++) {
args[i] = p_args[i + 2];
argptrs[i] = &args[i];
}
live_editor->_res_call_func(p_args[0], p_args[1], (const Variant **)argptrs.ptr(), argptrs.size());

Should for p_args.size() == 2 the call be changed to live_editor->_res_call_func(p_args[0], p_args[1], nullptr, 0) or is it fine as is?

@Faless
Copy link
Collaborator

Faless commented Jul 19, 2023

Should for p_args.size() == 2 the call be changed to live_editor->_res_call_func(p_args[0], p_args[1], nullptr, 0) or is it fine as is?

Yes, I think it should be something like:

live_editor->_res_call_func(p_args[0], p_args[1], argptrs.size() ? (const Variant **)argptrs.ptr() : nullptr, argptrs.size());

@kleonc kleonc force-pushed the scene-dubugger-remove-outdated-error-check branch from 3273b81 to 95809e5 Compare July 19, 2023 16:13
@kleonc kleonc changed the title Fix outdated arg count check in SceneDebugger Fix arg count checks in SceneDebugger Jul 19, 2023
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for looking into this 🏆

@YuriSizov YuriSizov merged commit 4770e87 into godotengine:master Jul 21, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@kleonc kleonc deleted the scene-dubugger-remove-outdated-error-check branch July 21, 2023 17:05
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Aug 31, 2023
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.

Error with Adjustments and Color Correction
3 participants