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

Autocompletion from get_node() still does not work for some cases #86172

Open
dhoverml opened this issue Dec 14, 2023 · 10 comments
Open

Autocompletion from get_node() still does not work for some cases #86172

dhoverml opened this issue Dec 14, 2023 · 10 comments

Comments

@dhoverml
Copy link

dhoverml commented Dec 14, 2023

Tested versions

v4.2.1.stable.official [b09f793]
v4.2.1.stable.official [b09f793] + #85224
v3.5.3.stable.official [6c81413]

System information

Ubuntu 20.04 LTS

Issue description

Since #79386, autocompletion improved, but it still does not work for other get_node() cases.
This is a regression (since GDScript 2.0 as far as I can tell) from Godot 3.5 since all cases (except 4, see NOTES in Steps to reproduce) autocomplete.

These related bugs differ slightly and are not using get_node() directly.
#74888 (custom function returns)
#71296 (autoload)
#78860 (superseded by this new bug, I also thought it's a little unclear)
#78820 (return from Dictionary)

I also thought it would be nice to have one MRP to exhaust all combinations for get_node() autocompletion since it's a common usecase.

Steps to reproduce

  1. Load the MRP
  2. In the script attached to node A. Try typing out do_something() and observe similar behavior as noted in the comments. I've pasted script A.gd below for reference.
extends Control

@onready var b = $B
@onready var b_get_node = get_node("B")
@onready var b_typed := $B
@onready var b_typed_get_node := get_node("B")

@onready var c = $C
@onready var c_get_node = get_node("C")
@onready var c_typed : C = $C
@onready var c_typed_get_node : C = get_node("C")

var d
var d_get_node

var e
var e_get_node
var e_typed : E
var e_typed_get_node : E

@onready var f = %F
@onready var f_get_node = get_node("%F")
@onready var f_typed := %F
@onready var f_typed_get_node := get_node("%F")

@onready var g = %G
@onready var g_get_node = get_node("%G")
@onready var g_typed : G = %G
@onready var g_typed_get_node : G = get_node("%G")

var h
var h_get_node

var i
var i_get_node
var i_typed : I
var i_typed_get_node : I

