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

GDScript Analyzer cannot correctly resolve C# arguments methods. #89924

Closed
ZerxZ opened this issue Mar 26, 2024 · 2 comments · Fixed by #90968
Closed

GDScript Analyzer cannot correctly resolve C# arguments methods. #89924

ZerxZ opened this issue Mar 26, 2024 · 2 comments · Fixed by #90968

Comments

@ZerxZ
Copy link
Contributor

ZerxZ commented Mar 26, 2024

Tested versions

v4.3.dev5.mono.official.89f70e98d

System information

Windows 10.0.22000 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 Laptop GPU (NVIDIA; 31.0.15.5123) - 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz (16 Threads)

Issue description

It runs correctly in version 4.2.1, but errors occur starting from version 4.3.
If this segment of source code is removed, runs normally.

validate_call_arg(par_types, default_arg_count, method_flags.has_flag(METHOD_FLAG_VARARG), p_call);

CSharp:

using Godot;
using System;

public partial class GlobalDotnetScript : Node
{
    public static void SayHi(int a, int b, int c, int d, int e, int f)
    {
        GD.Print($"[GlobalDotnetScript] SayHi: {a}, {b}, {c}, {d}, {e}, {f}");
    }

    public void SayHello(int a, int b, int c, int d, int e, int f)
    {
        GD.Print($"[GlobalDotnetScript] SayHello: {a}, {b}, {c}, {d}, {e}, {f}");
    }
    public static void SayHi()
    {
        GD.Print($"[GlobalDotnetScript] SayHi!");
    }

    public void SayHello()
    {
        GD.Print($"[GlobalDotnetScript] SayHello!");
    }
}

GDScript:

extends Node

const GlobalDotnetScript = preload("res://GlobalDotnetScript.cs");

func _ready() -> void:
	GlobalDotnetScript.SayHi(1,2,3,4,5,6);
	var node =GlobalDotnetScript.new();
	add_child(node);
	node.SayHello(1,2,3,4,5,6)
	# Parse Error: Too few arguments for "SayHello()" call. Expected at least 6 but received 0.
	node.SayHello();
	# Parse Error: Too few arguments for "SayHi()" call. Expected at least 6 but received 0.
	GlobalDotnetScript.SayHi();

Steps to reproduce

  1. Open the project.
  2. Open the 'test_node.tscn' scene and run it.

Minimal reproduction project (MRP)

csharp-test-method.zip

@dalexeev
Copy link
Member

dalexeev commented Mar 26, 2024

I think that this is not a regression, since GDScript was not previously aware of the other languages. Probably the problem is that the Godot API (MethodInfo/get_method_list()) does not support method overloads (only built-in constructors and operators). So this is probably a question for the @godotengine/dotnet team.

I think that if you use one method with optional arguments instead of two overloads, it will work.

@roookeee
Copy link

roookeee commented Mar 27, 2024

Giving my 2 cents because I dove into the C# binding recently:

#85703 was implemented by calling get_method_info(string) which only does a lookup by name, disregarding any kind of parameter count checks.

Technically speaking MethodInfo contains the argument count information which could be used for looking up the correct overload by name and parameter count.

Please note that same argument count overloading by type will not work as that is currently not handled by the C# binding.

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.

4 participants