Skip to content

Commit

Permalink
TINSEL: Fix token function calls from non-coroutines
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
PushmePullyu authored and sev- committed Aug 6, 2023
1 parent b351758 commit 7b2924b
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions engines/tinsel/token.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace Tinsel {

struct Token {
Common::PROCESS *proc;
bool isFree;
};

// These vars are reset upon engine destruction
Expand All @@ -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);
}

/**
Expand All @@ -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;
}
}

Expand All @@ -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;
}


Expand All @@ -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;
}

/**
Expand All @@ -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;
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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;
}
}

Expand Down

0 comments on commit 7b2924b

Please sign in to comment.