func _ready():
  # -- B (onready)

  # inline
  $B.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  get_node("B").do_something() # does not autocomplete

  # script vars
  b.do_something() # autocompletes
  b_get_node.do_something() # does not autocomplete
  b_typed.do_something() # autocompletes
  b_typed_get_node.do_something() # does not autocomplete

  # local vars
  var blocal = $B
  var blocal_get_node = get_node("B")
  var blocal_typed := $B
  var blocal_typed_get_node := get_node("B")
  blocal.do_something() # does not autocomplete
  blocal_get_node.do_something() # does not autocomplete
  blocal_typed.do_something() # does not autocomplete
  blocal_typed_get_node.do_something() # does not autocomplete

  # -- C (onready, class_name)

  # inline
  $C.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  get_node("C").do_something() # does not autocomplete

  # script vars
  c.do_something() # autocompletes
  c_get_node.do_something() # does not autocomplete
  c_typed.do_something() # autocompletes
  c_typed_get_node.do_something() # autocompletes

  # local vars
  var clocal = $C
  var clocal_get_node = get_node("C")
  var clocal_typed : C = $C
  var clocal_typed_get_node : C = get_node("C")
  clocal.do_something() # does not autocomplete
  clocal_get_node.do_something() # does not autocomplete
  clocal_typed.do_something() # autocompletes with 4.2.1-stable + #85224
  clocal_typed_get_node.do_something() # autocompletes with 4.2.1-stable + #85224

  # -- D

  # script vars
  d = $D
  d.do_something() # does not autocomplete
  d_get_node = get_node("D")
  d_get_node.do_something() # does not autocomplete

  # -- E (class_name)

  # script vars
  e = $E
  e.do_something() # does not autocomplete
  e_get_node = get_node("E")
  e_get_node.do_something() # does not autocomplete
  e_typed = $E
  e_typed.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  e_typed_get_node = get_node("E")
  e_typed_get_node.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111

  # -- F (onready, unique name)

  # inline
  %F.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  get_node("%F").do_something() # does not autocomplete

  # script vars
  f.do_something() # autocompletes
  f_get_node.do_something() # does not autocomplete
  f_typed.do_something() # autocompletes
  f_typed_get_node.do_something() # does not autocomplete

  # local vars
  var flocal = %F
  var flocal_get_node = get_node("%F")
  var flocal_typed := %F
  var flocal_typed_get_node := get_node("%F")
  flocal.do_something() # does not autocomplete
  flocal_get_node.do_something() # does not autocomplete
  flocal_typed.do_something() # does not autocomplete
  flocal_typed_get_node.do_something() # does not autocomplete

  # -- G (onready, class_name, unique name)

  # inline
  %G.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  get_node("%G").do_something() # does not autocomplete

  # script vars
  g.do_something() # autocompletes
  g_get_node.do_something() # does not autocomplete
  g_typed.do_something() # autocompletes
  g_typed_get_node.do_something() # autocompletes

  # local vars
  var glocal = %G
  var glocal_get_node = get_node("%G")
  var glocal_typed : G = %G
  var glocal_typed_get_node : G = get_node("%G")
  glocal.do_something() # does not autocomplete
  glocal_get_node.do_something() # does not autocomplete
  glocal_typed.do_something() # autocompletes with 4.2.1-stable + #85224
  glocal_typed_get_node.do_something() # autocompletes with 4.2.1-stable + #85224

  # -- H (unique_name)

  # script vars
  h = %H
  h.do_something() # does not autocomplete
  h_get_node = get_node("%H")
  h_get_node.do_something() # does not autocomplete

  # -- I (class_name, unique_name)

  # script vars
  i = %I
  i.do_something() # does not autocomplete
  i_get_node = get_node("%I")
  i_get_node.do_something() # does not autocomplete
  i_typed = %I
  i_typed.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111
  i_typed_get_node = get_node("%I")
  i_typed_get_node.do_something() # autocompletes with 4.2.1-stable + #85224 + #86111

NOTES

  blocal.do_something()
  blocal_get_node.do_something()
  blocal_typed.do_something()
  blocal_typed_get_node.do_something()

Minimal reproduction project (MRP)

getnodebug.zip

@dhoverml dhoverml changed the title get_node() still does not work for some cases Autocompletion from get_node() still does not work for some cases Dec 14, 2023
@HolonProduction
Copy link
Member

HolonProduction commented Dec 18, 2023

Read this before adding a comment:

Autocompletion is a complex topic where a lot of internal things can go wrong. Just because the title of this issue is related to the same topic, does not mean that this is the same issue that you encountered. Even if the title sounds like it, this is not a multipurpose thread. The original poster of this issue provided a specific problem (or problems), which are being discussed here. Read the original issue carefully. If it aligns with your problem there is no need to add a comment just leave a emoji reaction to let us know the issue is a common problem. If your problem requires adding code that is not related to the OP, it is most likely worth a new issue. If you are sure that you provide additional info related to the issue: Great, go ahead and help us figure it out!

Let's keep the Github issues well organized together. 😃


So i looked to the cases and splited them up into some categories:

  • get_node: everything using get_node instead of $/% won't work. This is hard to fix without bigger implementation changes. Furthermore whether this should be supported needs to be discussed first. In comparison to $/% the method may add a lot of edge cases that may feel very buggy if not handled correctly. (Talking about things like calling get_node on a variable. It might be another node, it might be a reference to self, ...)
  • Inline: Inline usage of $/% does not work. This is a regression from my orginial PR and was fixed in Fix regression when autocompleting subscript on get node #86111. It will probably get cherry picked for the next release.
  • local untyped (or type infered): this results from wrong code for determining the assigned value of a local var inside of _get_subscript_type. This could be fixed there but would duplicate code from _get_expression_type so my preferred solution would be to remove _get_subscript_type and integrate it into the later. This however may break quite some stuff so before working on this I wan't to have solid unit tests for autocompletion.
  • property with local asignment (d): _get_subscript_type does not search for local assignments, _get_expression_type does. Another case were merging them would make sense.
  • local assignment of typed class property: Basically the same problem as Prefer identifiers annotated type if assigned type is incompatible to it #85224 but with properties instead of local variables I updated Prefer identifiers annotated type if assigned type is incompatible to it #85224 so that it should fix this case as well now.

