From 1a082f7f158d93418622052e3568900070975c8f Mon Sep 17 00:00:00 2001 From: katianderic Date: Fri, 13 Mar 2020 00:36:28 +0100 Subject: [PATCH] let move stack determine whose turn it is When something goes wrong with a move (typically because of undo), the move stack and server's idea of whose turn it is get out of sync. Since it being my turn means it's up to me to put the next move on the stack, let the stack own "next turn". (Can probably eventually get rid of the ServerCtxt variable that tracks it.) --- xwords4/common/model.c | 31 ++++++++++++++++++----------- xwords4/common/model.h | 2 ++ xwords4/common/movestak.c | 41 +++++++++++++++++++++++++++++++++------ xwords4/common/movestak.h | 5 +++-- xwords4/common/server.c | 22 +++++++++++---------- 5 files changed, 72 insertions(+), 29 deletions(-) diff --git a/xwords4/common/model.c b/xwords4/common/model.c index c4bd6044a..b6b643327 100644 --- a/xwords4/common/model.c +++ b/xwords4/common/model.c @@ -270,7 +270,7 @@ model_writeToTextStream( const ModelCtxt* model, XWStreamCtxt* stream ) void model_setSize( ModelCtxt* model, XP_U16 nCols ) { - ModelVolatiles vol = model->vol; /* save vol so we don't wipe it out */ + ModelVolatiles saveVol = model->vol; /* save vol so we don't wipe it out */ XP_U16 oldSize = model->nCols; /* zero when called from model_make() */ XP_ASSERT( MAX_COLS >= nCols ); @@ -279,22 +279,23 @@ model_setSize( ModelCtxt* model, XP_U16 nCols ) model->nCols = nCols; model->nRows = nCols; - model->vol = vol; + model->vol = saveVol; + ModelVolatiles* vol = &model->vol; if ( oldSize != nCols ) { - if ( !!model->vol.tiles ) { - XP_FREE( model->vol.mpool, model->vol.tiles ); + if ( !!vol->tiles ) { + XP_FREE( vol->mpool, vol->tiles ); } - model->vol.tiles = XP_MALLOC( model->vol.mpool, TILES_SIZE(model, nCols) ); + vol->tiles = XP_MALLOC( vol->mpool, TILES_SIZE(model, nCols) ); } - XP_MEMSET( model->vol.tiles, TILE_EMPTY_BIT, TILES_SIZE(model, nCols) ); + XP_MEMSET( vol->tiles, TILE_EMPTY_BIT, TILES_SIZE(model, nCols) ); - if ( !!model->vol.stack ) { - stack_init( model->vol.stack, model->vol.gi->inDuplicateMode ); + if ( !!vol->stack ) { + stack_init( vol->stack, vol->gi->nPlayers, vol->gi->inDuplicateMode ); } else { - model->vol.stack = stack_make( MPPARM(model->vol.mpool) - dutil_getVTManager(model->vol.dutil), - model->vol.gi->inDuplicateMode ); + vol->stack = stack_make( MPPARM(vol->mpool) + dutil_getVTManager(vol->dutil), + vol->gi->nPlayers, vol->gi->inDuplicateMode ); } } /* model_setSize */ @@ -2112,6 +2113,14 @@ model_assignPlayerTiles( ModelCtxt* model, XP_S16 turn, model_addNewTiles( model, turn, &sorted ); } /* model_assignPlayerTiles */ +XP_S16 +model_getNextTurn( const ModelCtxt* model ) +{ + XP_S16 result = stack_getNextTurn( model->vol.stack ); + // LOG_RETURNF( "%d", result ); + return result; +} + void model_assignDupeTiles( ModelCtxt* model, const TrayTileSet* tiles ) { diff --git a/xwords4/common/model.h b/xwords4/common/model.h index 882ff72d9..799738bdb 100644 --- a/xwords4/common/model.h +++ b/xwords4/common/model.h @@ -348,6 +348,8 @@ void model_packTilesUtil( ModelCtxt* model, PoolContext* pool, Tile model_askBlankTile( ModelCtxt* model, XP_U16 turn, XP_U16 col, XP_U16 row); +XP_S16 model_getNextTurn( const ModelCtxt* model ); + #ifdef CPLUS } #endif diff --git a/xwords4/common/movestak.c b/xwords4/common/movestak.c index f203909b9..bebd33919 100644 --- a/xwords4/common/movestak.c +++ b/xwords4/common/movestak.c @@ -49,6 +49,7 @@ struct StackCtxt { XP_U16 bitsPerTile; XP_U16 highWaterMark; XP_U16 typeBits; + XP_U16 nPlayers; XP_U8 flags; XP_Bool inDuplicateMode; @@ -60,10 +61,11 @@ struct StackCtxt { #define HAVE_FLAGS_MASK ((XP_U16)0x8000) void -stack_init( StackCtxt* stack, XP_Bool inDuplicateMode ) +stack_init( StackCtxt* stack, XP_U16 nPlayers, XP_Bool inDuplicateMode ) { stack->nEntries = stack->highWaterMark = 0; stack->top = START_OF_STREAM; + stack->nPlayers = nPlayers; stack->inDuplicateMode = inDuplicateMode; /* I see little point in freeing or shrinking stack->data. It'll get @@ -91,13 +93,14 @@ stack_setBitsPerTile( StackCtxt* stack, XP_U16 bitsPerTile ) } StackCtxt* -stack_make( MPFORMAL VTableMgr* vtmgr, XP_Bool inDuplicateMode ) +stack_make( MPFORMAL VTableMgr* vtmgr, XP_U16 nPlayers, XP_Bool inDuplicateMode ) { StackCtxt* result = (StackCtxt*)XP_MALLOC( mpool, sizeof( *result ) ); if ( !!result ) { XP_MEMSET( result, 0, sizeof(*result) ); MPASSIGN(result->mpool, mpool); result->vtmgr = vtmgr; + result->nPlayers = nPlayers; result->inDuplicateMode = inDuplicateMode; } @@ -193,7 +196,7 @@ stack_copy( const StackCtxt* stack ) stack_writeToStream( stack, stream ); newStack = stack_make( MPPARM(stack->mpool) stack->vtmgr, - stack->inDuplicateMode ); + stack->nPlayers, stack->inDuplicateMode ); stack_loadFromStream( newStack, stream ); stack_setBitsPerTile( newStack, stack->bitsPerTile ); stream_destroy( stream ); @@ -205,8 +208,8 @@ pushEntryImpl( StackCtxt* stack, const StackEntry* entry ) { XWStreamCtxt* stream = stack->data; - XP_LOGF( "%s(typ=%s, player=%d)", __func__, - StackMoveType_2str(entry->moveType), entry->playerNum ); + XP_LOGFF( "(typ=%s, player=%d)", StackMoveType_2str(entry->moveType), + entry->playerNum ); if ( !stream ) { stream = mem_stream_make_raw( MPPARM(stack->mpool) stack->vtmgr ); @@ -270,6 +273,12 @@ pushEntry( StackCtxt* stack, const StackEntry* entry ) #ifdef DEBUG_HASHING XP_Bool correct = XP_TRUE; XP_U32 origHash = stack_getHash( stack, correct ); + + StackEntry prevTop; + if ( 1 < stack->nPlayers && + stack_getNthEntry( stack, stack->nEntries - 1, &prevTop ) ) { + XP_ASSERT( prevTop.playerNum != entry->playerNum ); + } #endif pushEntryImpl( stack, entry ); @@ -281,7 +290,11 @@ pushEntry( StackCtxt* stack, const StackEntry* entry ) XP_ASSERT( origHash == stack_getHash( stack, correct ) ); pushEntryImpl( stack, &lastEntry ); XP_ASSERT( newHash == stack_getHash( stack, correct ) ); - XP_LOGF( "%s: all ok", __func__ ); + XP_LOGFF( "all ok; pushed type %s for player %d into pos #%d, hash now %X (was %X)", + StackMoveType_2str(entry->moveType), entry->playerNum, + stack->nEntries, newHash, origHash ); + } else { + XP_ASSERT(0); } #endif } @@ -509,6 +522,22 @@ stack_popEntry( StackCtxt* stack, StackEntry* entry ) return found; } /* stack_popEntry */ +XP_S16 +stack_getNextTurn( StackCtxt* stack ) +{ + XP_S16 result = -1; + XP_U16 nn = stack->nEntries - 1; + + StackEntry dummy; + if ( stack_getNthEntry( stack, nn, &dummy ) ) { + result = (dummy.playerNum + 1) % stack->nPlayers; + stack_freeEntry( stack, &dummy ); + } + + LOG_RETURNF( "%d", result ); + return result; +} + XP_Bool stack_redo( StackCtxt* stack, StackEntry* entry ) { diff --git a/xwords4/common/movestak.h b/xwords4/common/movestak.h index 6da35fdeb..dd68db062 100644 --- a/xwords4/common/movestak.h +++ b/xwords4/common/movestak.h @@ -82,10 +82,10 @@ typedef struct StackEntry { typedef struct StackCtxt StackCtxt; -StackCtxt* stack_make( MPFORMAL VTableMgr* vtmgr, XP_Bool inDuplicateMode ); +StackCtxt* stack_make( MPFORMAL VTableMgr* vtmgr, XP_U16 nPlayers, XP_Bool inDuplicateMode ); void stack_destroy( StackCtxt* stack ); -void stack_init( StackCtxt* stack, XP_Bool inDuplicateMode ); +void stack_init( StackCtxt* stack, XP_U16 nPlayers, XP_Bool inDuplicateMode ); XP_U32 stack_getHash( const StackCtxt* stack, XP_Bool correct ); void stack_setBitsPerTile( StackCtxt* stack, XP_U16 bitsPerTile ); @@ -116,6 +116,7 @@ XP_U16 stack_getNEntries( const StackCtxt* stack ); XP_Bool stack_getNthEntry( StackCtxt* stack, XP_U16 n, StackEntry* entry ); XP_Bool stack_popEntry( StackCtxt* stack, StackEntry* entry ); +XP_S16 stack_getNextTurn( StackCtxt* stack ); XP_Bool stack_redo( StackCtxt* stack, StackEntry* entry ); void stack_freeEntry( StackCtxt* stack, StackEntry* entry ); diff --git a/xwords4/common/server.c b/xwords4/common/server.c index 60042b7ac..1517a3ae6 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -679,7 +679,10 @@ server_initClientConnection( ServerCtxt* server, XWStreamCtxt* stream ) XP_UCHAR* name; XP_U8 len; - XP_ASSERT( ii++ < MAX_NUM_PLAYERS ); +#ifdef DEBUG + XP_ASSERT( ii < MAX_NUM_PLAYERS ); + ++ii; +#endif if ( !lp->isLocal ) { continue; } @@ -1369,7 +1372,7 @@ makeRobotMove( ServerCtxt* server ) if ( canMove || NPASSES_OK(server) ) { juggleMoveIfDebug( &newMove ); model_makeTurnFromMoveInfo( model, turn, &newMove ); - XP_LOGF( "%s: robot making %d tile move", __func__, newMove.nTiles ); + XP_LOGFF( "robot making %d tile move for player %d", newMove.nTiles, turn ); if ( !!stream ) { XWStreamCtxt* wordsStream = mkServerStream( server ); @@ -2505,7 +2508,6 @@ nextTurn( ServerCtxt* server, XP_S16 nxtTurn ) { LOG_FUNC(); CurGameInfo* gi = server->vol.gi; - XP_U16 nPlayers = gi->nPlayers; XP_Bool playerTilesLeft = XP_FALSE; XP_S16 currentTurn = server->nv.currentTurn; XP_Bool moreToDo = XP_FALSE; @@ -2517,10 +2519,10 @@ nextTurn( ServerCtxt* server, XP_S16 nxtTurn ) if ( inDuplicateMode(server) ) { nxtTurn = dupe_nextTurn( server ); } else { - nxtTurn = (currentTurn+1) % nPlayers; + nxtTurn = model_getNextTurn( server->vol.model ); } } else { - XP_LOGF( "%s(): turn == -1 so dropping", __func__ ); + XP_LOGFF( "%s", "turn == -1 so dropping" ); } } else { /* We're doing an undo, and so won't bother figuring out who the @@ -2670,9 +2672,7 @@ sendMoveTo( ServerCtxt* server, XP_U16 devIndex, XP_U16 turn, stream_putBits( stream, 1, isTrade ); if ( isTrade ) { - traySetToStream( stream, tradedTiles ); - } else { stream_putBits( stream, 1, legal ); @@ -2828,7 +2828,6 @@ reflectMoveAndInform( ServerCtxt* server, XWStreamCtxt* stream ) if ( success ) { if ( isTrade ) { - sendMoveToClientsExcept( server, whoMoved, XP_TRUE, &newTiles, &tradedTiles, sourceClientIndex ); @@ -3580,8 +3579,8 @@ finishMove( ServerCtxt* server, TrayTileSet* newTiles, XP_U16 turn ) } else { nextTurn( server, PICK_NEXT ); } - XP_LOGF( "%s(): player %d now has %d tiles", __func__, turn, - model_getNumTilesInTray( model, turn ) ); + XP_LOGFF( "player %d now has %d tiles", turn, + model_getNumTilesInTray( model, turn ) ); } /* finishMove */ /* return XP_TRUE; */ @@ -3815,6 +3814,9 @@ setTurn( ServerCtxt* server, XP_S16 turn ) turn = dupe_nextTurn( server ); } server->nv.currentTurn = turn; + if ( 0 <= turn ) { + XP_ASSERT( turn == model_getNextTurn( server->vol.model ) ); + } server->nv.lastMoveTime = dutil_getCurSeconds( server->vol.dutil ); callTurnChangeListener( server ); }