Generic Typed GetNode C#

[hasahmed]

This is really a feature request and I think it may have been sort of touched on here #16309. Anyway GetNode("path/to/mynode") will return a Node instance and will likely need a cast (i.e. GetNode("path/to/mynode") as Sprite2D), and often you will want to modify a property of the node so you will need parentheses (i.e.(GetNode("path/to/mynode") as Sprite2D).Position = foo();) I was thinking that this could be made a little more pretty by passing GetNode the type i.e. GetNode<Sprite>("path/to/mynode"); and then the need for casting (and by extension parentheses) could be avoided.

[mysticfall]

I agree that we need such a feature and it's also a safer one to have before attempting more complex approaches, like the one described in #16309.

However, it's my understanding that the current system auto generates the C# binding source from an XML metadata. So, I believe we'd need to find a way to customize the process(#15548) before we could deal with such a problem.

In the meanwhile, we can use C#'s extension method to mitigate the problem as I've done for my project:

• https://github.com/mysticfall/AlleyCat/blob/master/Program/AlleyCat/Common/NodeExtensions.cs

[akien-mga]

CC @neikeq

[paulloz]

This is massively asked for on Discord (maybe on other communication platforms too, I don't know).
Even if this is not a blocking issue, the current usage of .GetNode(NodePath) feels weird and really is not right. Needing to cast the function return is basicaly transforming what would be compilation errors to unexpected runtime errors. Correcting this (as soon as possible) would be a good first step toward having more C# users, leading to more tests.

Not the best solution but: adding a small NodeExtension class to the glue would do the trick, even temporarly while a better option is chosen (i.e. making some classes generated from ClassDB partial so they can be extended).

[NathanWarden]

I like this idea. It still, in spirit, matches the GDScript API. It would be super easy to implement as well since it can essentially just be:

public T GetNode<T>(NodePath path)
{
    return GetNode(path) as T;
}

Although I haven't double checked, I'm pretty sure it's safe even if the original GetNode returns null due to no node existing at the path given.

[paulloz]

Well, you have to where T : Node or at least where T : class or it won't compile, but yes.
It is safe even if GetNode returns null (null as XXX is null).
I'm using the branch I linked above as my custom build for some time now without any issue to report.

[neikeq]

Should this generic method use a cast or as? I think it should be the former, since I cannot see this being used without assuming the type (unless followed by ?). Whatever we choose, we can also provide an alternative (GetNodeOrNull or GetNodeOrThrow).

[paulloz]

I personally tend to prefer as but I guess both have their advantages and inconveniences (and the most important is that a working solution is found 😄).
+1 for the alternative method GetNodeOrNull / GetNodeOrThrow.

[mysticfall]

I definitely agree that it's better to provide two different versions for a mandatory and optional dependency respectively.

As to the type, I'd use cast since I think it's better to design API unambiguously, so instead of returning null for both cases when it fails to find the requested node and when the node is of a wrong type, I prefer showing which is the case explicitly by the contract.

Of course, we can use as and add additional check for type safety but it'd be redundant since explicit cast would throw an appropriate exception in that case any way.

Probably the only real reason why we'd rather use as could be from a performance perspective, assuming it's better not to use exceptions anywhere. But I don't think such type of optimization is necessary, especially considering it's an error condition and people can always use something like GetNode<Node>(path) as ExpectedType and do a manual check if they are using it in a normal execution path and performance is of utmost importance.

[NathanWarden]

I'd just keep it simple and use as. Having two methods makes it more convoluted where only a knowledged programmer would know the difference between the two. But, if that same programmer writes their code properly, then this shouldn't constantly be called anyway since the programmer should just cache it once in the Ready function, so there shouldn't be any noticeable performance hit in a great majority of cases.

If that same well versed programmer has a specific use case where they're calling something like this every frame for some strange reason they can always make their own extension method that throws an exception.

So, I really don't see any reason why we should have two generic methods that do the same thing, but slightly differently. Just implementing the safer of the two seams more reasonable.

[neikeq]

Probably the only real reason why we'd rather use as could be from a performance perspective, assuming it's better not to use exceptions anywhere.

Raising the exception is more costly. However, if we go with a cast, it's assumed you know the object can be cast, therefore as would be slower because of the safety check. I don't think we should worry too much about this though. I think the performance penalty is quite small, and GetNode is already quite slow so you want to call it only once (on _Ready for example) and cache the result.

and people can always use something like GetNode<Node>(path) as ExpectedType

You mean GetNode(path) as ExpectedType :stuck_out_tongue:

[mysticfall]

@NathanWarden The reason why we'd may want two different methods is that it's more than likely the API would be used to resolve required dependencies in such places like _Ready().

In that case, if we don't check for the exception path, it'd be passed to the user's responsibility to validate the result. However, if they have to check for a null value or type safety every time they invoke GetNode, I feel like it's defeating the purpose we consider providing such a shortcut for.

And obviously, we can't simply give them a version with such validation checks, since it's still possible they might want to use the API to resolve optional dependencies.

Personally, I don't see much problem why we shouldn't provide two different versions of such methods, especially when we consider that C#'s core API often provides many different overrides for the same functionality, and also the fact that it's quite common to see similar APIs tends to provide such alternatives like GetOrElse, or GetOrDefault to make it easier to write concise code and to use it inside a functional composition (i.e. Rx).

@neikeq Yeah you're right, I'm still clunky with C#, especially when I'm not using an IDE 😅

[mysticfall]

And as to the bound check, I'd suggest just to use where T : class rather than T : Node since it might be needed to use the API to resolve interfaces.

[neikeq]

I'd just keep it simple and use as. Having two methods makes it more convoluted where only a knowledged programmer would know the difference between the two.

The reasons I prefer a classic cast is that most of the times you use GetNode -and specially for this use case- you know the type beforehand.

I think the name of these methods is clear enough, and they would also have a description, so it won't be confusing.

If that same well versed programmer has a specific use case where they're calling something like this every frame for some strange reason they can always make their own extension method that throws an exception.

So, I really don't see any reason why we should have two generic methods that do the same thing, but slightly differently. Just implementing the safer of the two seams more reasonable.

This is a very common use case, so I think it's better to provide it ourselves rather than make the users define their own extension methods.

[NathanWarden]

@mysticfall

if we don't check for the exception path, it'd be passed to the user's responsibility to validate the result

The problem with this conclusion is that if you want safety in your code you have to do error checking regardless of which one you use. If it throws an exception you need to use a try/catch or if it passes null you'll need an if statement. The experienced C# developer will know what to do with as or a straight cast. The new C# programmer won't know what to do in either case and will probably ask someone more experienced, in which case the as version is a much more common (and much faster) method of error checking.

The reason why we'd may want two different methods is that it's more than likely the API would be used to resolve required dependencies in such places like _Ready()

This really only validates my point, there is very little performance implication in going with the slightly slower as if almost all uses of this function are in the Ready function, which is most likely the case. The only case that it will likely be used in something like the process function it will likely be doing a null check against whether it has a spawned node or not, in which case you'd still want to use the as version since a try/catch would be a pretty big performance hit if you have to do it every frame.

IE:

public override void _Process(delta)
{
    if (nodeThatGetsSpawned == null)
     {
          nodeThatGetsSpawned = GetNode<NodeType>(path); // as version
     }
}

vs.

public override void _Process(delta)
{
    if (nodeThatGetsSpawned == null)
     {
          try
          {
               nodeThatGetsSpawned = GetNode<NodeType>(path); // try/catch version
          }
          catch {}
     }
}

I still really don't see any significant reason why we need two different methods as it convolutes the API with very little grounds to do so with.

[NathanWarden]

@neikeq

This is a very common use case, so I think it's better to provide it ourselves rather than make the users define their own extension methods.

As in my post just above, I realized using a straight cast version every frame would actually be pretty big performance hit since you'd be forced to use a try/catch statement, in which case you'd still want to use the as version.

[paulloz]

@mysticfall GetNode will always return a class inheriting Node though, or at least Godot.Object.

[NathanWarden]

@paulloz I agree. Even if it didn't have to return a Node, using class would cause the API to do something other than what it explicitly tells you it does, which violates the rules of good API design.

[mysticfall]

@NathanWarden

If it throws an exception you need to use a try/catch or if it passes null you'll need an if statement. The fact that a certain method throws an exception doesn't mean the caller must catch it with try-catch.

Normally it's better to let it propagate further up the hierarchy and be handled by some global exception handling logic, like logging to a file or showing an exception dialog, for example.

The reason why it is better to throw a specific exception instead of returning null for both the normal and exception paths is that it prevents such cases where it'd trigger an unexpected behavior further down the code as a side effect, like causing NullReferenceException in a different line, thus making it more difficult to track down the actual cause.

This really only validates my point, there is very little performance implication in going with the slightly slower as if almost all uses of this function are in the Ready function, which is most likely the case.

Sorry if I wasn't very clear with my previous comment, but performance wasn't my primary concern and I just mentioned it as a hypothetical argument that might be put against my preferred approach.

@paulloz

GetNode will always return a class inheriting Node though.

It's true, but still there can be such cases where we'd want to reference a node by its interface, rather than by its concrete type.

Suppose we have a class which reads user settings, but in different ways according to the configuration. We can implement this by dynamically adding a child node which implement IConfigurable interface to the class. And while it's possible we can make an abstract class like ConfigurableNode : Node, there's no reason to make it mandatory.

Also, it's often considered better to program against interfaces rather than actual types, and I don't think it's better to exclude such cases where there are other classes that also represent something configurable, but not actually a node.

[paulloz]

@mysticfall I'd advise to use the non generic GetNode in this case and let the user cast the returned value as they want. So we can have a strong compilation error when GetNode<T> is used with a wrong type in the general use case.

And @mysticfall @neikeq: I think you convinced me that a () cast would be better than using as here.

[mysticfall]

@paulloz It sounds reasonable to me. I don't find it to be as importance an issue as whether or not we should provide different methods to resolve nodes anyway, so I wouldn't object if that's what the other people consider to be a better option.

[NathanWarden]

@mysticfall

The reason why it is better to throw a specific exception instead of returning null for both the normal and exception paths is that it prevent such cases where it'd trigger an unexpected behavior further down the code, like causing NullReferenceException in a different line, thus making it more difficult to track down the actual cause.

But, needing to know whether the exception was thrown because the node doesn't exist at all or if it's just the wrong type will be a very rare case. In most cases the person making the game will know the reason, and if they don't it's not hard to debug.

It still seems to me that needing a straight cast is mostly hypothetical and might want to be used in a few rare cases, in which case why not just use (MyType)GetNode(path) as it has far less typing than GetNodeOrThrow<MyType>(path)

Also, using as for GetNode<T>(path) where it just returns null is consistent with GetNode(path) where it returns null.

It's true, but still there can be such cases where we'd want to reference a node by its interface, rather than by its concrete type.

But, if you need this then you can just assign it to the non concrete type. IE. Object myObj = GetNode<Node>(path);

[mysticfall]

@NathanWarden

But, needing to know whether the exception was thrown because the node doesn't exist at all or if it's just the wrong type will be a very rare case. In most cases the person making the game will know the reason, and if they don't it's not hard to debug.

But what about the possibility that it could trigger a side effect? It's the same reason why we use assertions or argument validations to make the code fail immediately when a requirement is not met, rather than let it flow and cause unexpected secondary problems further down the road.

And even such a case where the developer needs to distinguish between a missing node and a node with a wrong type, it's still a valid use case while having multiple overrides or different versions of the same functionality is a commonly used practice.

So, unless we have any practical reason to keep the number of methods as few as possible, I don't see any reason why we should ignore a valid use case or best practice to save a few lines of a class that users don't need to edit.

[NathanWarden]

But, the only side effect would be that the node was null, in which case you'd get an exception thrown anyway when you try to reference it.

[mysticfall]

@NathanWarden It can be a few lines down, or a few method calls apart. And it's not even always the case that it causes NullReferenceException on the same reference, since the exception can be thrown by some other variable that fails to initialize because of the missing node, and it can also trigger a different branch without throwing an exception.

[NathanWarden]

If you're trying to reference the type is will always be a NullReferenceException

There's no way it's because it failed to get a value unless you did something like this:

MyType node = GetNode<MyType>();
int valueToGet = 0;

if (node != null)
{
     valueToGet = node.GetValue();
}

DoSomethingWithValue(valueToGet);

You have to explicitly ignore your own error checking code for your scenario.

[mysticfall]

@NathanWarden I don't understand why you'd want to force users to do if-checks every time, if the reason why you're opposing the idea of adding additional methods to the API is the verbosity.

In my opinion, it's much better to save the users' work instead of that of the developers if we have a choice.

And if you can't think of any case where returning null would cause confusion, please consider the following scenario:

var node = GetNode<MyType>(path);

// potentially more codes here.

// 'otherValue' also can be null.
if (node.GetValue() == "A" || otherValue.GetValue() == "B") {
//...
}

Or this :

MyType node = null;

if (someCondition) {
    node = GetNode<MyType>(path);
}

var value = node?.GetValue();

If value is null, how do you know if it was caused by a missing node rather than by someCondition being false?

There are numerous other cases that might obfuscate the real cause of the problem with returning null instead of throwing an exception.

And one of the reasons why we have exceptions in the first place is because it is generally accepted that it's better to explicitly notify the caller something is wrong than letting programmers use arbitrary values like -1 or null for error conditions and rely on manual checks.

[neikeq]

It still seems to me that needing a straight cast is mostly hypothetical and might want to be used in a few rare cases, in which case why not just use (MyType)GetNode(path) as it has far less typing that GetNodeWithException<MyType>(path)

Because we need a readable and convenient to write syntax for chaining a call to the returned node. e.g.:

• ((MyType)GetNode(path)).DoSomething()
• GetNode<MyType>(path).DoSomething()

The former is less readable because the amount of parenthesis, and less convenient to write because you would usually write GetNode( first (or (MyType)GetNode() and then go back to surround the cast result in order to be able to call the method. It's easier to refactor code with the latter for the same reason.

Regarding the InvalidCastException vs NullReferenceException, @mysticfall explains it well. The correct way is to throw the exception right where the problem happened.

We should also use the appropriate exception for the error. A NullReferenceException doesn't say much about the cause being an invalid cast, in fact it could make the user believe it is caused by something else. This applies even more for GetNode, which returns null when no node is found.

public override void _Process(delta)
{
    if (nodeThatGetsSpawned == null)
     {
          try
          {
               nodeThatGetsSpawned = GetNode<NodeType>(path); // try/catch version
          }
          catch {}
     }
}

The problem here is you are using the wrong tool for what you want to do. You can write the above code like:

nodeThatGetsSpawned = GetNode(path) as NodeType;
if (nodeThatGetsSpawned != null) // ...

or

GetNodeOrNull<NodeType>(path)?.DoSomething();

[NathanWarden]

I will give it over to throwing an exception team :)

I'd still prefer to have a less convoluted API though. If at all possible, can we please just keep it as GetNode<Type>(path) where it will throw an exception instead of having multiple methods like GetNodeOrThrow and GetNodeOrNull. There are other C# standard ways to handle this with as and null checking if this is what we want to do.

IE:

if (GetNode(path) is MyType myType)
{
     myType.DoMyTypeStuff();
}

I just think adding multiple methods to the API when it could be done manually without much issue is bloating the API and making it inconsistent with the core API revealed via GDScript. If I'm not mistaken this is one of the Godot PR rules also that if it can be done via scripting to not add it.

[paulloz]

Side note: is this the full list of methods needing the same treatment?

• GetNode(NodePath)
• GetChild(int)
• GetOwner()
• GetParent()

[mysticfall]

@NathanWarden I'd like to point out how C#'s core API is handling such an issue, like for example, IEnumerable interface as shown below:

• https://msdn.microsoft.com/en-us/library/9eekhta0(v=vs.110).aspx#Extension%20Methods

You can see how it provides many useful extensions, including several overrides of the same method, or similar versions that do it in a slightly different ways, like Last and LastOrDefault (something very close to what we are intending to do).

Maybe we can consider keeping the Node API as concise as possible and provides many utilities as extension method, as it seems to be the standard approach in C#'s core API.

But I don't think we shouldn't include anything that if users cannot do it themselves. If it were the hard and fast rule, we probably need to remove about 2/3 of methods exposed by such classes like Vector3 or Transform.

And even though I agree that it's better to keep the codebase less convoluted, if it comes to a question whether we should add a few methods to the core API or we should make every users of the platform to spray plenty of null checks everywhere in their individual projects, I don't see any reason why we should regard the latter to be a more desirable situation, especially considering the former is pretty much a common practice that's used by the core API of C# itself.

[NathanWarden]

You can see how it provides many useful extensions

That's fine, I'm not against using extension methods at all if they're useful :)

But I don't think we shouldn't include anything that if users cannot do it themselves.

I agree, if it helps produce a project faster I'm all for it. I just don't think it makes sense to provide two additional generic versions of methods that do essentially the same thing when a person can easily add the other one (probably as an extension method) if they really want it.

So, I propose we just use the GetNodeOrThrow version, but just call it GetNode<T>(path) and anyone else can implement the GetNodeOrNull on their own.

[PJB3005]

Instead of a GetNodeOrNull, another option might be a bool TryGetNode<T>(string path, out T node), which follows the classic Try* pattern of using an out parameter and returning it if was written to successfully. You can still use it for assignment if you don't care about the possibility of null, but of course you can't do a chain like GetNode().Thing().

Also, a fundamental issue with extension methods is that they don't work if you're using implicitly this.:

public class Foo : Node
{
    public override void _Ready()
    {
        // Compiler error due GetNode not being generic, it's not finding the extension.
        GD.Print(GetNode<Sprite>("Sprite2D"));
    }
}

public static class Exts
{
    public static T GetNode<T>(this Node parent, string key) where T : Node
    {
        return (T)parent.GetNode(key);
    }
}

whereas this.GetNode<Sprite>(...) does work.

[paulloz]

Also, a fundamental issue with extension methods is that they don't work if you're using implicitly this.

That's one reason why I was saying it could be a temporary solution.

[maikramer]

I dont like the idea of using any kind of strings because if i change the name of the node in tree or use a refactor, i have a exception. This is what i use in my game, then if anybody likes the idea, can be used. Of course this is slow but is called only one time in _Ready()

public static T FindClass<T>(this Node node) where T : Node
        {
            var children = node.GetChildren();
            foreach (Node t in children)
            {
                if (t is T child)
                    return child;
                if (t.GetChildCount() > 0)
                {
                    var ret = t.FindClass<T>();
                    if (ret != null)
                        return ret;
                }
            }

            return null;
        }

Then i find what i want like this

//Extreme example (very slow)
var gameManager = GetTree().Root.FindClass<GameManager>();

//Normal example
var playerController = player.FindClass<Controller>();

[paulloz]

I think it's not a good habit to iterate over every node to find what you're looking for.
In addition, there's currently a correlation between the node name in the tree and the class name in the script. If you changed one you'll need to change the other so... Using the node name as a string or the type is pretty much the same thing.

[maikramer]

@paulloz Yes I understand, even if you are not forced to, is a good practice in Godot right? I could use maybe. After your commit.

node.GetNode<Car>(nameof(Car));

Or do you suggest something else?

[mysticfall]

@maikramer This is a version I use :

[NotNull]
public static T GetChild<T>([NotNull] this Node node) where T : class
{
    switch (node.GetChildren().FirstOrDefault(n => n is T))
    {
        case T result:
            return result;
        default:
            throw new InvalidOperationException(
                $"Unable to find node of type '{typeof(T)}' in '{node.Name}'.");
    }
}

However, I think it's something we may consider providing in addition to the API we've discussed above rather than replacing it. Often, we might have several children with the same type and other times it could be undesirable to iterate over many nodes, as @paulloz already mentioned.

[reduz]

I personally think throwing exceptions for this is likely not a good idea, given the Godot C++ API won't throw exceptions anywhere else. It will be completely unexpected and unintuitive.

I root for GetNode and GetNodeOrNull but that's it.

[paulloz]

I root for GetNode and GetNodeOrNull but that's it.

@reduz I don't see the difference between those two functions if none is throwing an exception 🤔

[reduz]

@reduz I don't see the difference between those two functions if none is throwing an exception

Because Godot is designed to not throw exceptions, instead it logs the errors and tries to recover as best as possible.

This is by design to ensure your game does not crash for a stupid mistake and remains stable once published, even if common errors happen.

In this case, GetNode will log the error and GetNodeOrNull will not.

[mysticfall]

@reduz I can understand how other languages handle it differently, but it is a quite expected and intuitive behavior in the context of C# API though, as it mimics the way IEnumerable.First / FirstOrDefault works exactly, which is one of the often used core APIs by C# developers.

And I believe it'd be better to provide an API that is idiomatic to each specific languages rather than a rough translation of Godot's C++ API. Aside from exceptions, GDScript doesn't have a concept of an interface too but I don't think we shouldn't use interfaces in our C# API simply because they are not used by some other bindings.

I think there could be more chance for a typical developer who wants to develop a Godot game using its C# API to be already familiar with C#'s conventions and its core API, than to be so with Godot's C++ API.

[paulloz]

I understand, however (correct me if I'm wrong) there is only a get_node function in GDScript that'll return a null instance and always log an error if the requested Node does not exist.

[reduz]

@paulloz yeah, i always thought about doing a get_node_or_null in GDScript too, so you avoid the has_node()

@mysticfall As I mentioned before, this is done by design, and if you get used to it you will find it to be a priceless feature, it's unrelated to idiomatic things because C++ also has exceptions and we don't use them.

And even in the case you still wanted exceptions, you have to understand that Godot is not a C# engine, it only provides C# bindings. If you want it to have idiomatic consistence with C# and all functions that can fail return exceptions, it would be needed to be done manually for every function in the C# bindings, at a huge human and performance cost. It's just not worth it either way.

[neikeq]

I actually like @maikramer's FindClass<T>() method without the mask parameter (it should include the parameters bool recursive = true, bool owned = true though). It's too slow though, because you are iterating the tree in managed code just to check if the node has a script of type T, and that will require marshaling, which creates a managed instance for each node iteration, even when these nodes don't have a C# script. I would implement it in native code and add it to the NodeExtensions.

In addition, there's currently a correlation between the node name in the tree and the class name in the script. If you changed one you'll need to change the other so...

No, the correlation is between the file name and class name, e.g.: Controller.cs and Controller.

[paulloz]

@neikeq Obviously, my bad I got confused 😰.

[mysticfall]

@reduz Personally, I do not agree that not throwing exceptions is a better way in every cases (i.e. how can it be possible to recover when the game can't find GameManager because of a typo?).

But if someone believes it to be so, what prevents him or her from using GetNodeOrDefault instead of GetNode everywhere? If it's a recommended practice in Godot, then we may put it in the documentation to advise users to prefer GetNodeOrDefault, or we may even flip the behavior so GetNode wouldn't throw an exception but something like GetNodeOrFail would.

And I don't see much need to provide two different versions if all the differences they have is whether to log the error. In that case, we would be giving a false impression to C# developers that GetNode would throw an exception by providing GetNodeOrDefault because the pair of such names will remind them of core C# APIs which behave that way.

So, if @reduz is so against the idea of throwing an exception anywhere in C# API, I'd suggest to drop the whole idea of providing two different versions of GetNode and let people who think differently to provide something like GetNodeOrFail for their own projects. I think not following a common C# convention is one thing but misleading users is a whole different matter.

[reduz]

@mysticfall well, get_node_or_null makes sense in the context of efficiency mostly, not the error or exceptions one to me. if (has_node): get_node() is less efficient than get_node_or_null(). This is not a specific C# related problem from my side, so I think different issues are being mixed up here.

[neikeq]

The problem with the exceptions in #17720 (except the InvalidCastException which is fine) is that they work differently than the non-generic methods. GetNode<T> throws NodeNotFoundException and GetChild<T> throws IndexOutOfRangeException, but GetNode and GetChild don't throw anything. GetParent<T> and GetOwner<T> shoudn't even throw a NodeNotFoundException IMO. It would be different if it would be explicit in the name that an exception can be thrown, like GetNodeOrThrow<T>. The generated methods won't be made to throw exceptions. That would require hard-coding each of these methods in the bindings generator. It's also confusing that just that small subset of the generated API can throw exceptions.

I'm fine with having the non-generated API throw exceptions as long as it's documented and it doesn't cause confusion like the previous examples.

[mysticfall]

@reduz Yeah, I assumed to be so, and I agree that it'd be a good thing to provide the most efficient way to do things, although I somehow thought turning off the debug messages in a release build could be a common practice.

I think now I'm convinced by @neikeq's arguments that it's better to ensure generic and non-generic versions of the same method to behave in the same way.

So, what about flipping the behavior so as to make GetNode<T> to return null like its non-generic counterpart, while providing an alternate version as GetNodeOrFail or GetNodeOrThrow which throws an exception?

Personally, I believe it's good to provide a version with a fail fast behavior in the case of a missing required dependency, and also I agree with @neikeq that it's ok to use exceptions at least in the C# specific APIs.

But if we decide we shouldn't do that, I think we better avoid reusing the same naming convention as DoSomething / DoSomethingOrDefault in a different context than it is commonly used in the core C# API to prevent potential confusion.

[PJB3005]

@reduz

And even in the case you still wanted exceptions, you have to understand that Godot is not a C# engine, it only provides C# bindings. If you want it to have idiomatic consistence with C# and all functions that can fail return exceptions, it would be needed to be done manually for every function in the C# bindings, at a huge human and performance cost. It's just not worth it either way.

I think this is a pretty weak argument. If we're gonna have C# support in Godot, we should put the effort in to make it good. Unless of course the plan is to follow @mysticfall's idea and let a third party provide a proper API for Godot via C#.

Unrelated:

Also, I do agree that the generic method should be purely the non-generic method + cast. If we make GetNode<T> do a nullcheck, GetNode should too.