derkork / godot-statecharts

A state charts extension for Godot 4
MIT License
734 stars 35 forks source link

event_received emits same event every time? #64

Closed Granshmeyr closed 8 months ago

Granshmeyr commented 8 months ago

I'm trying to implement an input buffer in my game. This will allow the player to buffer 'inputs' during an attack animation. In this case however they are really buffering SendEvent() function calls.

In my game there is an intermediary script called StateChartQueue.cs which forwards events to the StateChart node. At any point I can disable the forwarding of events and instead store them into a buffer. This buffer can later be 'flushed' to the StateChart node.

The event_received signal, as emitted by the StateChart node, is used in StateChartQueue.cs to validate the order of events flushed and received. This is so I can truly control the order in which the events are processed when flushing to the StateChart. What I'm trying:

  1. eventList = { "apple", "orange" }
  2. SendEvent("apple")
  3. OnEventRecieved(event)
  4. If event == "apple"
  5. SendEvent("orange")
Relevant code ```csharp partial class DominoRunner : Node { readonly string[] events; readonly StateChart stateChart; string topplingEvent; int dominoIndex; internal DominoRunner(StateChart stateChart, string[] events) { this.events = events; this.stateChart = stateChart; } void OnEventReceived(StringName @event) { GD.Print($"Received {@event}."); if (@event != topplingEvent) { GD.Print($"{@event} was not {topplingEvent}."); //return; } dominoIndex++; if (dominoIndex >= events.Length) { GD.Print("Finished domino."); QueueFree(); return; } topplingEvent = events[dominoIndex]; GD.Print($"Toppling {topplingEvent}, index {dominoIndex}."); stateChart.SendEvent(topplingEvent); } internal void Topple() { dominoIndex = 0; topplingEvent = events[dominoIndex]; foreach (string str in events) { GD.Print($"{str} in domino list."); } stateChart.Connect("event_received", new Callable(this, nameof(OnEventReceived))); GD.Print($"Toppling initial event {topplingEvent}, index {dominoIndex}."); stateChart.SendEvent(topplingEvent); } static string[] SortEventsByWeight(string[] events) { Dictionary weights = new() { { "usable", 1 } }; return events.OrderBy(item => weights.ContainsKey(item) ? weights[item] : 0).ToArray(); } } ```
Full StateChartQueue.cs script ```csharp using System; using System.Collections.Generic; using System.Linq; using Godot; using GodotStateCharts; using Grindle; public partial class StateChartQueue : Node2D { readonly Dictionary recording = new(); readonly Dictionary> events = new(); readonly Dictionary eventBlackLists = new(); bool flushing; readonly Godot.Collections.Array baseEventBlacklist = new() { "restorePlayer" }; public static void SetExpressionProperty(StateChart stateChart, string name, Variant value) { stateChart.SetExpressionProperty(name, value); } public void SendEvent(StateChart stateChart, string @event) { if (!Methods.IsValid(stateChart)) { throw new ArgumentException("StateChart is null."); } if (!recording.ContainsKey(stateChart)) { GD.Print($"{stateChart.Wrapped} not found in keys, creating keys."); recording.Add(stateChart, false); events.Add(stateChart, new()); eventBlackLists.Add(stateChart, Array.Empty()); } if (flushing) { GD.Print("Cannot send event during flush."); return; } if (recording[stateChart]) { if (eventBlackLists[stateChart].Contains(@event)) { GD.Print($"Bypassing record for blacklisted event {@event}."); stateChart.SendEvent(@event); return; } GD.Print($"Recording {@event} to key {stateChart.Wrapped}."); events[stateChart].Add(@event); return; } GD.Print($"Not recording on {stateChart.Wrapped}, sending event {@event}."); stateChart.SendEvent(@event); } public void Flush(StateChart stateChart) { if (!Methods.IsValid(stateChart)) { throw new ArgumentException("Given StateChart is null."); } if (!recording.ContainsKey(stateChart)) { throw new Exception($"Cannot flush unregistered {stateChart.Wrapped}."); } recording[stateChart] = false; if (events[stateChart].Count == 0) { GD.Print($"Nothing to flush for key {stateChart.Wrapped}."); return; } flushing = true; DominoRunner dominoRunner = new(stateChart, events[stateChart].ToArray()); dominoRunner.Topple(); events[stateChart].Clear(); flushing = false; } public void Record(StateChart stateChart, Godot.Collections.Array eventBlackList) { if (!Methods.IsValid(stateChart)) { throw new ArgumentException("StateChart is null."); } foreach (string str in eventBlackList) { GD.Print($"{str} is in received blacklist."); } Godot.Collections.Array _eventBlackList = new(); _eventBlackList.AddRange(baseEventBlacklist); _eventBlackList.AddRange(eventBlackList); foreach (string str in _eventBlackList) { GD.Print($"{str} is in blacklist."); } if (!recording.ContainsKey(stateChart)) { GD.Print($"{stateChart.Wrapped} not found in keys, adding."); recording.Add(stateChart, true); events.Add(stateChart, new()); eventBlackLists.Add(stateChart, _eventBlackList.ToArray()); return; } if (recording[stateChart]) { GD.PrintErr($"Already recording {stateChart.Wrapped}."); return; } GD.Print($"{stateChart.Wrapped} now recording."); recording[stateChart] = true; events[stateChart].Clear(); eventBlackLists[stateChart] = _eventBlackList.ToArray(); } partial class DominoRunner : Node { readonly string[] events; readonly StateChart stateChart; string topplingEvent; int dominoIndex; internal DominoRunner(StateChart stateChart, string[] events) { this.events = events; this.stateChart = stateChart; } void OnEventReceived(StringName @event) { GD.Print($"Received {@event}."); if (@event != topplingEvent) { GD.Print($"{@event} was not {topplingEvent}."); //return; } dominoIndex++; if (dominoIndex >= events.Length) { GD.Print("Finished domino."); QueueFree(); return; } topplingEvent = events[dominoIndex]; GD.Print($"Toppling {topplingEvent}, index {dominoIndex}."); stateChart.Wrapped.CallDeferred("send_event", topplingEvent); } internal void Topple() { dominoIndex = 0; topplingEvent = events[dominoIndex]; foreach (string str in events) { GD.Print($"{str} in domino list."); } stateChart.Connect("event_received", new Callable(this, nameof(OnEventReceived))); GD.Print($"Toppling initial event {topplingEvent}, index {dominoIndex}."); stateChart.SendEvent(topplingEvent); } static string[] SortEventsByWeight(string[] events) { Dictionary weights = new() { { "usable", 1 } }; return events.OrderBy(item => weights.ContainsKey(item) ? weights[item] : 0).ToArray(); } } } ```

