Make IActionMarkupHandler.OnLineDisplayComplete asynchronous
#347
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
Currently, typewriters will notify all action markup handlers when each character is shown and await any tasks returned by
IActionMarkupHandler.OnCharacterWillAppear. This lets us write custom markup handlers that respond to tags to pause in the middle of a line, run an animation, or await some other signal. However, when a line has been fully displayed, typewriters callIActionMarkupHandler.OnLineDisplayCompletewhich is synchronous, so ActionMarkupHandlers cannot pause execution without hanging the main event thread.What is the new behavior (if this is a feature change)?
This changes the interface for
IActionMarkupHandler.OnLineDisplayCompleteso that it accepts a cancellation token and returns aYarnTask, allowing it to pause dialogue execution at the end of the line.Existing implementations of
IActionMarkupHandlerhave been updated with the new signature, simply returningYarnTask.CompletedTask. The three implementations ofIAsyncTypewriterall await on tasks from all registered action markup handlers at the completion of the line.This introduces a breaking change for any classes that implement
IActionMarkupHandler, as they must modify the signature and return value ofOnLineDisplayCompleteor make it async. Any classes that callIActionMarkupHandlershould also awaitOnLineDisplayCompletein the same manner that they were already awaitingOnCharacterWillAppear.This should not otherwise change any actual behavior; existing implementations can simply return
YarnTask.CompletedTaskor become async, and behavior should be the same.This is my first proposed PR for you all! I'm not positive how the tests get run, so I'm hoping they get run for this PR. As this could be annoying for downstream implementations of
IActionMarkupHandler, I definitely won't be offended if y'all this this is too much trouble for too little gain. I also don't see any tests at all related toIActionMarkupHandlerso if you want to point me in the right direction I'd be happy to add some for this change.