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

self. completion suggesting incorrect code #85833

Closed
am45-001 opened this issue Dec 6, 2023 · 11 comments · Fixed by #86341
Closed

self. completion suggesting incorrect code #85833

am45-001 opened this issue Dec 6, 2023 · 11 comments · Fixed by #86341

Comments

@am45-001
Copy link

am45-001 commented Dec 6, 2023

Tested versions

Tested in v4.2.stable.official [46dc277], v4.2.1.stable.official [b09f793].

System information

Godot v4.2.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated Radeon RX 580 Series (Advanced Micro Devices, Inc.; 31.0.21905.1001) - AMD Ryzen 5 3600 6-Core Processor (12 Threads)

Issue description

The self keyword are assumed to be classes by code completion (unlike before in v4.1 and previous versions), and only display constants, static functions, and new().

Edit: After further investigation by @Vilcrow, I jumped the gun on Singletons suffering the same issue as the self keyword. Apologies to @MitrB for stealing your thunder on this one.

Steps to reproduce

  • In any project open in version 4.2, create or open a script.
  • Type "self." and observe the code completion suggestions.
  • You should see something like this:
    Screenshot 2023-12-06 061131

Minimal reproduction project (MRP)

BugProject.zip

@yosoyfreeman
Copy link
Contributor

Confirmed in Linux too.

@dalexeev
Copy link
Member

dalexeev commented Dec 7, 2023

@dalexeev dalexeev added this to the 4.3 milestone Dec 7, 2023
@dalexeev dalexeev changed the title Code completion suggesting incorrect code self. and singletons completion suggesting incorrect code Dec 7, 2023
@grumpOldman
Copy link

I'm pretty sure this is a regression as I could use this before and waiting for 4.3 seems odd.

@AThousandShips
Copy link
Member

AThousandShips commented Dec 9, 2023

It being marked for 4.3 doesn't mean you have to wait for 4.3, if a compatible bug fix can be made it can be cherry picked for 4.2.x

We only assign a released milestone to bugs if they are unique to that milestone, like a regression specifically in 4.2 that isn't in the development branch

@am45-001
Copy link
Author

am45-001 commented Dec 9, 2023

I'm pretty sure this is a regression as I could use this before and waiting for 4.3 seems odd.

as a workaround you can use an external code editor like VSCode, and it will get around the missing instance variable and method issues. It will still kind of treat self and singletons like classes, but you'll actually be able to code with it.

@am45-001
Copy link
Author

am45-001 commented Dec 9, 2023

I'm pretty sure this is a regression as I could use this before and waiting for 4.3 seems odd.

as a workaround you can use an external code editor like VSCode, and it will get around the missing instance variable and method issues. It will still kind of treat self and singletons like classes, but you'll actually be able to code with it.

Well, this is awkward, I just tried this again and it doesn't work [edit] for self specifically, singletons work fine.[/edit]

Edit: the real workaround is casting self as the class it is, which will force the completer to treat it as an instance.

Edit 2: The final workaround I've come up with is to assign self to a variable while casting self as the class it is. It treat the variable as an instance otherwise.

@HolonProduction
Copy link
Member

Did a bisect. Was introduced in 56e2fad. (#73196)
The changes to gdscript_editor.cpp don't make sense to me and don't seem to have gotten any discussion during the PR. Could you explain how they relate to the issue @Vilcrow

@Vilcrow
Copy link
Contributor

Vilcrow commented Dec 18, 2023

@HolonProduction changes to gdscript_editor.cpp have been made to correct #72353 which led to the incorrect completion. I've tried to figure it out a little bit now, but so far there are no results. I will try to fix this bug as soon as I have time.

@HolonProduction
Copy link
Member

Yeah I read the isssue and you also described what caused the issue in you PR. What I don't understand is how these changes:

if (p_context.type != GDScriptParser::COMPLETION_SUPER_METHOD) {
    r_type.type = p_context.current_class->get_datatype();
} else {
    r_type.type = p_context.current_class->base_type;
}

are related to the cause of the issue that you described in the PR (you said it had something todo with a wrong position being used in CodeEdit).

This issue (as in #85833) results from p_context.current_class->get_datatype() being a meta type (the class, not an instance of the class; that is why you only see new. So this should just be switched back to how the type was constructed before.

I also don't see what you are trying to do with p_context.type != GDScriptParser::COMPLETION_SUPER_METHOD. Could you give an example case were this would be false.

So yeah IMO I would just try to revert all changes to gdscript_editor.cpp. But you propably know better what this code is supposed to do.

@am45-001 am45-001 changed the title self. and singletons completion suggesting incorrect code self. completion suggesting incorrect code Dec 24, 2023
@MitrB
Copy link

MitrB commented Dec 24, 2023

@am45-001 No need to apologise, I wasn't going to be very involved either way

@am45-001
Copy link
Author

am45-001 commented Jan 4, 2024

@Vilcrow would it be appropriate for me to upload the custom version of 4.2.1 with your fix here? Or would I need to publish it somewhere else while we wait for the next version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants