godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
87.2k stars 19.6k forks source link

Mono editor plugins can't cast non-tool C# types #36395

Open scambier opened 4 years ago

scambier commented 4 years ago

Godot version: 3.2.stable

OS/device including version: Windows 10 Pro

Issue description:

With the following hierarchy unknown

Game (Control) and ToolGame (Node2D) have their own C# classes with the same names. ToolGame is a [Tool], and makes a GetParent<Game>() call, that works in-game, but fails in-editor with the following error:

 modules/mono/mono_gd/gd_mono_utils.cpp:357 - System.InvalidCastException: Specified cast is not valid.

The same message is shown between 3 and 5 times. The exact same structure and code in Gdscript works as intended.

Steps to reproduce:

Create a scene like the one in the screenshot, and add corresponding classes.

ToolGame.cs contains this minimal code:

using Godot;

[Tool]
public class ToolGame : Node2D
{
    public override void _Ready()
    {
    }

    public override void _Draw()
    {
        var control = GetParent<Control>(); // Fine
        var game = GetParent<Game>(); // InvalidCastException
    }
}
scambier commented 4 years ago

Update from someone on the Discord:

If you have a Tool class, and refer to another class (in your case Game), it won't work unless Game is also a Tool class. If it's not, it appears as it's superclass (i.e. Node/Spatial/etc). Basically the editor has no knowledge of your Game class

And indeed, setting my Game class as a [Tool] "solves" the problem.

DeathTBO commented 4 years ago

This is probably the same issue I was having: #36192

Is there any drawback to marking something as [Tool]?

scambier commented 4 years ago

If the question is for me: yes, separation of concerns. I want my [Tool] classes to exclusively contain code related to the editor, and not mix-and-match game logic and editor logic. That's also why, in my example, I have a child node ToolGame on my Game node

endlesstravel commented 4 years ago

I also encountered this inexplicable prompt. It turned out to be [Tool] problem. Maybe it's better to modify the error message of the prompt .

endlesstravel commented 4 years ago

I realized that the key of problem on the other side was that Godot didn't output the error information of C# completely.

what godot output error info in editor, I can't find useful information from it.:

modules/mono/mono_gd/gd_mono_utils.cpp:357 - System.InvalidCastException: Specified cast is not valid.

Here's the message I got using try-catch, so I can locate the problem in GooOrganism.cs:17 at least:

System.InvalidCastException: Specified cast is not valid. at (wrapper castclass) System.Object.__castclass_with_cache(object,intptr,intptr) at Godot.Node.GetNode[T] (Godot.NodePath path) [0x00001] in /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/Extensions/NodeExtensions.cs:7 at GooOrganism._Ready () [0x0000a] in D:\GameWorkSpace\xx\godot_project\scene_card\GooOrganism.cs:17

geneishouko commented 4 years ago

I have a minimal test case that helps reproduce the issue. CSharpCastTest.zip I wanted to open a new issue because of this test case.

Can we rename the issue so that it may get proper attention? Something like "Mono editor plugins can't cast non-tool C# types"

cgbeutler commented 2 years ago

As noted in the duplicate #52729, this can trigger in the default constructors called by the Editor. I saw it with GD.Load<CustomResource>(...). I'm pretty sure this error is caused by the scripts not being available to the editor. When using [Tool] mode, that means the engine can't instance any class that is not in tool mode. Even more tricky, the default constructors called by Godot to build mono's default objects can throw if also trying to instance non-tool mode classes.

There's a chance that the tool classes referring to non-tool classes is actually sorta expected behavior. I mean, Tool is kinda meant to say "The editor will need this". Could probably use better errors, though.

The one that seems more questionable to me is the case highlighted in #52729, as classes not marked with Tool are still having their default constructors run by the editor before all the classes are available to it. I have only seen it in custom Resources, though. Pretty sure Resources are meant to all be marked Tool anyway, since Resources are meant to be used in-editor.

I'm still trying to understand this issue, so sorry if some of that was wrong.

DissonantVoid commented 2 years ago

just encountered this problem today, is this going to be fixed in V4.0 ?

scambier commented 2 years ago

(More than a year later, I renamed the issue, according to @geneishouko's proposition)

Drkie commented 2 years ago

Running into this right now trying to add a button to the inspector for a non-tool class.

I don't think adding [Tool] to classes we want to use from a plugin is an acceptable solution, since that tells the Godot to run the node in the editor (executing _Ready(), _Process(), _Draw() and so on), which is not desirable most of the time. In my case, I have hundreds of such custom nodes and sanitizing them in order to not crash the editor would be a monumental task, not to mention it would make the code dirty with constant checks against Engine.EditorHint.

My particular case:

// Code inside an EditorInspectorPlugin
public override bool CanHandle(Godot.Object @object)
{
    if (@object is MyCustomType) // This fails if MyCustomType doesn't have the [Tool] annotation,
                                 // even if MyCustomType derives from a [Tool] class and even if 
                                 // MyCustomType was registered by the plugin using AddCustomType() 
        return true;
    // Etc...
    return false;
}

There's a proposal (https://github.com/godotengine/godot-proposals/issues/900) that suggests having a secondary annotation for this purpose (perhaps [Plugin]), but I don't think an annotation should be required at all in order for plugins to see custom types (especially if those types were already registered with AddCustomType()).

romifauzi commented 1 year ago

I have also encountered this issue just after upgrading to 4.0, in my case, I want to assign a Custom Resource (that is not marked with [Tool]) to a inspector slot from a script that is marked with [Tool]. It was working just fine on 3.5.

Nosh-Ware commented 5 months ago

I have also encountered and have to work around this. Is there any way for this to be noted in the documents or made clear in the editor? I find it quite difficult when it comes down to making scripts that act as UI, as its usually quite useful to have that UI run in editor, and this UI need to reference a resource or node that I would rather not have running in the editor, and in some cases this might even require me to make multiple other unrelated scripts tool scripts as well just for the ability to have these nodes be assigned via the editor.

MedusaZenovka commented 2 months ago

I also encountered the problem and adding [Tool] solved it. Since I don't use methods like "_Ready" or "_Process" to run my game, the drawback of this solution is not an issue for me. Should still not be required though.

Ikaroon commented 2 months ago

[Tool] Should definitely not be required for a node to be referenced in a plugin. Sometimes you just want to visualize a node in the 3D view like for example a grid or box, whatever, and you don't want these classes to run during the editor for a good reason.

A workaround that doesn't include [Tool] is to load the script and test for that instead of the C# type and then get and set the properties using the variants:

private static readonly Script ScriptResource = GD.Load<Script>("res://ScriptPath.cs");

public override bool _HasGizmo(Node3D node)
{
    return node.GetScript().Obj == ScriptResource;
}

public override void _Redraw(EditorNode3DGizmo gizmo)
{
   var node = gizmo.GetNode3D();
   var val = node.Get("ExportedPropertyName").As<ValueType>();
   node.Set("ExportedPropertyName", val);
}