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

[3.x] Add autocompletion for AnimatedSprite.play() #60757

Merged

Conversation

timothyqiu
Copy link
Member

3.x version of #60756

void AnimatedSprite::get_argument_options(const StringName &p_function, int p_idx, List<String> *r_options) const {
if (p_idx == 0 && p_function == "play" && frames.is_valid()) {
const String quote_style = EDITOR_GET("text_editor/completion/use_single_quotes") ? "'" : "\"";
List<StringName> al;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not very familiar with this but I noticed some of the other places in 3.x that do this, use this code:

#ifdef TOOLS_ENABLED
	const String quote_style = EDITOR_GET("text_editor/completion/use_single_quotes") ? "'" : "\"";
#else
	const String quote_style = "\"";
#endif

Is this necessary or does the EDITOR_GET work ok in non-tools builds?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, this whole get_argument_options() function is inside TOOLS_ENABLED, so it's not necessary to use the #ifdef.

I'm not sure if it's best practice. But I'm going to change it to use the #ifdef approach for the sake of consistency.

@timothyqiu timothyqiu force-pushed the animated-sprite-autocomplete-3.x branch from 8edc7c5 to 0f7f3d0 Compare May 5, 2022 01:08
Copy link
Member

@lawnjelly lawnjelly left a comment

Choose a reason for hiding this comment

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

I'm not an expert on this area but it looks fine to me.

@akien-mga akien-mga merged commit 6f5d57e into godotengine:3.x May 5, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added this to the 3.5 milestone May 5, 2022
@timothyqiu timothyqiu deleted the animated-sprite-autocomplete-3.x branch May 5, 2022 08:35
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