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

AddCommandHandlerCallable cannot support gdscript lambdas, but could be clearer about it #88

Open
misha-cilantro opened this issue Mar 8, 2025 · 2 comments

Comments

@misha-cilantro
Copy link

misha-cilantro commented Mar 8, 2025

GDScript is happy to let you pass a lambda to AddCommandHandlerCallable because a lambda is a Callable, but C# cannot accept a custom callable (which a lambda is); the method runs, but there is no target and no delegate (in fact, every property is null) so it's useless. See this issue:

godotengine/godot#76108 (comment)

The error you get is that the node may be freed, but this isn't the issue. Suggested check and error below:

public void AddCommandHandlerCallable(string commandName, Callable handler)
    {
        // Custom Callables cannot be passed to C#.
        // See: https://github.com/godotengine/godot/issues/76108#issuecomment-1719304017
        // A lambda is a custom callable, so lacks a method to call or any useful info.
        if (!IsInstanceValid(handler.Target) && handler.Delegate == null)
        {
            GD.PushError(
                $"Callable provided to {nameof(AddCommandHandlerCallable)} is null. " +
                "Did you pass a lambda? Lambdas cannot be passed to this method.");
            return;
        }

        ...

And suggest comment change:

/// <summary>
/// Add a command handler using a Callable rather than a C# delegate.
/// Mostly useful for integrating with GDScript.
///
/// Note that only object methods can be used, you CANNOT pass a GDScript lambda.

Full method:

/// <summary>
/// Add a command handler using a Callable rather than a C# delegate.
/// Mostly useful for integrating with GDScript.
///
/// Note that only object methods can be used, you CANNOT pass a GDScript lambda.
/// 
/// If the last argument to your handler is a Callable, your command will be
/// considered an async blocking command. When the work for your command is done,
/// call the Callable that the DialogueRunner will pass to your handler. Then
/// the dialogue will continue.
///
/// Callables are only supported as the last argument to your handler for the
/// purpose of making your command blocking.
/// </summary>
/// <param name="commandName">The name of the command.</param>
/// <param name="handler">The Callable for the <see cref="CommandHandler"/> that
/// will be invoked when the command is called.</param>
public void AddCommandHandlerCallable(string commandName, Callable handler)
{
    // Custom Callables cannot be passed to C#.
    // See: https://github.com/godotengine/godot/issues/76108#issuecomment-1719304017
    // A lambda is a custom callable, so lacks a method to call or any useful info.
    if (!IsInstanceValid(handler.Target) && handler.Delegate == null)
    {
        GD.PushError(
            $"Callable provided to {nameof(AddCommandHandlerCallable)} is null. " +
            "Note that lambdas cannot be passed to this method.");
        return;
    }
    
    if (!IsInstanceValid(handler.Target))
    {
        GD.PushError(
            $"Callable provided to {nameof(AddCommandHandlerCallable)} is invalid. " +
            "Could the Node associated with the callable have been freed?");
        return;
    }

    var methodInfo = handler.Target.GetMethodList().Where(dict =>
        dict["name"].AsString().Equals(handler.Method.ToString())).ToList();

    if (methodInfo.Count == 0)
    {
        GD.PushError();
        return;
    }

    var argsCount = methodInfo[0]["args"].AsGodotArray().Count;
    var argTypes = methodInfo[0]["args"].AsGodotArray().ToList()
        .ConvertAll((argDictionary) =>
            (Variant.Type) argDictionary.AsGodotDictionary()["type"].AsInt32());
    var invalidTargetMsg =
        $"Handler node for {commandName} is invalid. Was it freed?";


    async Task GenerateCommandHandler(params Variant[] handlerArgs)
    {
        if (!IsInstanceValid(handler.Target))
        {
            GD.PushError(invalidTargetMsg);
            return;
        }

        var castArgs = CastToExpectedTypes(argTypes, commandName, handlerArgs);

        var returnValue = handler.Call(castArgs.ToArray());
        if (returnValue.Obj != null && returnValue.As<GodotObject>().GetClass() == "GDScriptFunctionState")
        {
            // callable is from GDScript with await statements
            await ((SceneTree) Engine.GetMainLoop()).ToSignal(returnValue.AsGodotObject(), "completed");
        }
    }

    switch (argsCount)
    {
        case 0:
            AddCommandHandler(commandName,
                async Task () => await GenerateCommandHandler());
            break;
        case 1:
            AddCommandHandler(commandName,
                async Task (Variant arg0) =>
                    await GenerateCommandHandler(arg0));
            break;
        case 2:
            AddCommandHandler(commandName,
                async Task (Variant arg0, Variant arg1) =>
                    await GenerateCommandHandler(arg0, arg1));
            break;
        case 3:
            AddCommandHandler(commandName,
                async Task (Variant arg0, Variant arg1, Variant arg2) =>
                    await GenerateCommandHandler(arg0, arg1, arg2));
            break;
        case 4:
            AddCommandHandler(commandName,
                async Task (Variant arg0, Variant arg1, Variant arg2,
                        Variant arg3) =>
                    await GenerateCommandHandler(arg0, arg1, arg2, arg3));
            break;
        case 5:
            AddCommandHandler(commandName,
                async Task (Variant arg0, Variant arg1, Variant arg2,
                        Variant arg3, Variant arg4) =>
                    await GenerateCommandHandler(arg0, arg1, arg2, arg3, arg4));
            break;
        case 6:
            AddCommandHandler(commandName,
                async Task (Variant arg0, Variant arg1, Variant arg2,
                        Variant arg3, Variant arg4, Variant arg5) =>
                    await GenerateCommandHandler(arg0, arg1, arg2,
                        arg3, arg4, arg5));
            break;
        default:
            GD.PushError($"You have specified a command handler with too " +
                         $"many arguments at {argsCount}. The maximum supported " +
                         $"number of arguments to a command handler is 6.");
            break;
    }
}
@dogboydog
Copy link
Collaborator

Thanks. I do wonder if it might be possible to inspect a lambda so that its argument list could still be found. But if not this kind of error handling would be good to add.

@misha-cilantro
Copy link
Author

I did try to inspect it with Rider, and even though you do get a Callable object, every property is null. There's no Delegate to call or get a MethodInfo out of. It seems to just be an empty Callable essentially. Though, of course I could be missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants