-
Notifications
You must be signed in to change notification settings - Fork 13
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
Future of GodotOnReady in Godot 4.0+? #49
Comments
A few things:
|
Waiting for the final state of 4.0 to see how it ends up. But so far, it does look like Godot's adding the bare minimum and some people will still find GodotOnReady useful. |
It turns out 4.0 broke If a The obvious workaround is that you need to write a public override void _Ready()
{
OnReadySetup();
} Could use a partial method to let Godot's generator find the method and then fill it in with GodotOnReady: partial override void _Ready(); I prefer the former because:
In either case, analyzers could make sure that when (I haven't tried any of this yet, this is from discussion in the Discord C# channel.) |
I just tried it out in 4.0 and there's a more fundamental problem: GodotOnReady generates Any new version of GodotOnReady that works with 4.x will contain significantly more boilerplate than it previously did, and need a significantly different API because of that, so I don't see how it would be possible to continue the project in any recognizable form in 4.x+. As for myself, I'm going to go forward with only the new |
If Roslyn implements something like this, then GodotOnReady would have a way forward: |
Hello! I am super interested in using source generation to achieve what you've done here in Godot 4. I have a similar library where I use reflection to find Nodes with matching names instead of source generation: https://github.com/firebelley/GodotUtilities/blob/master/GodotUtilities/src/ChildNodeAttribute.cs The reason I bring this up is I am wondering if there's an alternate approach to generating the For instance, I define like so: [OnReadyGet]
private MyNode myNode; Under the hood, this tool could generate the following code (psuedocode example): myNode = GetNodeOrNull<MyNode>("MyNode") ?? GetNodeOrNull<MyNode>("myNode") ?? GetNodeOrNull<MyNode>("my_node") Basically generate every reasonable string that could be valid as a node name. Additionally the ability to provide a node path in the
I'm sure calling some kind of setup method would still be necessary. In my reflection code I still require the user to call Anyway, apologies if any of this is under-informed. But I am super curious about this project and how it could be used in Godot 4. Would appreciate your thoughts on anything I've suggested here! I'm thinking about potentially forking this code and trying my hand at implementing this. |
How would the user specify a node that's nested somewhat deeply in the node hierarchy? (How would you represent
In my experience, nodes defined in scenes rarely have a static structure, and change frequently. 😄 I suppose this depends on the nature of the project being worked on. Conceptually, I admit I prefer the purity of a node script defining its dependencies (which set of nodes it needs to be able to fetch, for what reasons), then hooking them up in the scene editor. This is probably why This part of GodotOnReady isn't addressed here yet, and it's probably the part I'll actually miss: [OnReadyGet("AnimationTree", Property = "parameters/playback")]
private AnimationNodeStateMachinePlayback _playback;
Yeah, this is my conclusion in #49 (comment). I think including an analyzer with a fixup is critical to make this approach user-friendly (whether for a reflection or source generation backed implementation). Thanks for the interest. 🙂 I don't think those solutions are right for GodotOnReady in 4.x. (Although I would support adding No argument from me about making a fork for 4.x with different semantics. It's MIT licensed, after all. 😄 I'm just happy it seems like reasonable enough code to fork. |
The new scene unique nodes are perfect for this, and could be used like so: [OnReadyGet("%MyNodeSomewhereDeep")]
private Node node; I think where we differ is I think many scenes (like a player scene) would have a large number of static nodes that all make up the player's behavior. I agree that when a node has external dependencies those should be configured using I definitely see where you're coming from though! Thanks for the response, it's helpful👍 |
For what it's worth, here's a design that I believe would work in 4.x without dropping features, but significantly clogs up the syntax: public partial class Foo : Node
{
[Export, GodotOnReady<Player>] public NodePath playerPath = "somewhere/is/my/player";
public override void _Ready()
{
base._Ready();
OnReadySetup();
}
public override void _Process(double dt)
{
player.Happy();
}
} Note: |
The name of the field wouldn't realistically be |
I guess it's not a big deal--here's what I came up with a little later, which I think I'd actually be happy with: [OnReadyGet(In = "my/node/somewhere")] private Node deep;
[OnReadyGet(Unique = true)] private Node myNodeSomewhereDeep; |
I see what you're saying regarding generated members. If I understand right, I think the approach I'm thinking of is that the user would specify the
Ah, I don't think I'm describing myself very well. Essentially I would expect that if a node path/name is provided, then there would be no need to make a best-guess effort. The guess would only be necessary if no node path information was made available. So for example: [OnReadyGet("%MyNodeSomewhereDeep")]
private Node node; Would generate the following code before doing any guessing: node = GetNode<Node>("%MyNodeSomewhereDeep"); So in essence, there would be multi-step assignment logic that first checks if there is an supplied node path. If not, then move on to trying assignment via guessing the node name. |
Yep, I understand the suggestion, it just isn't possible to make all the current features work with it.
I just wasn't expecting the guessing to stop working after any more than one-level-deep nodes. 😄 Getting direct children isn't a particularly common scenario for me. I assumed the point was to optionally remove the (near-)duplication of the member name in |
Just a side note to the I ended up using |
Well, well! I never considered generating a constructor. From there, the EnterTree or Ready signal makes sense. I don't actually have to pick EnterTree to preserve GodotOnReady functionality, because creating your own (I personally prefer Ready, because descendant node Ready methods/receivers might add additional nodes to the tree or cause other edge cases like that. EnterTree certainly works as its own thing, but as far as keeping GodotOnReady the same as it was in 3.x, it could cause unexpected behavior for some people.) Thanks, I'll give that a try. I'm hoping I can end up creating one NuGet package that picks the strategy based on the version of Godot it's being used in. |
Oh, never mind: I forgot that
|
What's the status of updating to Godot 4? I'd love to make use of this as I currently have several variables being initialized in my _Ready() functions |
No change, here's a summary:
|
I believe Godot 4.0 now lets you
[Export] public Node n;
. Providing[OnReadyGet] public Node n;
is the main reason I wrote this library, so I'm thinking it might now be obsolete.I've still got to get my hands on the 4.0 beta to confirm that works how I expect, and see if there's other feature gaps I'm forgetting that I want to use GodotOnReady to plug. GodotOnReady has evolved to support more cases (like defining a default NodePath in the attribute that can be overridden in the editor) but personally I'm not sure if those would justify using the library in my own projects.
Now that Godot has built-in source generators and analyzers, ideally any new features can be added to Godot directly through a proposal rather than a library like GodotOnReady.
Opening this issue for comments in case anyone else has some thoughts. 🙂
The text was updated successfully, but these errors were encountered: