Add OnReady annotation to C#

[DevinPentecost]

Describe the project you are working on

Any game project written in C#

Describe the problem or limitation you are having in your project

In many cases, a scene has known relatives in the tree which need to be referenced, but cannot be referenced until after the scene is ready. To do this, references would have to be set during the _Ready() function. However, GDScript has a shortcut for this utilizing the onready keyword. This helps a lot with improving the class functionality organization. It prevents the _Ready() function from being clogged with a bunch of property sets that search for nodes.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a [OnReady] annotation (similar to [Export]) that marks a class field as being assigned during the OnReady step of the scene setup.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Adds a new annotation that adjusts the initialization. An example would look like:

[OnReady] private MyOtherNodeClass _otherNode = GetNode<MyOtherNodeClass>("OtherNode");

I'm not an expert enough on C# to know how to implement this, but I will update this Issue when/if I figure out more details or someone else can help provide them.

If this enhancement will not be used often, can it be worked around with a few lines of script?

There is currently a workaround, but it makes the code messier and does not match existing GDScript syntax.

private MyOtherNodeClass _otherNode;
public override void _Ready() {
    _otherNode = GetNode<MyOtherNodeClass>("OtherNode");
    ...
}

Is there a reason why this should be core and not an add-on in the asset library?

It is an addition to the built in Godot C# functionality and mimics existing GDScript behavior.

[31]

[OnReady] private MyOtherNodeClass _otherNode = GetNode<MyOtherNodeClass>("OtherNode");

I'm not an expert enough on C# to know how to implement this

As far as I know this is not possible: field assignment is evaluated in the constructor, and _Ready gets called much later (if/when the node ends up getting added to a tree). Also, field default value assignment happens in a static context--can't use this, like a this.GetNode<T>() call.

---

For a more compact version of the workaround, I wrote a library (C# source generator, https://github.com/31/GodotOnReady) that lets you write this instead, and it generates the _Ready method:

[OnReadyGet("Other")] private MyOtherNodeClass _other;

[DevinPentecost]

Hi,

I played around this but then hit a wall. Maybe what I'm trying to do is impossible but here's what I have gotten.

First, we have the following scene:

I have created the following two classes. The first is the script attached to the root Node:

using Godot;

namespace Grid
{
	public class TestNode : Node
	{
		[OnReadyGetNode("Child")] public Sprite SomeNode;  // This one is as expected
		[OnReadyGetNode("Missing")] public Node MissingNode;  // This node doesn't exist, so it will stay as `null`
		[OnReadyGetNode("WrongType")] public Spatial WrongType;  // This node is declared as a `Spatial` but is actually a `Timer` in the scene, so it will stay as `null`

		public override void _Ready()
		{
			base._Ready();
			this.GetOnReadyNodes();
		}
	}
}

We can see it has several Fields with a OnReadyGetNode(...) attribute.

This Attribute, and the GetOnReadyNodes() extension, are defined in the second file:

using System;
using System.Linq;

namespace Godot
{
	public class OnReadyGetNode : Attribute
	{
		public OnReadyGetNode(string nodePath)
		{
			NodePath = nodePath;
		}
		public string NodePath { get; }
	}

	public static class OnReadyGetNodeExtensions
	{
		public static void GetOnReadyNodes<T>(this T node)
		{
			var converted = node as Node;
			if (converted is null)
			{
				GD.PrintErr($"Object {typeof(T)} is not a superclass of {typeof(Node)}. Unable to assign OnReady Nodes");
				return;
			}
			
			var annotatedFields = typeof(T).GetFields()
				.Where(info => info.IsDefined(typeof(OnReadyGetNode), false));
			foreach (var annotatedField in annotatedFields)
			{
				//Retrieve the NodePath
				var nodePath = annotatedField.GetCustomAttributes(typeof(OnReadyGetNode), false).Cast<OnReadyGetNode>().First().NodePath;
				var targetNode = converted?.GetNode(nodePath);
				
				//If we couldn't get anything, let the user know
				if (targetNode is null)
				{
					GD.PrintErr($"Unable to find Node at path {nodePath}");
					continue;
				}
				
				try
				{
					//Set the field
					annotatedField.SetValue(node, targetNode);
				}
				catch (ArgumentException exception)
				{
					//The node we found was of the wrong type
					GD.PrintErr($"Found Node at path {nodePath}, but was of the incorrect type. {exception.Message}");
				}
			}
		}
	}
}

Everything works fine, and I have implemented basic error handling for when the types mismatch or the Node is not available at the given path.

However, the major problem I'm stuck on is running the GetOnReadyNodes() extension. In its current state, any superclass that uses the OnReadyGetNode Attribute would have to call the function manually. I don't know how to pull this down to the Node _Ready() base function while preserving the generic type T.

Ideally, the base _Ready() function in the Node class would call GetOnReadyNodes() which would then handle retrieval. It would then no longer need to be included in every .cs class.

If anyone has any ideas on how to move forward they are greatly appreciated! :)

