derkork / godot-statecharts

A state charts extension for Godot 4
MIT License
679 stars 33 forks source link

idea: Using Nodes to represent and manage events rather than Strings and a custom menu. #88

Closed mieldepoche closed 4 months ago

mieldepoche commented 4 months ago

Disclaimer: I discovered this addon recently and am currently looking at the demos and toying around to get a feel of how it works. Nevertheless, I had that idea and am curious about feedback.

Events are managed and sent as strings. String are a pain to work with sometimes (having to manually change values across code files, no errors until runtime when code has typos, no code autocompletion). Possible solution: rather than registering events through a custom menu, what about managing them as nodes?

mockup: image

The StateChart would now have 2 child nodes: the root state and the events node (this one would be added automatically). Then, events would be managed by adding or renaming nodes under that Events node, and picked from there by transition nodes:

class_name Transition
extends Node

# currently:
@export var event: StringName = ""

# would become:
@export var event: Event # Event would be the new node type

To send an event to the chart, you would do:

@onready var ev_food_reached: Event = $StateChart/Events/food_reached

func f():
  chart.new_send_event( ev_food_reached )
  # or
  chart.new_send_event( $StateChart/Events/food_reached )
  # or
  chart.new_send_event( %food_reached )

  # same behavior as: 
  chart.current_send_event( ev_food_reached.name )

The event nodes wouldn't have behavior on their own, but when exposed as such I think they would provide an usability upgrade.

upsides:

downsides:

ux details:

image

derkork commented 4 months ago

Thanks for writing this up! I'm not really sure how this is an improvement though. Right now we have strings for events.

Afterwards we have node paths for events which still are some kind of strings and can break in exactly the same way. But we now have added a lot of ceremony for adding and using these events. Instead of just typing an event name (or using the dropdown for selecting a previous one) the user now needs to create nodes and link these in the transitions. And in the code the user now needs to create export fields or do a get_node with a hard-coded nodepath just for sending a simple event to the state chart. In addition we just introduced a breaking change that will break all existing projects out there.

In my experience the strings really are not that big of a problem as it may seem. Once you create a state chart the events rarely change and even if they do, we now have a UI that quickly allows you to rename events in the chart and you can do a search/replace to fix code occurences. Or if you want to play it extra safe, you can make constants with the event names and use these (though you probably also want to rename the constants when the event name changes, so in practice this will not really change anything from a maintenance perspective). We have some talk over at #87 how the state chart could generate code with these constants but it's not without its problems either.

mieldepoche commented 4 months ago

which still are some kind of strings and can break in exactly the same way

yes. The benefit being that broken node paths forbid you to launch the game, whereas right now, you won't get a warning if you send an event no transition is reacting to.

In my experience the strings really are not that big of a problem as it may seem

well, that's good to know! :smile: I always try to avoid them as much as I can, but as you mentioned using constants should be enough.

Thanks for the feedback!