Cat-Lips / GodotSharp.SourceGenerators

C# source generators for the Godot Game Engine
MIT License
112 stars 10 forks source link

[SceneTree] does not work if the script is not part of a root scene node #69

Open valkyrienyanko opened 1 week ago

valkyrienyanko commented 1 week ago

Untitled

I want to access LanguageButton with [SceneTree] attribute. However I cannot because UIOptionsGeneral.cs (the script attached to the node named "General") is not a scene.

I could convert General node into a scene called UIOptionsGeneral.tscn and move the UIOptionsGeneral.cs script to be next to this scene however I would like to avoid this.

Cat-Lips commented 1 week ago

Very true. [SceneTree] needs to parse a .tscn to populate. By default, this is done using a simple filename match, but you can specify an alternative as a property of the attribute. In this case, however, that would mean UIOptionsGeneral.cs would contain a scene tree structure from the Options node, which is not ideal, but could work.

Another option could be for the Options script to call an Init method on the General script (which should be a UIOptionsGeneral instance in the scene tree) and pass in the Language button. (If General was also uniquely named, this should be as simple as calling General.Init(LanguageButton) in OnReady).

So... To actually solve the problem for this use case, I'm thinking we could add another property to [SceneTree] to specify a root path (default '.'), so when parsed, everything outside of said root is ignored. When used in conjunction with the alternative path to .tscn property, I think we could achieve a working SceneTree solution for sub-nodes. On the other hand, the above Init approach might be easier and reduce the need for additional complexity in an already complex parsing environment.

Your thoughts?

valkyrienyanko commented 1 week ago

I'm not sure how to use any of the params in the [SceneTree] attribute maybe you could show examples on how to use them in the readme or if you think that will clutter the readme maybe include it in GitHubs wiki feature.

If everything above UIOptionsGeneral.cs in the scene tree is not ignored, I assume trying to get anything above would result in a error, unless this was handled with some GetParent() magic. If there are no errors on getting any node above then I think it would be fine to include everything above. (This could be added as a bool param to the attribute but it looks like the attribute already has a lot of params to begin with)

I don't think I would like doing General.Init(LanguageButton).

Cat-Lips commented 1 week ago

I'm not sure how to use any of the params in the [SceneTree] attribute maybe you could show examples on how to use them in the readme or if you think that will clutter the readme maybe include it in GitHubs wiki feature.

Yep, always good to add more examples. The tests have some if you need it now:

If everything above UIOptionsGeneral.cs in the scene tree is not ignored, I assume trying to get anything above would result in a error, unless this was handled with some GetParent() magic. If there are no errors on getting any node above then I think it would be fine to include everything above. (This could be added as a bool param to the attribute but it looks like the attribute already has a lot of params to begin with)

Sorry, I meant ignore (skip) everything above when parsing the tscn, so the _ entry point would be the General node rather than the scene root (Options). That being said, if the full scene is sufficient, then you can achieve this already with [SceneTree("<relative_path_to>options.tscn")] (ie, doesn't recognise res://).

In the meantime, I'll create a branch to allow sub-node scripts to access SceneTree (although I can't see how to do it (yet) without having to provide a path to the tscn)

I don't think I would like doing General.Init(LanguageButton).

As a fellow artist, I can fully appreciate that ;)

(Of course, another option is to call GetNode directly in UIOptionsGeneral.cs, ... or move functionality to root script, but yeah, I get it - code bloat in root class. In the past, I've used a mix of partial classes and nested classes to try to deal with that, but as with everything, pros and cons...)