fix: reentrance lock during state changes / callbacks

CHANGES.md

@@ -4,10 +4,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
-
+## [0.13.1] - 2024-02-23
 ### Fixed
 - The state chart now issues a better error message when being called while not yet ready ([#81](https://github.com/derkork/godot-statecharts/issues/81)).
+- The state chart now properly handles property changes which happen during state or transition callbacks. These will be queued after the current event or property change is fully processed. This way consistency is maintained and reactions to an event or property change will not see intermediate property changes during their execution ([#82](https://github.com/derkork/godot-statecharts/issues/82)). A big thanks goes to [Matt Idzik](https://github.com/MidZik) who supplied a PR that helped implementing this fix.
+
 ### Improved
 - The history in the state chart debugger now uses little icons to show the type history entry. This makes it easier to see what happened at a glance. A big thanks goes out to [Alireza Zamani](https://github.com/alitnk) for suggesting this improvement.
 

addons/godot_state_charts/plugin.cfg

@@ -3,5 +3,5 @@
 name="Godot State Charts"
 description="A simple, yet powerful state charts library for Godot"
 author="Jan Thomä & Contributors"
-version="0.13.0"
+version="0.13.1"
 script="godot_state_charts.gd"

addons/godot_state_charts/state_chart.gd

@@ -32,19 +32,17 @@ var _state:State = null
 var _expression_properties:Dictionary = {
 }
 
-## A list of events which are still pending resolution.
-var _queued_events:Array[StringName] = []
-
-## Flag indicating if the state chart is currently processing an 
-## event. Until an event is fully processed, new events will be queued
-## and then processed later.
-var _event_processing_active:bool = false
+## A list of pending changes
+var _queued_changes:Array[PendingChange] = []
 
+## Flag indicating if the state chart is currently processing. 
+## Until a change is fully processed, no further changes can
+## be introduced from the outside.
+var _locked_down:bool = false
 
 var _queued_transitions:Array[Dictionary] = []
 var _transitions_processing_active:bool = false
 
-
 var _debugger_remote:DebuggerRemote = null
 
 
@@ -58,7 +56,7 @@ func _ready() -> void:
 		return
 
 	# check if the child is a state
-	var child = get_child(0)
+	var child:Node = get_child(0)
 	if not child is State:
 		push_error("StateMachine's child must be a State")
 		return
@@ -90,36 +88,71 @@ func send_event(event:StringName) -> void:
 		push_error("State chart has no root state. Ignoring call to `send_event`.")
 		return
 
-	if _event_processing_active:
-		# the state chart is currently processing an event
-		# therefore queue the event and process it later.
-		_queued_events.append(event)
-		return	
-
-	# enable the reentrance lock for event processing
-	_event_processing_active = true
+	_run_change(PendingEvent.new(event))
+
+
+## Sets a property that can be used in expression guards. The property will be available as a global variable
+## with the same name. E.g. if you set the property "foo" to 42, you can use the expression "foo == 42" in
+## an expression guard.
+func set_expression_property(name:StringName, value) -> void:
+	if not is_node_ready():
+		push_error("State chart is not yet ready. If you call `set_expression_property` in `_ready`, please call it deferred, e.g. `state_chart.set_expression_property.call_deferred(\"my_property\", 5).")
+		return
+
+	if not is_instance_valid(_state):
+		push_error("State chart has no root state. Ignoring call to `set_expression_property`.")
+		return
+
+	_run_change(PendingPropertyChange.new(name, value))
+
+
+func _run_change(change:PendingChange):
+	if _locked_down:
+		# we are currently running changes, so we need to 
+		# queue this.
+		_queued_changes.append(change)
+		return
+
+	# enable the reentrance lock
+	_locked_down = true
 
-	# first process this event.
-	event_received.emit(event)
-	_state._process_transitions(event, false)
+	_do_run_change(change)
 
-	# if other events have accumulated while the event was processing
+	# if other changed have accumulated while this change was processing
 	# process them in order now
-	while _queued_events.size() > 0:
-		var next_event = _queued_events.pop_front()
-		event_received.emit(next_event)
-		_state._process_transitions(next_event, false)
+	while _queued_changes.size() > 0:
+		var next_change:PendingChange = _queued_changes.pop_front()
+		_do_run_change(next_change)
+
+	_locked_down = false
+
+
+## Actually runs a change through the state chart.
+func _do_run_change(change:PendingChange):
+	if change is PendingEvent:
+		# emit the received signal
+		event_received.emit(change.event_name)
+		_state._process_transitions(change.event_name, false)
+
+	elif change is PendingPropertyChange:
+		_expression_properties[change.property] = change.value
+		# run a property change event through the state chart to run automatic transitions
+		_state._process_transitions(&"", true)
+
 
-	_event_processing_active = false
 
 ## Allows states to queue a transition for running. This will eventually run the transition
 ## once all currently running transitions have finished. States should call this method
 ## when they want to transition away from themselves. 
 func _run_transition(transition:Transition, source:State):
-	# if we are currently inside of a transition, queue it up
+	# if we are currently inside of a transition, queue it up. This can happen
+	# if a state has an automatic transition on enter, in which case we want to
+	# finish the current transition before starting a new one.
 	if _transitions_processing_active:
 		_queued_transitions.append({transition : source})
 		return
+
+	_transitions_processing_active = true
 
 	# we can only transition away from a currently active state
 	# if for some reason the state no longer is active, ignore the transition	
@@ -132,6 +165,7 @@ func _run_transition(transition:Transition, source:State):
 		var next_transition_source = next_transition_entry[next_transition]
 		_do_run_transition(next_transition, next_transition_source)
 
+	_transitions_processing_active = false
 
 ## Runs the transition. Used internally by the state chart, do not call this directly.	
 func _do_run_transition(transition:Transition, source:State):
@@ -146,21 +180,6 @@ func _do_run_transition(transition:Transition, source:State):
 func _warn_not_active(transition:Transition, source:State):
 	push_warning("Ignoring request for transitioning from ", source.name, " to ", transition.to, " as the source state is no longer active. Check whether your trigger multiple state changes within a single frame.")
 
-## Sets a property that can be used in expression guards. The property will be available as a global variable
-## with the same name. E.g. if you set the property "foo" to 42, you can use the expression "foo == 42" in
-## an expression guard.
-func set_expression_property(name:StringName, value) -> void:
-	if not is_node_ready():
-		push_error("State chart is not yet ready. If you call `set_expression_property` in `_ready`, please call it deferred, e.g. `state_chart.set_expression_property.call_deferred(\"my_property\", 5).")
-		return
-
-	if not is_instance_valid(_state):
-		push_error("State chart has no root state. Ignoring call to `set_expression_property`.")
-		return
-
-	_expression_properties[name] = value
-	# run a property change event through the state chart to run automatic transitions
-	_state._process_transitions(&"", true)
 
 
 ## Calls the `step` function in all active states. Used for situations where `state_processing` and 
@@ -176,11 +195,32 @@ func step():
 	_state._state_step()
 
 func _get_configuration_warnings() -> PackedStringArray:
-	var warnings = []
+	var warnings:PackedStringArray = []
 	if get_child_count() != 1:
 		warnings.append("StateChart must have exactly one child")
 	else:
-		var child = get_child(0)
+		var child:Node = get_child(0)
 		if not child is State:
 			warnings.append("StateChart's child must be a State")
 	return warnings
+
+
+class PendingChange:
+	extends RefCounted
+
+class PendingEvent:
+	extends PendingChange
+	var event_name:StringName
+
+	func _init(event_name:StringName):
+		self.event_name = event_name
+
+class PendingPropertyChange:
+	extends PendingChange
+	var property:StringName
+	var value:Variant
+
+	func _init(property:StringName, value:Variant):
+		self.property = property
+		self.value = value
+