Add new wait started and wait ended events

[AlejandorLazaro]

Summary

Adds new "On Pause Started" and "On Pause Ended" events to TMPWriter that activate before and after wait logic requested by invocables.

Changes

• New "On Pause Started Writer" and "On Pause Ended Writer" events in the TMP Writer Editor view
  • Package/Editor/TMPWriterEditor.cs
• New pause start and end events that trigger before and after wait calls. Inspired by logic from Yarn Spinner's Effects.cs
  • Package/Runtime/Components/TMPWriter/TMPWriter.cs

Investigation

Inspired by logic from Yarn Spinner's Effects.cs PausableTypewriter function

    if (text.maxVisibleCharacters == pausePositions.Peek().Item1)
    {
        var pause = pausePositions.Pop();
        onPauseStarted?.Invoke();
        yield return Effects.InterruptableWait(pause.Item2, stopToken);  // <-- events raised before/after wait
        onPauseEnded?.Invoke();

Here's a test GIF showing how the pause start and end events trigger an event for a talking robot to shift between talking and paused states, with debug logs to note the call counts for each pause event as well.

The Yarn dialogue file used:

title: Start
---
Narrator: <+grow>Hi</+grow>,  <!wait=.3>I'm the <wave>narrator</wave> for this beginner's guide!
Narrator: I'm talking to you with <palette>Yarn Spinner</palette>!
Narrator: Here's a wait test! <!wait=.3>One second... <!wait=1> Two seconds... <!wait=2>And lastly, <!wait=.3>Three seconds!<!wait=3> <+grow>..done!</+grow>
Narrator: What do you think of all this, <!wait=.3>then?
    -> It's alright, <!wait=.3><wave>I guess</wave>.
        Narrator: Well, <!wait=.3>that's not very nice.
        Narrator: I'm trying my best here.
    -> It's great. <!wait=.3>I <grow>love</grow> it.
        Narrator: Oh, <!wait=.3>you're too kind.
        Narrator: I'm just doing my job.
Narrator: Anyways, <!wait=.3>Let's start over to help with dialogue testing.
<<jump Start>>
===

[Luca3317]

Concept looks good 👍 Will look at the actual files when I have the time. Also, if youre experienced with github workflow stuff lmk. Running into some issues here I could use help with 🙏

[AlejandorLazaro]

Not familiar with GitHub workflows myself, but I just triggered a workflow on my main fork of this repo and saw the same failure—so I imagine it might have to do with my personal tokens and paths being invalid for the workflow you made, although it's valid for you... or something to that nature.

https://github.com/AlejandorLazaro/TMPEffects/actions/runs/11080616144

I'm curious what happens if this forked branch gets remade in your base repo. That might solve the issue?

[Luca3317]

Yeah, the issue is that check runs from pull requests cant access repository secrets (which contains my Unity license, password etc). I was trying to redo the workflow on a testing branch and switched this pull request to that branch (for now), but it kept running the main branch workflow 🤔 Ill look more into it later

[Luca3317]

Added a new temporary workflow which I hope might work. Cant figure out how to trigger it though, keeps running the old one. Read somewhere online that we might have to update the PR for it to pick up the new workflow.

Can you go ahead and fix the small typo in the event comment? Also, something to consider; should the OnPauseStarted/Stopped events be triggered for conditional waits as well? I feel like yes, but there probably should be some way to figure out whether the event was triggered because of a timed wait or a conditional wait. Also, maybe a second float parameter that indicates how long the wait will be?

Let me know your thoughts 🙂

[AlejandorLazaro]

Let me know what typo it is? I can't find it 😅

The OnPauseStarted/Stopped events sound like it should always occur for all types of waits, conditional or otherwise—that's imagining these being used to help control character expressions, which I can imagine would always want to trigger on dialogue pausing/unpausing. (e.g.: a character has longer/more pauses in a game because their "lying/pressured" gauge is higher, and that gets used for a conditional wait)

I do like the idea of a passed-in wait value, just in case the user wants to be able to use that time to schedule something else for the planned duration. An example I could immediately use in my project is determining how long to fade a specific audio track in/out. Might not be for all users, but would be useful in some cases.

[Luca3317]

Ah I mean the "due to a ending a wait" comment on the OnPauseEndedWriter event.

Btw, appreciate you keeping the OnEvent/OnEventPreview pattern for TMPWriter events 🙂

[AlejandorLazaro]

One quick question since we're on on the topic: the summary for OnStartWriter has the language resumes writing. That event and OnStopWriter both made me wonder if Pause events even made sense, since I would hate for those to add confusion when other events seem like they'd be involved in a pause as well.

        /// <summary>
        /// Raised when the TMPWriter starts / resumes writing.
        /// </summary>
        public UnityEvent<TMPWriter> OnStartWriter;

If they're different enough, what language do you think would be best to differentiate Pause events from the Start/Stop ones? I was thinking maybe the following:

        /// <summary>
        /// Raised when the TMPWriter pauses writing.
        /// </summary>
        public UnityEvent<TMPWriter> OnPauseStartedWriter;
        /// <summary>
        /// Raised when the TMPWriter ends a pause.
        /// </summary>
        public UnityEvent<TMPWriter> OnPauseEndedWriter;

P.S.: I'm not sure if including the term 'wait' would help make the distinction, but I'm also not sure if all waits are effectively pauses. Maybe Pause should be Wait?

[Luca3317]

Pause definitely should be wait 👍 much clearer imo The comments should reflect that as well, so for example:

        /// <summary>
        /// Raised when the TMPWriter begins waiting.
        /// </summary>
        public UnityEvent<TMPWriter, float> OnWaitStartedWriter;
        /// <summary>
        /// Raised when the TMPWriter is done waiting.
        /// </summary>
        public UnityEvent<TMPWriter> OnWaitEndedWriter;

About being able to differentiate between a timed wait and a conditional one, maybe we can indicate a conditional one by a negative value for the float? Should be good enough as long as that is also documented.

[Luca3317]

Actually I dont like "OnWaitStartedWriter", should be more smth like "OnWriterStartedWait" or just "OnWaitStarted". Agree?

[AlejandorLazaro]

Sounds good! I'll update the terms in the PR in a bit.

[AlejandorLazaro]

Updated! Let me know if you think onWaitEndedProp should instead be onWaitStoppedProp. I think since waits really 'complete' rather than just 'stop', the word choice should reflect that in something like ended/finished/completed.

[Luca3317]

I think since waits really 'complete' rather than just 'stop', the word choice should reflect that in something like ended/finished/completed.

Agreed, "ended" is good.

About being able to differentiate between a timed wait and a conditional one, maybe we can indicate a conditional one by a negative value for the float? Should be good enough as long as that is also documented.

What do you think of that? I dont love it but if we mention it in the comment like so

/// <summary>
/// Raised when the TMPWriter starts waiting.
/// The float parameter indicates the amount of time the TMPWriter will wait, in seconds.
/// -1 if the wait is conditional, not time-based.
/// </summary>
public UnityEvent<TMPWriter, float> OnWaitStarted;
/// <summary>
/// Raised when the TMPWriter ends waiting.
/// </summary>
public UnityEvent<TMPWriter> OnWaitEnded;

it should do.

[AlejandorLazaro]

I agree on both ideas. If the documentation (and maybe an example) is clear, teaching the user about a “conditional wait sentinel value” shouldn’t be a problem.

[Luca3317]

Great! If you add those things (including raising the event for conditional waits) and my workflow finally works, it should be good to merge🙂

[AlejandorLazaro]

Hmm, hitting some issues with getting the condition-based waiter events working correctly in my local experiments. My local testing is showing TMPWriter is continuing despite adding a conditional waiter unless I happen to add it during an existing wait period.

I'm also not 100% sure I'm holding the conditional waiting feature right, so—if you're fine with it—I may just make this PR focus on non-conditional waiters and the conditional wait logic can be added later.

And just in case you can see a clear solution to the problem, here's what I've got so far for the conditional wait handling:

-                    if (continueConditions != null) yield return HandleWaitConditions();
+                    if (continueConditions != null)
+                    {
+                        RaiseWaitStartedEvent(-1f);
+                        yield return HandleWaitConditions();
+                        RaiseWaitEndedEvent();
+                    }

[Luca3317]

Hmm no that code looks right to me. Thinking about it now, conditional waits are almost completely untested, so they might just be broken to begin with.

Totally fine to make this PR just for timed waiting; go ahead and add the float paramerter to the WaitStarted event and Ill merge (assuming you consider it done as well?)

[AlejandorLazaro]

Done! Changed it to just add a float for OnWaitStarted events and also removed the -1 is returned for conditional comment.

Should be good to go from here!

[Luca3317]

Awesome! Workflow worked now as well, yay 😄