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

[BUG] Calling GetNode GetChild and GetChildren from C# scripts always returns null #166

Closed
IAmSegfault opened this issue Mar 10, 2023 · 12 comments
Labels
bug status: upstream Depending on upstream fix (typically Godot)

Comments

@IAmSegfault
Copy link

IAmSegfault commented Mar 10, 2023

I found a bug that affects C# scripts but not equivalent ones in GDScript. Calling the aforementioned Methods from a C# script when trying to access a node made with Rust seems to always return null.

Here's the code I tested for the rust node:

use godot::engine::Label;
use godot::prelude::*;

#[derive(GodotClass)]
#[class(base=Label)]
pub struct RustLabel {
    #[base]
    base: Base<Label>,
}

#[godot_api]
impl RustLabel {
    #[func]
    pub fn show_label(&mut self, lbl: Variant) {
        self.base.show();
        let gdstr = lbl.try_to().unwrap();
        self.base.set_text(gdstr);
    }

    #[func]
    pub fn call_cslabel(&self) {
        let np = NodePath::try_from("../../cslabel").unwrap();
        let cslabel_option = self.base.get_node(np);
        if cslabel_option.is_some() {
            let mut cslabel_node = cslabel_option.unwrap();
            let mut cslabel = cslabel_node.cast() as Gd<Label>;
            cslabel.show();
            let mut gs = GodotString::from_str("cs string set by rust");
            cslabel.set_text(gs.unwrap());
        }
    }
}

#[godot_api]
impl GodotExt for RustLabel {
    fn init(base: Base<Label>) -> Self {
        RustLabel { base }
    }

    fn ready(&mut self) {
        self.call_cslabel();
    }
}

And here's my C# code:

using Godot;
using System;

public partial class cslabel : Label
{
	// Called when the node enters the scene tree for the first time.
	public override void _Ready()
	{
	}

	// Called every frame. 'delta' is the elapsed time since the previous frame.
	public override void _Process(double delta)
	{
		NodePath pth = new NodePath("/root/myroot/cslabel/rl");
		var rs = GetNode(pth);
		String s = "Calling rust from cs";
		var gdstring = GD.StrToVar(s);
		rs.Call("show_label", gdstring);
	}
}

There's a node2d at the root of the scene called myroot, I tried adding cslabel and rl as children of myroot and also changing rl to be a child of cslabel and it made no difference.

Calling get_node from the Rust node to access the C# node works as intended.

I ran this on a Thinkpad T480s with an amd cpu and vega graphics chip running Pop!_OS. I don't have a way to test this on other platforms.

[Edit bromeon: syntax highlighting]

@Bromeon
Copy link
Member

Bromeon commented Mar 10, 2023

That's an interesting case. You say Node::GetChild and Node::GetChildren return null, but don't have those calls in your code?
Can you show the code that reproduces the problem and highlight where it exactly occurs?

I found a bug that affects C# scripts but not equivalent ones in GDScript.

This is strange, so it works with GDScript? In that case it could also be a bug with the C# binding.

One way to find out, although it's a bit involved: could you try the same thing with the godot-cpp binding instead of godot-rust? If the issue appears, we confirm it's not a problem on the Rust side 🤔

Also a small formatting tip: with ```rs and ```cs code tags, you can add syntax highlighting 🙂
(I did it for the initial post now)

@IAmSegfault
Copy link
Author

No, calling those methods from c# is what returns null. Calling get_node from rust will correctly get nodes with either c# or gdscript scripts attached to them. I'm not 100% sure if this has something to do with this library or if this is some kind of bug that impacts c# getting a node from any language using gdextension.

@Bromeon
Copy link
Member

Bromeon commented Mar 10, 2023

I'm not 100% sure if this has something to do with this library or if this is some kind of bug that impacts c# getting a node from any language using gdextension.

That's why it would be interesting to see if the same behavior can be reproduced with godot-cpp (if you have the time, of course). They have a small demo project here, maybe you could simply try that one 🙂

Other than that I wouldn't know where to start looking into this; I also have zero experience with Godot's C# binding.

@IAmSegfault
Copy link
Author

I'm going to see later today if I can reproduce the problem in c++. I'm also going to try out the following solution here for generating c# glue code and see if that works. If it does then probably all that needs to happen is some documentation describing generating the glue needs to be made. https://www.reddit.com/r/godot/comments/zlglpk/is_it_possible_to_call_gdextension_code_from_c/

