From 7b2924b83e8336b2984086659f46592703b8bb54 Mon Sep 17 00:00:00 2001
From: PushmePullyu <127053144+PushmePullyu@users.noreply.github.com>
Date: Sun, 25 Jun 2023 02:54:14 +0200
Subject: [PATCH] TINSEL: Fix token function calls from non-coroutines

Add extra bools to track token status. This allows setting
the owner to nullptr when a token is acquired from a
non-coroutine context / nullContext without marking it as free.

Fixes the following assert in Discworld 1 (see #13220):

After using the matches on the drainpipe to get the golden chimney
brush, the game switches to non-interactive scenes twice.
When changing back to the original scene, player control should be
disabled while a short animation plays, but this is not the case.
A mouse click by the player during this animation will then initiate
a walk action and cause a failed assert:

engines/tinsel/events.cpp:280:
 void Tinsel::WalkProcess(Common::CoroBaseContext*&, const void*):
 Assertion `_ctx->pMover->hCpath != NOPOLY' failed.

The game attempts to disable user control via the following call chain:
Tinsel::TinselEngine::NextGameCycle()
  Tinsel::ChangeScene()
    Tinsel::DoRestoreSceneFrame()
      Tinsel::Background::StartupBackground()
        Tinsel::ControlStartOff()
	  Tinsel::Control()
            Tinsel::GetControlToken()

This will set the owner of the control token to the current coroutine:
g_tokens[TOKEN_CONTROL].proc = CoroScheduler.getCurrentProcess()

However, since StartupBackground() is called from a non-coroutine
with a nullContext, getCurrentProcess() will return nullptr,
thereby marking the control token as free and allowing player actions.
---
 engines/tinsel/token.cpp | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/engines/tinsel/token.cpp b/engines/tinsel/token.cpp
index 6494bc28836..2acdbd7fbe0 100644
--- a/engines/tinsel/token.cpp
+++ b/engines/tinsel/token.cpp
@@ -31,6 +31,7 @@ namespace Tinsel {
 
 struct Token {
 	Common::PROCESS		*proc;
+	bool				isFree;
 };
 
 // These vars are reset upon engine destruction
@@ -44,13 +45,15 @@ static void TerminateProcess(Common::PROCESS *tProc) {
 
 	// Release tokens held by the process
 	for (int i = 0; i < NUMTOKENS; i++) {
-		if (g_tokens[i].proc == tProc) {
+		if (!g_tokens[i].isFree && g_tokens[i].proc == tProc) {
 			g_tokens[i].proc = nullptr;
+			g_tokens[i].isFree = true;
 		}
 	}
 
 	// Kill the process
-	CoroScheduler.killProcess(tProc);
+	if (tProc != nullptr)
+		CoroScheduler.killProcess(tProc);
 }
 
 /**
@@ -59,8 +62,9 @@ static void TerminateProcess(Common::PROCESS *tProc) {
 void GetControlToken() {
 	const int which = TOKEN_CONTROL;
 
-	if (g_tokens[which].proc == NULL) {
+	if (g_tokens[which].isFree) {
 		g_tokens[which].proc = CoroScheduler.getCurrentProcess();
+		g_tokens[which].isFree = false;
 	}
 }
 
@@ -70,6 +74,7 @@ void GetControlToken() {
 void FreeControlToken() {
 	// Allow anyone to free TOKEN_CONTROL
 	g_tokens[TOKEN_CONTROL].proc = nullptr;
+	g_tokens[TOKEN_CONTROL].isFree = true;
 }
 
 
@@ -84,12 +89,13 @@ void FreeControlToken() {
 void GetToken(int which) {
 	assert(TOKEN_LEAD <= which && which < NUMTOKENS);
 
-	if (g_tokens[which].proc != NULL) {
+	if (!g_tokens[which].isFree) {
 		assert(g_tokens[which].proc != CoroScheduler.getCurrentProcess());
 		TerminateProcess(g_tokens[which].proc);
 	}
 
 	g_tokens[which].proc = CoroScheduler.getCurrentProcess();
+	g_tokens[which].isFree = false;
 }
 
 /**
@@ -102,6 +108,7 @@ void FreeToken(int which) {
 	assert(g_tokens[which].proc == CoroScheduler.getCurrentProcess());	// we'd have been killed if some other proc had taken this token
 
 	g_tokens[which].proc = nullptr;
+	g_tokens[which].isFree = true;
 }
 
 /**
@@ -111,7 +118,7 @@ bool TestToken(int which) {
 	if (which < 0 || which >= NUMTOKENS)
 		return false;
 
-	return (g_tokens[which].proc == NULL);
+	return (g_tokens[which].isFree);
 }
 
 /**
@@ -120,6 +127,7 @@ bool TestToken(int which) {
 void FreeAllTokens() {
 	for (int i = 0; i < NUMTOKENS; i++) {
 		g_tokens[i].proc = nullptr;
+		g_tokens[i].isFree = true;
 	}
 }