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 the autocomplete function for the self keyword. #86341

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

Vilcrow
Copy link
Contributor

@Vilcrow Vilcrow commented Dec 19, 2023

Fixes #85833

UPD: I will try to fix the bug with singleton in the coming days. While I'm converting the PR into a draft. there is no bug with singleton.

@Vilcrow Vilcrow requested a review from a team as a code owner December 19, 2023 21:36
@AThousandShips AThousandShips added bug topic:gdscript topic:editor regression cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Dec 19, 2023
@AThousandShips AThousandShips added this to the 4.3 milestone Dec 19, 2023
@am45-001
Copy link

am45-001 commented Dec 21, 2023

After implementing this pull request and compiling the 4.2.1 source code, it still fails to treat Singletons properly.
self. completion is fixed with this though. I haven't tried the master branch yet, so maybe there's something different there.
edit: Just realized, that singletons weren't intended to be fixed by this one, lol.

@adamscott
Copy link
Member

CC. @dalexeev @anvilfolk

@Vilcrow Vilcrow marked this pull request as draft December 22, 2023 13:15
@Vilcrow
Copy link
Contributor Author

Vilcrow commented Dec 24, 2023

I didn't find any problems with singleton. Everything seems to be working. Describe in more detail what it is @am45-001 .
Have you scrolled through the entire list of suggested completions for singleton?

@am45-001
Copy link

@Vilcrow , after taking another look, I seem to have been mistaken about singletons, and RenderingServer in particular.

@Vilcrow Vilcrow marked this pull request as ready for review December 24, 2023 17:33
@HolonProduction
Copy link
Member

As stated on the original issue, I don't quite understand the reason for the if statement. While you are at this code again, could you maybe add an explanatory comment, since its purpose isn't directly obvious.

@vnen
Copy link
Member

vnen commented Jan 2, 2024

As stated on the original issue, I don't quite understand the reason for the if statement. While you are at this code again, could you maybe add an explanatory comment, since its purpose isn't directly obvious.

I also don't understand how this could ever be false. With self being used explicitly, there's no way to also use super AFAICT.

In any case, even if the if is necessary for some reason, the r_type.type.is_meta_type = false line should be done after the if block, as it would apply to both branches.

@Vilcrow Vilcrow force-pushed the fix-self-completion branch from 79b48dd to c8fc824 Compare January 2, 2024 17:54
@Vilcrow
Copy link
Contributor Author

Vilcrow commented Jan 2, 2024

Initially, this code snippet was entered elsewhere(see #72789), where the if block made sense. Then it was moved here without the necessary adjustments(my bad).

@akien-mga akien-mga merged commit 8be3c4b into godotengine:master Jan 3, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga changed the title Fix the autocomplete function for the 'self' keyword. Fix the autocomplete function for the self keyword. Jan 4, 2024
@Vilcrow Vilcrow deleted the fix-self-completion branch January 5, 2024 13:23
@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 25, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

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.

self. completion suggesting incorrect code
8 participants