Code completion (type inference?) not working for custom types

[essial]

Godot version

4.0

System information

MacOS

Issue description

When calling a function that has a custom class, the return type is disregarded unless I explicitly cast the result to the type in the editor (breaking code completion)...

I have this function defined in a global class:

When I try to use the value returned by it, I get incorrect code completion:

Creating a new variable of the same type works (in the same file)

And if I cast the type to itself it also works...

Steps to reproduce

Create a custom class with class_name. Create a function in a global scene that returns an instance of that class type. Try to reference the return value and see code completion is broken.

Minimal reproduction project

I've provided specific screenshots describing the issue.

[vnen]

When I try to use the value returned by it, I get incorrect code completion:

BTW, is this really "incorrect" or just "incomplete"? The screenshot shows methods for RefCounted which is the default base type if you don't use extends.

[essial]

I believe so (in my opinion) because it's explicitly typed to the class, which has those properties. Using a variable specifically declared as PlayerState does not include RefCounted properties. You're a godot dev, so please correct me if I'm wrong!

I have a class, PlayerState which is in a PlayerState.gd file that has no extends at all. Simply class_name PlayerState at the top, followed by the vars which are the properties of the instance.

If I have a function that returns a value of type PlayerState, I would expect that it would be treated the same in the code editor as a variable who's type is explicitly defined as PlayerState.

Even more confusingly, casting the returned value with "as PlayerState" makes the right values show up -- even though I'm literally casting it to itself. It feels like the editor seems to just default to RefCounted type when using a custom base type.

Either that or I'm dumb and doing something wrong.

[essial]

Here's a partial file of PlayerState.gd (only included some of the vars, but the rest of the file is just more vars in the same manner:

## Represents an instance of a player
class_name PlayerState

## The multiplayer network ID
var remote_id:int = -1      

## The name of the account (a@b.com)
var account_name:String = ""        

## The selected character name
var character:String = ""       

## True if account is logged in
var logged_in:bool = false     

## True if character is loading a map
var loading:bool = true   

Here's the function in another module that uses this type:

func player_get(network_id:int) -> PlayerState:
    if not Director.is_server:
        push_error("Tried to get a player on the client side!")
        return null

    if not players.has(network_id):
        push_error("Failed to find player " + str(network_id) + "!")
        return null

    return players[network_id] as PlayerState

Interesting to note that I have to cast players[network_id] as PlayerState because I can't define value types in Dictionaries (as far as I can tell), but the content is absolutely a PlayerState object. But perhaps this is somehow confusing things, or maybe the paths that return NULL?

[Piralein]

Did a quick test, is your PlayerManager an autoload?

Using the function in the same file works, using a class instance works, using a static function works,

using an autoloaded class doesn't work.

[essial]

Yes, PlayerManager is autoloaded as it needs to basically be a singleton for the instance. Is there an alternative to AutoLoad for static instances (singletons) or is this a legit bug with auto loaded instances?

Great find @Piralein!

[essial]

Here's my autoload if it's any help:

And the first part of PlayerManager.gd:

extends Node

## Represents the list of players in the game
var players:Dictionary

## Gets a player based on the network ID
func player_get(network_id:int) -> PlayerState:
    if not Director.is_server:
        push_error("Tried to get a player on the client side!")
        return null

    if not players.has(network_id):
        push_error("Failed to find player " + str(network_id) + "!")
        return null

    return players[network_id] as PlayerState

Note that I'm only extending node because I have to in order for it to be part of the scene tree. Ideally I'd just have a static 'instance' variable I could use, but I don't see a way to do this in gdscript without autoloading a node.

[vnen]

I believe so (in my opinion) because it's explicitly typed to the class, which has those properties. Using a variable specifically declared as PlayerState does not include RefCounted properties. You're a godot dev, so please correct me if I'm wrong!

I have a class, PlayerState which is in a PlayerState.gd file that has no extends at all. Simply class_name PlayerState at the top, followed by the vars which are the properties of the instance.

If you don't use extends it behaves as if you had used extends RefCounted, since that's the default base. The fact that it does not show members from RefCounted is also a bug, because it should list members of base classes (which also includes Object).

There's definitely something odd going on given it's detecting something (RefCounted) but not the actual class.

[essial]

Awesome, I though I was going crazy and doing something horribly wrong. I just finished refactoring my entire codebase to use strongly defined class types and was worried I wasted a bunch of time. Thanks for figuring out what the specific problem is!

[santibag]

v4.1.stable.official [970459615] It would be so good if this gets fixed.

It doesn't have to have class keywords. If you defined a var or func, it's not suggested in a different script's code completion. Along with increasing the risk of typos, it adds additional steps for writing correct code.