The problem is that OnEventReceived(StringName @event) is being passed the same event string every time it is called, which is mainly what I don't understand. Because of this, I just can't use the event to the effect that I want. Here is a log showing this:

Relevant logs ``` Not recording on , sending event usable. restorePlayer is in blacklist. now recording. Recording jog to key . Recording moveUp to key . Recording stop to key . Recording jog to key . Recording moveRight to key . Recording stop to key . Bypassing record for blacklisted event restorePlayer. jog in domino list. moveUp in domino list. stop in domino list. jog in domino list. moveRight in domino list. stop in domino list. Toppling initial event jog, index 0. Received jog. Toppling moveUp, index 1. Received jog. jog was not moveUp. Toppling stop, index 2. Received jog. jog was not stop. Toppling jog, index 3. Received jog. Toppling moveRight, index 4. Received jog. jog was not moveRight. Toppling stop, index 5. Received jog. jog was not stop. Finished domino. ```

As you can hopefully see, the StateChart node is passing the first event I sent to it for this entire 'loop'. There is still so much I don't know about game engines and programming in general, so maybe I'm just dumb. Any idea what the issue might be?

Info: I'm executing Record() and Flush() from an AnimationPlayer.

Granshmeyr commented 8 months ago

Sorry, I forgot to check the history debugger tab which indicates that I might be doing something wrong in a way that's unrelated to the StateChart:

