Closed Prakkkmak closed 5 days ago
Thank you very much for submitting this PR! While I agree on the intention of this, I think this approach has a few drawbacks.
state_processing
or state_physics_processing
then the library will automatically disable the node's processing and thus avoid incurring runtime cost for something that isn't used. With the implementation given here, this optimization would no longer work.[Signal]
and even there it doesn't seem to work reliably). I find this to be a major hindrance in game development where actors can appear and disappear at any time. On the other hand, any connection made with Godot's Connect
will be automatically removed if either the source or the destination are freed. This means when people use the new events introduced by this change, they will have to manually disconnect their receivers when they are freed. This is an extra required step and will likely raise quite a bit of support tickets here.I think a better approach would be to provide a type-safe Connect
variant. I have created a branch lining out the idea (check https://github.com/derkork/godot-statecharts/compare/main...csharp-type-safe-signals). I think the external API is quite nice, you can do a:
my_state.StateEntered.Connect(OnStateEntered);
// ...
void OnStateEntered() {
}
though internally it's a lot to write in the wrapper classes. Then again, it's a one and done.
I followed the guideline of += and -= of the documentation to keep consistency:
https://docs.godotengine.org/en/stable/tutorials/scripting/c_sharp/c_sharp_signals.html
I would use your approach eventually in future updates.
This merge request introduces the ability to connect Godot state signals to C# events using Callable.From, providing a more idiomatic way to handle signals in C#.
Changes:
Motivation: Connecting signals using Callable.From aligns with the recommended practices for handling signals in Godot using C#. This change improves type safety and readability of the code.
~ Co Created With Chat GPT