[31]

I played around this but then hit a wall.

I'm not sure this is the right place to ask about this--it sounds like you have a fairly broad C# question.

Ideally, the base _Ready() function in the Node class would call GetOnReadyNodes() which would then handle retrieval. It would then no longer need to be included in every .cs class.

The way I fixed this in GodotOnReady (without changing the Godot Engine source code) is the generator writes the _Ready override method for each node that uses [OnReadyGet]. (And then I went a little further and got rid of all reflection. 😄)

[cgbeutler]

I usually do a lazy-getter using c# 8's ??= operator. (I also have nullable reference types turned on.)

private MyNode? someNode;
protected MyNode SomeNode => someNode ??= GetNode<MyNode>("SomeNode");

Course, you still have to make sure not to call the property too early.
As a side benefit, this can help when hot reloading scrubs away the variable.

[31]

I just noticed this was added to the 4.0 roadmap/project/milestone 19 days ago, and I'm curious--what is the plan for the API? What will it look like to use this feature in 4.0?

[DomiStyle]

Out of interest, since this is on the 4.0 roadmap but #5216 is not, which is the preferred syntax for the Godot implementation?

A variant of #5216:

[OnReady("Some/OtherNode")]
    public Node OtherNode { get; set; }

From the original post:

[OnReady]
    public Node OtherNode { get; set; } = GetNode<Node>("Some/OtherNode");

I personally really like the first variant but since I can't use GodotOnReady because they have hit a roadblock for their Godot 4 implementation (31/GodotOnReady#49 (comment)), I'm looking for an alternative.

I assume implementing this directly in Godot would still be feasible?

[cphillips83]

I came here to actually create this proposal until I came across this one. I'm not sure about the GodotOnReady roadblock, but this is how I implemented it in my source generator.

Please note that this is code thrown together and can/will fail in scenarios (things like nested classes, etc).