Relevant logs ``` [4118]: Event received: restorePlayer [4118]: Transition: ToRestorePlayer from Root/UsableAnimations/ShortSword to Root/PlayerRoot/Parallel/Player/RestorePlayer [4118]: exiT: BasicUp1 [4118]: exiT: ShortSword [4118]: exiT: UsableAnimations [4118]: Enter: PlayerRoot [4118]: Enter: Parallel [4118]: Enter: Player [4118]: Enter: States [4118]: Enter: Behavior [4118]: Enter: Idle [4118]: Enter: Animation [4118]: Enter: Idle [4118]: Enter: IdleDown [4118]: Enter: UsableTransitions [4118]: Enter: Nothing [4118]: exiT: Idle [4118]: exiT: Behavior [4118]: exiT: IdleDown [4118]: exiT: Idle [4118]: exiT: Animation [4118]: exiT: States [4118]: Enter: States [4118]: Enter: Behavior [4118]: Enter: Idle [4118]: Enter: Animation [4118]: Enter: Idle [4118]: Enter: IdleDown [4118]: Enter: IdleUp [4118]: exiT: IdleDown [4118]: Event received: jog [4118]: Transition: ToJog from Root/PlayerRoot/Parallel/Player/States/Behavior/Idle to Root/PlayerRoot/Parallel/Player/States/Behavior/Jog [4118]: exiT: Idle [4118]: Enter: Jog [4118]: Transition: ToJogUp from Root/PlayerRoot/Parallel/Player/States/Animation/Idle/IdleUp to Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogUp [4118]: exiT: IdleUp [4118]: exiT: Idle [4118]: Enter: Jog [4118]: Enter: JogUp [4118]: Event received: jog [4118]: Transition: ToJogUp from Root/PlayerRoot/Parallel/Player/States/Animation/Jog to Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogUp [4118]: exiT: JogUp [4118]: Enter: JogUp [4118]: Event received: jog [4118]: Transition: ToIdle from Root/PlayerRoot/Parallel/Player/States/Behavior/Jog to Root/PlayerRoot/Parallel/Player/States/Behavior/Idle [4118]: exiT: Jog [4118]: Enter: Idle [4118]: Transition: ToIdleUp from Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogUp to Root/PlayerRoot/Parallel/Player/States/Animation/Idle/IdleUp [4118]: exiT: JogUp [4118]: exiT: Jog [4118]: Enter: Idle [4118]: Enter: IdleUp [4118]: Event received: jog [4118]: Transition: ToJog from Root/PlayerRoot/Parallel/Player/States/Behavior/Idle to Root/PlayerRoot/Parallel/Player/States/Behavior/Jog [4118]: exiT: Idle [4118]: Enter: Jog [4118]: Transition: ToJogUp from Root/PlayerRoot/Parallel/Player/States/Animation/Idle/IdleUp to Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogUp [4118]: exiT: IdleUp [4118]: exiT: Idle [4118]: Enter: Jog [4118]: Enter: JogUp [4118]: Event received: jog [4118]: Transition: ToJogRight from Root/PlayerRoot/Parallel/Player/States/Animation/Jog to Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogRight [4118]: exiT: JogUp [4118]: Enter: JogRight [4118]: Event received: jog [4118]: Transition: ToIdle from Root/PlayerRoot/Parallel/Player/States/Behavior/Jog to Root/PlayerRoot/Parallel/Player/States/Behavior/Idle [4118]: exiT: Jog [4118]: Enter: Idle [4118]: Transition: ToIdleRight from Root/PlayerRoot/Parallel/Player/States/Animation/Jog/JogRight to Root/PlayerRoot/Parallel/Player/States/Animation/Idle/IdleRight [4118]: exiT: JogRight [4118]: exiT: Jog [4118]: Enter: Idle [4118]: Enter: IdleRight ```

EDIT: After looking at the above logs, the title of this post is not accurate. The StateChart is indeed receiving the first event of the array six times in a row, even though I'm attempting to iterate through it. My new title would be "SendEvent not sending correct event?" or something.

Granshmeyr commented 8 months ago

Sorry for spam, but I just found the 'solution'... I guess? Basically, I just changed stateChart.SendEvent(topplingEvent); in my code to stateChart.Wrapped.CallDeferred("send_event", topplingEvent); and now everything works perfectly fine. Of course, I've changed the Wrapped property to public. I have zero understanding of threads or anything like that so I don't know why this solves my problem.

derkork commented 8 months ago

You have found a bug! Thank you very much for reporting this. I have tried to set up a minimal reproducer which looks like this:

extends Node

@onready var chart = $StateChart

func _ready():
    chart.send_event("a")
    chart.send_event("b")
    chart.send_event("c")

func _on_state_chart_event_received(event):
    print("got " , event)
    if event == "a":
        chart.send_event("d")

This should print

got a
got d
got b
got c

but it prints got a in an endless loop. The reason for this is that the library will only process new events once the currently running event has been fully processed by all states and all callbacks have been called. If it would not do this, we'd get some interesting (read: extremely hard to understand) behaviour when a new event is sent in an event handler . So what the library does is, when it you send a new event while one is currently still being processed, it puts it into an internal queue and then processes them one by one. So what happened was:

The reason why it works when you do a call_deferred is because in this case you give each event time to fully play out before your insert the new one. So no events are queued and the buggy line in the library is never run.

I'm making a fix for this.

derkork commented 8 months ago

The fix is in and I also submitted it to the asset library. May take a while to materialize there because the approval process is manual there and I'm not sure how many people are currently available to approve it. The fix is really easy so if you want to fix your copy in the meantime, just change line 106 addons/godot_state_charts/state_chart.gd like this:

image