@dhoverml
Copy link
Author

dhoverml commented Dec 18, 2023

Thank you for the detailed response!
I've edited the original description to include results from #86111. Looks like it fixes the inline cases as you've stated.

get_node: everything using get_node instead of $/% won't work. This is hard to fix without bigger implementation changes. Furthermore whether this should be supported needs to be discussed first.

Godot 3.5.3 had this working, but I am unaware of whether its implementation produced bugs considering how difficult it is. This may be a hack, but would it be possible to hardcode these cases? For example, if the code explicitly parses get_node, implicitly convert it to an inline case and handle that? Though this is similar to preprocessing which is an entirely different discussion.

@HolonProduction
Copy link
Member

I updated #85224 to fix case 5.

@Muntaner
Copy link

Muntaner commented Dec 24, 2023

Was about to open a new issue, but maybe adding a comment here with my experience is enough.

I'm also experiencing autocomplete issues with Godot 4.2.1 (v4.2.1.stable.official [b09f793]).
In particular, when referring to another node by using the $ notation, the autocomplete does not show properties of the target node, but only under a particular condition: if the currently selected scene is the one containing the target node, the autocomplete works properly. If I select another scene, it does not work.

Screenshots follow.
Working autocompletion:
works
Bugged autocompletion:
bug

@HolonProduction please let me know if you need a separate issue for this or a minimal project reproducing it to help troubleshooting (and if this is an issue at all, maybe it's working like this by design?)

Edit: it looks like the initial variable assignment in the code reported in the screenshots has some effect on the autocompletion feature. If I remove the assignment and refer to the node directly with the $ notation, the autocomplete does not work - no matter what the selected scene is:

extends Node2D

func _ready():
	$Logo.rotation_ // <- never autocompletes, no matter the scene

@HolonProduction
Copy link
Member

HolonProduction commented Dec 24, 2023

There goes my "No Godot on holidays plan". But to net let this get out of hand here is my quick response:

The problem you described in your edit was pointed out in this issue already. Recap: it is fixed, the fix is merged but it is not yet part of an official release.

Your original issue is from what I know a design decision. The problem is the following: While you have a specific scene in mind for your scrip, Godot has not. In theory you could attach the script to every node or scene, so Godot can't know which you are referring to. So the decision was made, to assume you are referring to the currently open scene (in many cases this holds true, when you open the script via the scene tree dock, but it becomes a problem if you tend to switch your scripts in the script editor). In theory Godot could check all scenes or all open scenes, but this won't scale well and lead to performance issues down the road.

There are some proposals to provide a tight coupling between a script and a certain scene/node. But I don't know what the status is on that front. Wouldn't expect it to be solved soon, but if it gets in it would provide a way to provide autocompletion beyond the currently open scene.

Let's end this with a quick disclaimer (not specifically targeting you, but more as a copy and pastable thing for all autocompletion issues, since there tends to be a lack of organization on them).

Edit: Moved the disclaimer to the beginning of my first comment so that we can focus on the issue here.

@Nanaschi
Copy link

#45414 (comment)

@JekSun97
Copy link
Contributor

The problem is still present in version 4.3 beta1

@HolonProduction
Copy link
Member

The problem is still present in version 4.3 beta1

Please reread my first comment and be specific about your issue.

@JekSun97
Copy link
Contributor

The problem is still present in version 4.3 beta1

Please reread my first comment and be specific about your issue.

#92185

@HolonProduction
Copy link
Member

#92185

This is not fixed yet and I wouldn't bet on it getting fixed any time soon. I updated my first comment to reflect which points are still open. Use dollar literals in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Up for grabs
Development

No branches or pull requests

6 participants