@IAmSegfault
Copy link
Author

Attempting to generate c# glue code for the custom rust node resulted in the following errors. My best guess is getting the glue code generating correctly will solve the issue since the C# binding generator also claims that the rust node is null. It looks like the reason why is because there's a function that expects some kind of struct that details the node's method return types.

ERROR: Parameter "return_type" is null.
   at: _populate_method_icalls_table (modules/mono/editor/bindings_generator.cpp:800)
ERROR: Failed to generate icalls table for type: RustLabel
   at: _initialize (modules/mono/editor/bindings_generator.cpp:3899)
ERROR: Failed to initialize the bindings generator
   at: handle_cmdline_options (modules/mono/editor/bindings_generator.cpp:3912)

You can try creating the glue code yourself using:

./[path_to_godot_executable] --project /[path_to_project.godot] --generate-mono-glue /[where_to_save_the_bindings]

Here's the function in question that produces the glue code generator error:
https://github.com/godotengine/godot/blob/master/modules/mono/editor/bindings_generator.cpp#L799

And the struct it looks like it wants:
https://github.com/godotengine/godot/blob/518b9e5801a19229805fe837d7d0cf92920ad413/modules/mono/editor/bindings_generator.h#L123

I'll try looking into getting the c++ unit test working tomorrow, I don't currently have cmake installed or set up on my computer.

@Bromeon
Copy link
Member

Bromeon commented Mar 25, 2023

Any update on this?

I'd like to avoid keeping issues open that aren't actionable on our part, in order to maintain a good overview of what we need to work on 🙂

@IAmSegfault
Copy link
Author

I haven't had more time to look into this, since I just used a workaround by using gdscript to call back into rust. If I'm able to figure out what is causing the issue I will try submitting a pull request at some point. Likely though this will require asking one of the contributors that works on the C# part of Godot's internals.

@stevenctl
Copy link

Was able to reproduce this. A non-gdscript workaround (pretty icky) is to wrap the rust node type by extending the base type.
Ex:

public partial class RustLabel : Label {
   public void ShowLabel(String text) {
       var gdstring = GD.StrToVar(s);
       Call("show_label", gdstring);
   }
}

It's actually not even necessary to create the C# method. Simply making the Node in the tree have a C# defined class allows GetNode and then Call("rust_fn") still works.

@raulsntos
Copy link
Contributor

TL;DR: This is an issue in Godot's .NET module that affects every GDExtension, not just Godot Rust.


When trying to get a Godot Object in C#, the .NET module creates an instance of a C# type that acts as a proxy. So, for example, if you are trying to get a reference to a Label node, in C# we create an instance of the C# Label type that was generated by the bindings generator. The bindings generator creates C# proxy types for every Godot type registered in ClassDB.

In order to create an instance of the right type, the .NET module looks for a type in the GodotSharp or GodotSharpEditor assemblies with the same name as the native name (the name used in ClassDB). However, the bindings generator doesn't create proxy types for GDExtension classes, because the bindings generator is executed when building Godot and GDExtensions are loaded during runtime so we don't know they exist yet.

Generating C# glue for the rust types should fix it, but this would require rebuilding Godot (or at least, rebuilding the C# assemblies), which I would assume is not an acceptable solution. After all, the point of GDExtension is that you can extend Godot without having to rebuild the engine.

This issue affects every GDExtension and not just Godot Rust, it's a bug in Godot's .NET module. The closest issue we have to track this seems to be godotengine/godot#74801.

It may be possible to use C# Source Generators to generate the proxy types for GDExtension types when building the C# project, but as far as I know no one has started working on this at the moment.

@raulsntos
Copy link
Contributor

We have just released Godot 4.1 beta 2. This beta release includes godotengine/godot#75955 which should fix this issue.

@Bromeon
Copy link
Member

Bromeon commented Jun 14, 2023

Thanks a lot for fixing and reporting back here! 👍
I'm closing this then.

@IAmSegfault in case the problem still appears with Godot 4.1 beta 2 or later, let us know, then we can reopen.

@Bromeon Bromeon closed this as completed Jun 14, 2023
@Bromeon Bromeon added the bug label Jun 14, 2023
@IAmSegfault
Copy link
Author

I'll see if I can make time this week to run a unit test for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: upstream Depending on upstream fix (typically Godot)
Projects
None yet
Development

No branches or pull requests

5 participants