I'm a relatively new dev. When I learned about type hints, I started using them too. Because getting completion saves many things.

Edit: Images as an example:

As can be seen, particle.gd file is saved, with a var called "potato". To make it resemble public variables, I even added \@export to it. But it's not suggested in border.gd when I refer to the node in the script. I also made sure to edit both codes in main.tscn scene to make sure they were both instantiated in the scene in some way.

[dalexeev]

@santibag In this particular case, the behavior is expected. The get_node() method, and also $ and % shorthands have Node type, not a specific subtype. GDScript does not know what type the node will be at runtime and whether the node be at all.

Autocompletion can give you correct suggestions for node paths, variables, functions, etc. only if this scene is the currently open one in the editor. This does not work for other scenes as the same script can be attached to different scenes. See also godotengine/godot-proposals#1935.

To workaround this, I recommend using @onready variables with a specified static type, instead of using the $ notation multiple times in the function body. It also improves performance in frequently called functions like _process and loops.

Also, some people use the as operator (like ($Label as Label).text), but I prefer not to use it in this manner because it is 1. more verbose, 2. less optimal, extra code at runtime, 3. the as operator hides errors by silently casting the object to null on type mismatch.

[wagnerfs]

This is actually quite annoying, having code complete on normal custom classes with properties of other custom classes work just fine, you put it as Autload and bam, it doesn't work anymore, to make matters worse there's not work around to that, managing a project with a couple autoloads and several properties/methods with custom classes.

I'm almost certain this seems like a regression since I don't think I used to have this issue before 4.1. For now I'm updating my code to do casting on those, but I'm not quite sure if this kind of casting has any hit on performance.

[Nukiloco]

It felt like I been dealing with this bug since 3.x, even having type hints as your variables doesn't allow the autocomplete to work. Probably one of the most annoying bugs in this engine so far since most of my scripts in most of my projects point to an autoload so I basically never get any type hints at all. I usually have to do special key combos like Shift Alt O, and go between the files, just to check out the types. This happens to not just functions but variables as well in autoload. Wasn't there another issue on autoload not returning any type hints in a different way?

[HolonProduction]

So looking at this I found three issues in the code. Two of which should be easy to fix:

1. Easy to Fix: _guess_identifier_type knows the statically annotated type, but still tries to guess a type from the last assigned expression. However this interfered type should only be returned if it is compatible with the static one and more specific.
2. Easy to Fix: _type_from_property doesn't handle custom class names in the PropertyInfo it just returns a native type with the native class PlayerState. Since there is no such native class no auto completion is provided.

Those two things can be changed at one point and will fix most of the cases dealing with this. (That said before working on that, other issues and PR's should be cross referenced since the auto completion issues tend to overlap and I also kinda remember other issues and PR's regarding autoloads.

3. Hard to fix: Since I'm a fan of preloaded types over global class names I also looked into this. As long as we statically type everything in our script and properly fix 1. we should be fine. But sadly we cannot interfere the return type from the autoload. The autoload is resolved as SCRIPT type not as CLASS type, meaning we cannot use the GDScript internals and only have access to get_property_list. This limits the type information to a PropertyInfo which has no good way to store Script types that are not globally named. (There are ways for Resources and Nodes through hints but I don't know if they support custom scripts and also this feels very hacky).

The two solutions I see for this is either expanding PropertyInfo or resolving autoloads as CLASS types. I think the later sounds more feasible but I lack the knowledge to pull it off. Also since this is only a problem for users that use static typing though preload in their autoloads, but no static typing in the rest of their scripts, this sounds more like an edge case without priority.

[wagnerfs]

Not sure if this helps anything, but I noticed that if you set a custom type variable to static, suddenly the autoload code complete starts working

# main_controller.gd
static var game: GameScene

Quick update here...

I digged through the whole thing and like @HolonProduction said, it boils down to PropertyInfo which's fed by script->member_info that's populated by only the class's extended native.

StringName GDScript::get_instance_base_type() const {
    if (native.is_valid()) {
        return native->get_name();
    }
    if (base.is_valid() && base->is_valid()) {
        return base->get_instance_base_type();
    }
    return StringName();
}

And once it tries to check for a valid type from that property, it checks if a class_name is set inside _type_from_property to either fill up as SCRIPT or NATIVE