First I decided to hook in to TreeEntered event via a class constructor as I was having issues with the override variation not firing (assuming another source gen from godot is wiring this up via the godot methods file). This has the advantage of still being able to override TreeEntered on your class but has a disadvantage of not being able to use a ctor. My guess is godot would need to implement an init callback prior to the _Ready method being called (or possibly just require base._Ready() to be called and have the code implemented in the base class.

using System.Text;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;

namespace GDSrcGen
{
    [Generator]
    public class Class1 : ISourceGenerator
    {
        public void Execute(GeneratorExecutionContext context)
        {
            if (!(context.SyntaxContextReceiver is SyntaxReceiver receiver))
                return;


            var sb = new StringBuilder();
            sb.AppendLine("using Godot;");
            sb.AppendLine("using System;");
            foreach (var it in receiver.WorkItems)
            {
                var ns = it.Key.Split('.');
                if (ns.Length > 1)
                    sb.AppendLine($"namespace {string.Join(".", ns.Take(ns.Length - 1))}{{");

                sb.AppendLine($"  partial class {ns[ns.Length - 1]}{{");
                sb.AppendLine($"  public {ns[ns.Length - 1]}() {{");
                sb.AppendLine("    TreeEntered += () => {");
                sb.AppendLine("GD.Print(\"hello\");");
                foreach (var v in it.Value)
                    sb.AppendLine($"      {v}");
                sb.AppendLine("      };");
                sb.AppendLine("    }");
                sb.AppendLine("  }");
                if (ns.Length > 1)
                    sb.AppendLine("}");
            }

            context.AddSource("Logs.cs", SourceText.From($@"{Environment.NewLine + string.Join(Environment.NewLine, receiver.Log) + Environment.NewLine}{sb}", Encoding.UTF8));
        }

        public void Initialize(GeneratorInitializationContext context)
        {
            context.RegisterForPostInitialization(context =>
            {
                context.AddSource("OnReady",
            @"using System;
            using System.Collections.Generic;
            using System.Linq;
            using System.Threading.Tasks;

            [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
            public class OnReady : Attribute
            {
                public string NodePath { get; private set; }
                public OnReady() : this(string.Empty) { }
                public OnReady(string nodePath)
                {
                    NodePath = nodePath;
                }

            }");
            });
            context.RegisterForSyntaxNotifications(() => new SyntaxReceiver());
        }


    }

    public class SyntaxReceiver : ISyntaxContextReceiver
    {
        public List<string> Log { get; } = new List<string>();
        public Dictionary<string, List<string>> WorkItems = new();

        public void OnVisitSyntaxNode(GeneratorSyntaxContext context)
        {
            try
            {
                if (context.Node is FieldDeclarationSyntax fieldDeclarationSyntax)
                {
                    foreach (var it in fieldDeclarationSyntax.Declaration.Variables)
                    {
                        var testClass = (IFieldSymbol)context.SemanticModel.GetDeclaredSymbol(it)!;
                        var attr = testClass.GetAttributes().FirstOrDefault(x => x.AttributeClass.Name == "OnReady");
                        if (attr != null)
                        {
                            var type = testClass.ContainingType.ToString();
                            if (!WorkItems.TryGetValue(type, out var fields))
                            {
                                fields = new();
                                WorkItems.Add(type, fields);
                            }

                            if (attr.ConstructorArguments.Length == 0)
                                fields.Add($"{testClass.Name} = GetNode<{testClass.Type.Name}>(\"{testClass.Type.Name}\");");
                            else
                                fields.Add($"{testClass.Name} = GetNode<{testClass.Type.Name}>(\"{attr.ConstructorArguments.First().Value}\");");

                        }
                    }
                }
            }
            catch (Exception ex)
            {
                Log.Add("Error parsing syntax: " + ex.ToString());
            }
        }
    }
}

This code allows the following variations to be defined.

        [OnReady("Info")]
        private Label3D _info;

or no name passed where it assumes the name is the type (just like when you add a type to the scene its name defaults to the type).

        [OnReady()]
        private Label3D _info;

While this is hacked together code, it has saved me quite a lot of time just filling out boilerplate _ready functions even though its not a whole lot of keystrokes saved, it just feels like an entirely better flow.

Edit: Source generator code was designed from this article https://www.infoq.com/articles/CSharp-Source-Generator/ and it seems the GodotOnReady issue points out that its scanning for implemented methods as I suspected.

[Spartan322]

Would not godotengine/godot#62789 probably be preferred. Only case I could think of for an OnReady annotation otherwise is to inject an initialization expression/statement in the attribute via the source generator that would perform the same task as @onready would in GDScript.

[valkyrienyanko]

I've been looking into this as well and I've come across this https://github.com/firebelley/GodotUtilities#other-notes

[31]

Would not godotengine/godot#62789 probably be preferred.

I personally do prefer that, but a significant number of people want to define the node path in their C# code, not in the scene editor, so it doesn't apply. (Or they have even more advanced things in mind.)

GodotOnReady supported both of these scenarios in 3.x. In 4.x, [Export] public Node n; is half of GodotOnReady, and it satisfies me. The other half seems to have alternatives popping up now, pointed out in the thread. (Do check out https://github.com/firebelley/GodotUtilities#other-notes if you haven't already, it has nice QoL features for anyone who's on the "define node paths in code" side of the fence.)

---

I'd still really like clarification from the devs what this issue has actually been taken to mean.
What was the work planned here for 4.0, now planned for 4.x?
Knowing what's planned to be added to Godot proper could change how people approach developing their libraries/extensions, or even let someone contribute to getting it added once we know what's likely to be accepted.

[raulsntos]

@31 I think we wanted to support exporting nodes, but since that's already supported with [Export] this no longer seems necessary in Core.

Other features from your library, such as an OnReady attribute for methods, sound interesting but I think it'd be preferable to leave that to third-parties. Currently, implementing some of these features is too cumbersome (or maybe even impossible), but we'd like to give room for third-party source generators to extend current functionality.

I don't think there are any concrete plans yet, but I'd like to look into this after the 4.0 release.

[FlooferLand]

I dont have anything rather technical to add to the conversation. But I've found an [OnReadyGet] attribute in C# to almost be necessary due to the SHEER AMOUNT of nodes you GetNode while using Godot.

I think i spend at least 30% of my time while coding creating a variable to store a node in, calling GetNode, copy/pasting in the type, and assigning the result to that variable. It's genuinely gotten to the point its infuriating.

Unity has RequireComponent, which works sorta similarly (in the "get runtime object at compile-time" sense at least). Godot would be a whole other step up if we had a time saver like this.

---

Wish i was making it up, most of my classes look like this:

Very long example code [Click to expand]

---

public RigidBody2D FrontWheel;
public RigidBody2D BackWheel;
private GpuParticles2D backTireSmoke;
private AudioStreamPlayer tireScreech;
private AudioStreamPlayer musicPlayer;
private AudioStreamPlayer fallDamage;
private AudioStreamPlayer impactDamage;
private Timer impactDamageCooldown;
private RayCast2D groundCheck;
private Control userInterface;
public Gauge FuelGauge;
public Gauge BoostGauge;

public override void _Ready() {
	FrontWheel = GetNode<RigidBody2D>("FrontWheelHolder/FrontWheel");
	BackWheel = GetNode<RigidBody2D>("BackWheelHolder/BackWheel");
	backTireSmoke = GetNode<GpuParticles2D>("BackWheelHolder/TireSmoke");
	tireScreech = GetNode<AudioStreamPlayer>("Audio/TireScreech");
	musicPlayer = GetNode<AudioStreamPlayer>("Audio/MusicPlayer");
	groundCheck = GetNode<RayCast2D>("GroundCheck");
	userInterface = GetNode<Control>("UI/UI/UI");
	FuelGauge = userInterface.GetNode<Gauge>("Gauges/FuelGauge");
	BoostGauge = userInterface.GetNode<Gauge>("Gauges/BoostGauge");
	fallDamage = GetNode<AudioStreamPlayer>("Audio/FallDamage");
	impactDamage = GetNode<AudioStreamPlayer>("Audio/ImpactDamage");;
	impactDamageCooldown = GetNode<Timer>("Audio/ImpactDamage/Timer");

	// [..] other stuff
}

When they could just look like:

[OnReady("FrontWheelHolder/FrontWheel")] public RigidBody2D FrontWheel;
[OnReady("BackWheelHolder/BackWheel")]   public RigidBody2D BackWheel;
[OnReady("BackWheelHolder/TireSmoke")]   private GpuParticles2D backTireSmoke;
[OnReady("Audio/TireScreech")] 	         private AudioStreamPlayer tireScreech;
[OnReady("Audio/MusicPlayer")] 	         private AudioStreamPlayer musicPlayer;
[OnReady("Audio/FallDamage")]	         private AudioStreamPlayer fallDamage;
[OnReady("Audio/ImpactDamage")]          private AudioStreamPlayer impactDamage;
[OnReady("Audio/ImpactDamage/Timer")]    private Timer impactDamageCooldown;
[OnReady("GroundCheck")]                 private RayCast2D groundCheck;
[OnReady("UI/UI/UI")]                    private Control userInterface;
[OnReady("Gauges/FuelGauge")]            public Gauge FuelGauge;
[OnReady("Gauges/BoostGauge")]           public Gauge BoostGauge;

Mind you.. that first mess is present in classes where i have a lot of other variables and methods.. I basically have to depent on #region blocks

---

More examples [Click to expand]

---

This is how it actually looks in a code editor, I have to go all the way down to 10 pt font size to even fit that monstrosity on my screen.

I and a lot of other people also prefer to keep our node declarations all the way at the top of classes, which makes this problem EVEN WORSE

---

This proposal might also be able to check if a node is present or not at compile-time in simple scenarios as well, depending on the implementation.
I'd heavily suggest just adding something basic in now and changing it in the future, especially since the attribute syntax is so simple it wont change and there's no way to mess up a call to an attribute.

---

It's been YEARS since this proposal was made. Please devs, 👏 add this to 4.2 👏
EDIT: If it might be of any help, I've made a very basic implementation at FlooferLand/GodotGetNode. It's also available on Nuget for anyone that wishes to try it out directly.

[c-schembri]

Adding my support for this. To make matters for C# even worse, if we use nullability checks, then we have to declare all the types as nullable: private Node2D? _myNode;

In general, though, this would remove a lot of boilerplate code required in C#.

[BlankSourceCode]

I think that the problem with just using [Export] instead of some form of [OnReady] attribute, is that the workflow of export is cumbersome for nodes.

You first add the new child node in the editor, perhaps creating and assigning a new script to it, then you have to go to your C# IDE, add a new member variable with the export attribute for the node you just added, then go back to the editor, build the .NET assemblies, then select the root node again to make the editor properties refresh, then finally assign the default value of your newly exported variable to the child node you created.

That is a whole lot of steps, especially when you have multiple child nodes you are trying to assign. And it is easy to forget to assign the default value back in the editor. Or when the reference gets lost due to a rename or whatever. For non-node types like numbers, strings, etc. exporting is much easier because you can assign a default value in script so you often don't need to even go back to the editor until you want to customize it.

The way I see it, is that most of the people using this.foo = GetNode<Foo>("Foo"); in their _Ready() method are doing so because it is so much easier to just write it all in one go and have their member variable assigned via script than to go through the export dance.

If there was a magical way to give [Export] nodes a "use the child node with the same name as my variable" default value, I could see that replacing a need for [OnReady]. But since that would probably need new syntax anyway, it makes sense to me to still consider this proposal.

[FlooferLand]

@BlankSourceCode I personally like doing everything from my scripts since it's more verbose; allows me to know exactly what node a variable connects to or what a signal binds to without having to leave my IDE. And I'd also trust my code more since I fear Godot might bug out and remove references to signals or nodes (used to Unreal and Unity doing stuff like that anyway). It's a tad faster to just do everything from my IDE in general.

I've always seen [Export] be despised by people when working with nodes in Godot since it's the "Unity way of doing things" and I do sorta get that. It gets very messy on Unity sometimes mixing together references to your nodes with ordinary exported variables like floats and strings.

An [OnReady] or [GetNode] attribute would be the best way to go in my opinion, but that raises it's own problem of "What if a node is moved or deleted. Do you just leave the broken reference to that node and risk the player running into a runtime error, or do you magically update the user's code?" which is a big problem [Export] solves.

[BlankSourceCode]

@FlooferLand Isn't the problem with node moving/deletion also an issue if you use the @onready annotation with gdscript? Seems like it would be, so it shouldn't really be a blocker for adding the equivalent [OnReady] attribute to C#.

If changes in the editor could force a rebuild/source generator to run, then we could maybe replace the runtime error with a compile time error (because you could do some source generation validation). I don't know if that's possible though and seems like an addition that would be nice to have but not necessary to get usefulness out of [OnReady]

[FlooferLand]

@BlankSourceCode Very fair point XD
I'm just trying to find a way to fix issues the GDScript variant of the export attribute has even tho that's not the point of this proposal.
Compile time checks shouldn't be that hard to do but I do feel like there would be gaps, like if you changed something in a node hierarchy but you didn't change anything in any script it would still think it was all fine; it would still use the last build unless it recompiled every time your project ran (not sure if it already does that but it doesn't seem so) which is very time-consuming.

[EricDoherty]

Adding my support for this. To make matters for C# even worse, if we use nullability checks, then we have to declare all the types as nullable: private Node2D? _myNode;

In general, though, this would remove a lot of boilerplate code required in C#.

Instead of making the type nullable you can use the null forgiving operator ! and use the following code:
private Node2D _myNode = null!;

[poohcom1]

Another issue with [Export] is its prone to errors when instantiating scenes. 99% of @onready use-case are nodes relative to the scene root, but when you use [Export], the node path becomes available to edit even when in other scenes.

I've had countless bugs from exported nodes resetting due to compilation errors (godotengine/godot#78513) or broken references when copying nodes from one scene to another. So unless we have something like private or scene-only export (#7453) to ensure that these values won't get changed, I hesitate to use [Export] for NodePaths and would highly prefer a C# @onready equivalent.

[dkaszews]

Hi all,

Inspired by work done by @cphillips83 and @cgbeutler, I have written my own codegen solution for this problem. I have decided to go with the getter solution, as it plays much nicer with nullability. I have used it successfully in my toy project, here's a snippet showing usage:

    [GetNode("./EnemySprite")]
    private AnimatedSprite2D? _sprite;
    [GetNode]
    private RayCast2D? _rayCastLeft;
    [GetNode]
    private RayCast2D? _rayCastRight;

    public override void _Process(double delta)
    {
        if (RayCastLeft.IsColliding())
        {
            Sprite.FlipH = false;
        }
        // ...
    }

Please go to https://github.com/dkaszews/GetNodeAutoProp to download it and play around. It is very much a proof of concept, so there are likely some bugs to resolve and features to implement, feel free to add those as issues.