Issue with unintended possibility for multiple inheritance with Nodes and attached script objects.

[ZenDarva]

Tested versions

• reproducable in Godot v4.3.stable.official [77dcf97d8]

System information

Godot v4.3.stable - Windows 10.0.19045 - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 4060 Ti (NVIDIA; 32.0.15.6081) - AMD Ryzen 7 3800X 8-Core Processor (16 Threads)

Issue description

It's possible to get an object in godot into a state where it has two entirely different inheritance hierarchies.

You can still cast between the two different hierarchies, but it's messy. I suspect the whole thing is unintended.

Steps to reproduce

Start with a node of type CharacterBody2D, attach a script of type "second_type.gd" with any contents, which extends from Node, then attach an Area2D. Connect the "_on_area_2d_body_exited" or any other method that takes type Node2D. (The area2D is only used because it has a signal with an argument of type Node2D for demonstrating the issue)

In the body of the chosen function paste:

    var step = body;
    var x:second_type = step;
    print(body)
    print(step)
    print(x)

    #Semantically equivilent.  Does not work. Editor hates.
    var y:second_type = body as second_type

Observe that the editor complains about the last line saying "Invalid cast: cannot convert from "Node2D to "second_type"".

Comment that line out.

Run the application.

Trigger your event however you like.

Observe that body, step, and x are all the same object, although x is of type Node and body is of type Node2D, which should not be possible in Godots type structure.

Minimal reproduction project (MRP)

minimal_reproduction.zip

[ZenDarva]

A bit more testing makes it even more worrying if you expand the code to:

var step = body;
    var x:second_type = step;
    var y:Node = x
    print(body)
    print(step)
    print(x)    
    print(y)

You can see that y:Node == x:Node2D which seems bad?

[HolonProduction]

Attaching a script to a more specific node creates a kind of multiple inheritance on the value. Were is the problem with this though? As long as godot has clear rules for member resolution multi inheritance shouldn't be a problem right?

Also if you stick with static typing (which afaik is encouraged) you won't have this "problem" at all, since the multi inheritance is on the value, but a container (variable) can't represent it. This is why your cast fails. The assignment to x isn't semantically equivalent, since it does involve an untyped variable, which means that godot will check the assignment at runtime not at compile time (thus using the value and not the container type). In this case the runtime checks succeed because of the multi inheritance on the value.

Yeah it's a bit quirky but I don't see any issues coming from it. I also don't see a better way to handle it in a gradually typed language, as long as we want to keep the option to attach a script to a more specific node, which seems useful to me.

[ZenDarva]

Reasonable, i expected a wontfix.

However, the object will pass an "is a" check for Node2D and Node both. It is both types, and should be statically castable between them, if it's possible for it to actually be both.

The error message should probably be updated though. It's not that it can't be cast to that type, since it clearly can. something like "Cannot automatically cast from X to Y. Use an unsafe cast if this is intended", and only show that warning if a node is using multiple inheritance.

(This doesn't address what code gets called if you're calling methods shared between node and node2d either, but that's... beyond my scope of worry)

[kitbdev]

Maybe I misunderstood, but a Node2D is a Node. See the inheritance hierarchy, it inherits CanvasItem which inherits Node.

[ZenDarva]

Yes, and normally the Node2D would obviously be overriding the Node methods.

But, where what you're doing is welding an entire second heireracy onto the object are you comfortable in telling me for sure which methods or properties are going to get called in any particular situation?

Is it dependent on the type of the variable you're using to point at it? Is it always one, always the other? What? The docs don't give you a way to determine the answer.

Don't get hung up on Node2D vs Node as specific classes. You can do this with any two classes you want. It's not something specific to those two.

Suppose for a moment you glue together a Node2D and a Node3D. What type is .position? Vector2? Vector3?

[HolonProduction]

The error message should probably be updated though. It's not that it can't be cast to that type, since it clearly can.

This isn't possible, since GDScript does know nothing about the value that it will get at runtime. GDScript can only check against the static type which is Node2D the value is only known at runtime and can't be used for the error message.

Suppose for a moment you glue together a Node2D and a Node3D

That isn't possible (if you are able to, that would be worth adding to this issue), but what should be possible is to have a script which inherits Node, defines a visible variable and attach it to a Node2D. From brief testing, the script variable seems to always take precedence. But static type checks might check based on the Node2D type if stored in a container with this type.

We might want to add a configuration warning for this case :thinking:

[ZenDarva]

That isn't possible (if you are able to, that would be worth adding to this issue), but what should be possible is to have a script which inherits Node, defines a visible variable and attach it to a Node2D. From brief testing, the script variable seems to always take precedence. But static type checks might check based on the Node2D type if stored in a container with this type.

Whoops, i see you're right about the 2D->3D, my apologies. I thought i had tested that and it had worked, but I was wrong.

A configuration warning, or something in the docs explaining it, and what the different syntaxes are for dealing with the case might be nice.

I believe I've seen two, one walking it thru an untyped variable, and the other being [thing.]get_node(".") as <whatever>

It is slightly confusing when you're trying to statically type something, and come to a situation where you can't easily.