if (ScriptServer::is_global_class(p_property.class_name)) {

I changed get_instance_base_type to return the class_name if found and it basically worked, but didn't really get a list of the other properties and methods from the extended class, so it's either what to store for class_name in PropertyInfo or maybe update r_list in _get_script_property_list using member_indices instead of member_info

[wagnerfs]

Just out of curiosity, is there a reason why PropertyInfo stores the native class name instead of the class_name defined by the user when available?

[HolonProduction]

Just out of curiosity, is there a reason why PropertyInfo stores the native class name instead of the class_name defined by the user when available?

Could you provide the exact case were this happens?

With the following:

I get this as output:

{ "name": "test", "class_name": &"State", "type": 24, "hint": 0, "hint_string": "", "usage": 4096 }

Would be good to look into the cases were this does not get propagated correctly.

[HolonProduction]

Also changing get_instance_base_type is the wrong approach. It is the native type which needs to be instantiated. What needs fixing are the cases were the Global Class name gets not added to the property info.

[wagnerfs]

Also changing get_instance_base_type is the wrong approach. It is the native type which needs to be instantiated. What needs fixing are the cases were the Global Class name gets not added to the property info.

It's because, like you mentioned, autoloads are treated like SCRIPT, and when it access its properties, things like

var a1: CustomScript
var a2: Node

all properties with custom types become the native class they extend, so a1 type would be Node and that's why this kinda code doesn't complete properly:

Autoload.a1.

as for your example, I was talking about the source, apparently, if your property is of type SCRIPT or GDSCRIPT, class_name will always return the native/extended name instead of the current user defined name. The _get_script_property_list I was talking about is also in the source, specifically called when feeding the code complete for said autoload property.

[HolonProduction]

as for your example, I was talking about the source, apparently, if your property is of type SCRIPT or GDSCRIPT, class_name will always return the native/extended name instead of the current user defined name. The _get_script_property_list I was talking about is also in the source, specifically called when feeding the code complete for said autoload property.

Ok somewhat misread what you wrote. Doesn't change much for me:

Note Yeah I'm using GDScript here, but the bound method uses the internal method you are referring to, so results shouldn't change. Also I have seen the global class_name in the debugger multiple times, but GDScript is just simpler to reproduce.

Still gives me:

{ "name": "val", "class_name": &"State", "type": 24, "hint": 0, "hint_string": "", "usage": 4096 }

Furthermore I also debugged the problem and created three PR's for it. All those PR's make sure that the code completion properly handles the global class_name when it receives one. In all cases that I tested the global class_name reached the code completion without problems. So please provide clear steps to reproduce the case were the PropertyInfo does not contain the global class name.

all properties with custom types become the native class they extend, so a1 type would be Node and that's why this kinda code doesn't complete properly:

Autoload.a1.

In my tests a1 had the type CustomType but the code completion tried to use this type as native type, which leads to no results, since there is no native type with the name CustomType. That is why you don't receive any completion options at all instead of only node ones.

Note: The code completion options for Global don't get closed properly when typing the dot after a1. So you should close them with Esc. The code completion won't pop up again.

Screencast from 2023-11-30 11-12-14.webm

[wagnerfs]

Try this:

#thing.gd
extends Node
class_name StuffThing

var bla: int = 10

#autoload.gd
extends Node

var a: StuffThing

#any other file
extends Node2D

func _ready():
    var l = Autoload.get_script().get_script_property_list()
    print(l)

outputs:

[{ "name": "autoload.gd", "class_name": &"", "type": 0, "hint": 0, "hint_string": "res://autoload.gd", "usage": 128 }, { "name": "a", "class_name": &"Node", "type": 24, "hint": 0, "hint_string": "", "usage": 4096 }]

As you can see, var a is of type Node, not StuffThing. This happens because, like I mentioned before, instead of getting the class name when available, it skips that and looks for the native type ONLY, and I don't understand why since I deliberately defined a class_name for my thing.gd file.

Also like I mentioned before, class_name is requested different ways throughout the code, the one specifically for code completing requests native class name, not the user defined name (like my StuffThing) thus not completing it, and also like I mentioned, by changing that to retrieve the actual name when available, it fixed completing for that.

[HolonProduction]

Can't reproduce :confused:

[{ "name": "autoload.gd", "class_name": &"", "type": 0, "hint": 0, "hint_string": "res://autoload.gd", "usage": 128 }, { "name": "a", "class_name": &"StuffThing", "type": 24, "hint": 0, "hint_string": "", "usage": 4096 }]

v4.3.dev.custom_build [d76c1d0e5]

[wagnerfs]

Can't reproduce 😕

[{ "name": "autoload.gd", "class_name": &"", "type": 0, "hint": 0, "hint_string": "res://autoload.gd", "usage": 128 }, { "name": "a", "class_name": &"StuffThing", "type": 24, "hint": 0, "hint_string": "", "usage": 4096 }]

v4.3.dev.custom_build [d76c1d0]

OH, good call, I've been testing around 4.1 branch not master, my bad, sorry