From de3107271dd9fbc73df14c3f85623e11bca91e48 Mon Sep 17 00:00:00 2001 From: Eric House Date: Thu, 3 May 2012 19:02:42 -0700 Subject: [PATCH] changes to make hashing work -- make sure unused space in structs is 0 -- and to detect when an incoming move doesn't make sense. These latter changes may not be necessary now that hash code is checked first thing, but can't hurt, and there will be devices without hash codes for a while. --- xwords4/common/model.c | 299 ++++++++++++++++++++++------------------ xwords4/common/model.h | 5 +- xwords4/common/mscore.c | 20 +-- xwords4/common/server.c | 73 ++++++---- 4 files changed, 223 insertions(+), 174 deletions(-) diff --git a/xwords4/common/model.c b/xwords4/common/model.c index 480adf488..1d5c5a885 100644 --- a/xwords4/common/model.c +++ b/xwords4/common/model.c @@ -71,6 +71,8 @@ static void buildModelFromStack( ModelCtxt* model, StackCtxt* stack, MovePrintFuncPre mpfpr, MovePrintFuncPost mpfpo, void* closure ); static void setPendingCounts( ModelCtxt* model, XP_S16 turn ); +static XP_S16 setContains( const TrayTileSet* tiles, Tile tile ); +static void removeTile( TrayTileSet* tiles, XP_U16 index ); static void loadPlayerCtxt( const ModelCtxt* model, XWStreamCtxt* stream, XP_U16 version, PlayerCtxt* pc ); static void writePlayerCtxt( const ModelCtxt* model, XWStreamCtxt* stream, @@ -290,6 +292,13 @@ model_destroy( ModelCtxt* model ) XP_FREE( model->vol.mpool, model ); } /* model_destroy */ +XP_U32 +model_getHash( const ModelCtxt* model ) +{ + XP_ASSERT( !!model->vol.stack ); + return stack_getHash( model->vol.stack ); +} + #ifdef STREAM_VERS_BIGBOARD void model_setSquareBonuses( ModelCtxt* model, XWBonusType* bonuses, XP_U16 nBonuses ) @@ -787,39 +796,31 @@ model_undoLatestMoves( ModelCtxt* model, PoolContext* pool, XP_U16 nMovesSought, XP_U16* turnP, XP_S16* moveNumP ) { StackCtxt* stack = model->vol.stack; - StackEntry entry; - XP_U16 turn = 0; - Tile blankTile = dict_getBlankTile( model_getDictionary(model) ); - XP_Bool success = XP_TRUE; + XP_Bool success; XP_S16 moveSought = !!moveNumP ? *moveNumP : -1; - XP_U16 nMovesUndone; - XP_U16 nStackEntries; + XP_U16 nStackEntries = stack_getNEntries( stack ); - nStackEntries = stack_getNEntries( stack ); - if ( nStackEntries < nMovesSought ) { - return XP_FALSE; - } else if ( nStackEntries <= model->nPlayers ) { - return XP_FALSE; - } + if ( (0 <= moveSought && moveSought >= nStackEntries) + || ( nStackEntries < nMovesSought ) + || ( nStackEntries <= model->nPlayers ) ) { + success = XP_FALSE; + } else { + XP_U16 nMovesUndone = 0; + XP_U16 turn; + StackEntry entry; + Tile blankTile = dict_getBlankTile( model_getDictionary(model) ); - for ( nMovesUndone = 0; success && nMovesUndone < nMovesSought; ) { - - success = stack_popEntry( stack, &entry ); - if ( success ) { - ++nMovesUndone; - - if ( moveSought < 0 ) { - moveSought = entry.moveNum - 1; - } else if ( moveSought-- != entry.moveNum ) { - success = XP_FALSE; + for ( ; ; ) { + success = stack_popEntry( stack, &entry ); + if ( !success ) { break; } + ++nMovesUndone; turn = entry.playerNum; model_resetCurrentTurn( model, turn ); if ( entry.moveType == MOVE_TYPE ) { - /* get the tiles out of player's tray and back into the pool */ replaceNewTiles( model, pool, turn, &entry.u.move.newTiles); @@ -839,73 +840,87 @@ model_undoLatestMoves( ModelCtxt* model, PoolContext* pool, } } else if ( entry.moveType == PHONY_TYPE ) { - /* nothing to do, since nothing happened */ - } else { XP_ASSERT( entry.moveType == ASSIGN_TYPE ); success = XP_FALSE; break; } - } - } - /* Find the first MOVE still on the stack and highlight its tiles since - they're now the most recent move. Trades and lost turns ignored. */ - nStackEntries = stack_getNEntries( stack ); - for ( ; ; ) { - StackEntry entry; - if ( nStackEntries == 0 || - !stack_getNthEntry( stack, nStackEntries - 1, &entry ) ) { - break; - } - if ( entry.moveType == MOVE_TYPE ) { - XP_U16 nTiles = entry.u.move.moveInfo.nTiles; - XP_U16 col, row; - XP_U16* varies; - - if ( entry.u.move.moveInfo.isHorizontal ) { - row = entry.u.move.moveInfo.commonCoord; - varies = &col; - } else { - col = entry.u.move.moveInfo.commonCoord; - varies = &row; + /* exit if we've undone what we're supposed to. If the sought + stack height has been provided, use that. Otherwise go by move + count. */ + if ( 0 <= moveSought ) { + if ( moveSought == entry.moveNum ) { + break; + } + } else if ( nMovesSought == nMovesUndone ) { + break; } + } - while ( nTiles-- ) { - CellTile tile; - - *varies = entry.u.move.moveInfo.tiles[nTiles].varCoord; - tile = getModelTileRaw( model, col, row ); - setModelTileRaw( model, col, row, - (CellTile)(tile | PREV_MOVE_BIT) ); - notifyBoardListeners( model, entry.playerNum, col, row, - XP_FALSE ); + /* Find the first MOVE still on the stack and highlight its tiles since + they're now the most recent move. Trades and lost turns ignored. */ + for ( nStackEntries = stack_getNEntries( stack ); + 0 < nStackEntries; --nStackEntries ) { + StackEntry entry; + if ( !stack_getNthEntry( stack, nStackEntries - 1, &entry ) ) { + break; + } + if ( entry.moveType == ASSIGN_TYPE ) { + break; + } else if ( entry.moveType == MOVE_TYPE ) { + XP_U16 nTiles = entry.u.move.moveInfo.nTiles; + XP_U16 col, row; + XP_U16* varies; + + if ( entry.u.move.moveInfo.isHorizontal ) { + row = entry.u.move.moveInfo.commonCoord; + varies = &col; + } else { + col = entry.u.move.moveInfo.commonCoord; + varies = &row; + } + + while ( nTiles-- ) { + CellTile tile; + + *varies = entry.u.move.moveInfo.tiles[nTiles].varCoord; + tile = getModelTileRaw( model, col, row ); + setModelTileRaw( model, col, row, + (CellTile)(tile | PREV_MOVE_BIT) ); + notifyBoardListeners( model, entry.playerNum, col, row, + XP_FALSE ); + } + break; + } + } + + /* We fail if we didn't undo as many as requested UNLESS the lower + limit is the trumping target/test. */ + if ( 0 <= moveSought ) { + if ( moveSought != entry.moveNum ) { + success = XP_FALSE; } - break; - } else if ( entry.moveType == ASSIGN_TYPE ) { - break; } else { - --nStackEntries; /* look at the next one */ + if ( nMovesUndone != nMovesSought ) { + success = XP_FALSE; + } } - } - if ( nMovesUndone != nMovesSought ) { - success = XP_FALSE; - } - - if ( success ) { - if ( !!turnP ) { - *turnP = turn; - } - if ( !!moveNumP ) { - *moveNumP = entry.moveNum; - } - } else { - while ( nMovesUndone-- ) { - /* undo isn't enough here: pool's got tiles in it!! */ - XP_ASSERT( 0 ); - (void)stack_redo( stack, NULL ); + if ( success ) { + if ( !!turnP ) { + *turnP = turn; + } + if ( !!moveNumP ) { + *moveNumP = entry.moveNum; + } + } else { + while ( nMovesUndone-- ) { + /* redo isn't enough here: pool's got tiles in it!! */ + XP_ASSERT( 0 ); + (void)stack_redo( stack, NULL ); + } } } @@ -936,12 +951,6 @@ model_currentMoveToStream( ModelCtxt* model, XP_S16 turn, nColsNBits = NUMCOLS_NBITS_4; #endif -#ifdef STREAM_VERS_BIGBOARD - if ( STREAM_VERS_BIGBOARD <= stream_getVersion( stream ) ) { - stream_putU32( stream, stack_getHash(model->vol.stack) ); - } -#endif - XP_ASSERT( turn >= 0 ); player = &model->players[turn]; numTiles = player->nPending; @@ -985,48 +994,55 @@ model_makeTurnFromStream( ModelCtxt* model, XP_U16 playerNum, #endif model_resetCurrentTurn( model, playerNum ); -#ifdef STREAM_VERS_BIGBOARD - if ( STREAM_VERS_BIGBOARD <= stream_getVersion( stream ) ) { - XP_U32 hash = stream_getU32( stream ); - success = hash == stack_getHash( model->vol.stack ); - } -#endif if ( success ) { numTiles = (XP_U16)stream_getBits( stream, NTILES_NBITS ); XP_LOGF( "%s: numTiles=%d", __func__, numTiles ); - for ( ii = 0; ii < numTiles; ++ii ) { - XP_S16 foundAt; - Tile moveTile; - Tile tileFace = (Tile)stream_getBits( stream, TILE_NBITS ); - XP_U16 col = (XP_U16)stream_getBits( stream, nColsNBits ); - XP_U16 row = (XP_U16)stream_getBits( stream, nColsNBits ); - XP_Bool isBlank = stream_getBits( stream, 1 ); + Tile tileFaces[numTiles]; + XP_U16 cols[numTiles]; + XP_U16 rows[numTiles]; + XP_Bool isBlanks[numTiles]; + Tile moveTiles[numTiles]; + TrayTileSet curTiles = *model_getPlayerTiles( model, playerNum ); - /* This code gets called both for the server, which has all the - tiles in its tray, and for a client, which has "EMPTY" tiles - only. If it's the empty case, we stuff a real tile into the - tray before falling through to the normal case */ + for ( ii = 0; success && ii < numTiles; ++ii ) { + tileFaces[ii] = (Tile)stream_getBits( stream, TILE_NBITS ); + cols[ii] = (XP_U16)stream_getBits( stream, nColsNBits ); + rows[ii] = (XP_U16)stream_getBits( stream, nColsNBits ); + isBlanks[ii] = stream_getBits( stream, 1 ); - if ( isBlank ) { - moveTile = blank; + if ( isBlanks[ii] ) { + moveTiles[ii] = blank; } else { - moveTile = tileFace; + moveTiles[ii] = tileFaces[ii]; } - foundAt = model_trayContains( model, playerNum, moveTile ); - if ( foundAt == -1 ) { - XP_ASSERT( EMPTY_TILE == model_getPlayerTile(model, playerNum, 0)); - - (void)model_removePlayerTile( model, playerNum, -1 ); - model_addPlayerTile( model, playerNum, -1, moveTile ); + XP_S16 index = setContains( &curTiles, moveTiles[ii] ); + if ( 0 <= index ) { + removeTile( &curTiles, index ); + } else { + success = XP_FALSE; + } + } + + if ( success ) { + for ( ii = 0; ii < numTiles; ++ii ) { + XP_S16 foundAt = model_trayContains( model, playerNum, moveTiles[ii] ); + if ( foundAt == -1 ) { + XP_ASSERT( EMPTY_TILE == model_getPlayerTile(model, playerNum, + 0)); + /* Does this ever happen? */ + XP_LOGF( "%s: found empty tile and it's ok", __func__ ); + + (void)model_removePlayerTile( model, playerNum, -1 ); + model_addPlayerTile( model, playerNum, -1, moveTiles[ii] ); + } + + model_moveTrayToBoard( model, playerNum, cols[ii], rows[ii], foundAt, + tileFaces[ii] ); } - - model_moveTrayToBoard( model, playerNum, col, row, foundAt, tileFace ); } - } else { - XP_LOGF( "%s: dropping move on hash mismatch", __func__ ); } return success; } /* model_makeTurnFromStream */ @@ -1107,25 +1123,11 @@ model_countAllTrayTiles( ModelCtxt* model, XP_U16* counts, XP_S16 model_trayContains( ModelCtxt* model, XP_S16 turn, Tile tile ) { - PlayerCtxt* player; - XP_S16 i; - XP_S16 result = -1; - XP_ASSERT( turn >= 0 ); XP_ASSERT( turn < model->nPlayers ); - player = &model->players[turn]; - - /* search from top down so don't pull out of below divider */ - for ( i = player->trayTiles.nTiles - 1; i >= 0 ; --i ) { - Tile playerTile = player->trayTiles.tiles[i]; - if ( playerTile == tile ) { - result = i; - break; - } - } - - return result; + const TrayTileSet* tiles = model_getPlayerTiles( model, turn ); + return setContains( tiles, tile ); } /* model_trayContains */ XP_U16 @@ -1548,7 +1550,7 @@ commitTurn( ModelCtxt* model, XP_S16 turn, const TrayTileSet* newTiles, XP_U16 ii; PlayerCtxt* player; PendingTile* pt; - XP_S16 score; + XP_S16 score = -1; XP_Bool inLine, isHorizontal; const Tile* newTilesP; XP_U16 nTiles; @@ -1568,7 +1570,7 @@ commitTurn( ModelCtxt* model, XP_S16 turn, const TrayTileSet* newTiles, player = &model->players[turn]; if ( useStack ) { - MoveInfo moveInfo; + MoveInfo moveInfo = {0}; inLine = tilesInLine( model, turn, &isHorizontal ); XP_ASSERT( inLine ); normalizeMoves( model, turn, isHorizontal, &moveInfo ); @@ -1620,15 +1622,14 @@ commitTurn( ModelCtxt* model, XP_S16 turn, const TrayTileSet* newTiles, while ( nTiles-- ) { model_addPlayerTile( model, turn, -1, *newTilesP++ ); } - return score; } /* commitTurn */ -void +XP_Bool model_commitTurn( ModelCtxt* model, XP_S16 turn, TrayTileSet* newTiles ) { - (void)commitTurn( model, turn, newTiles, (XWStreamCtxt*)NULL, - (WordNotifierInfo*)NULL, XP_TRUE ); + XP_S16 score = commitTurn( model, turn, newTiles, NULL, NULL, XP_TRUE ); + return 0 <= score; } /* model_commitTurn */ /* Given a rack of new tiles and of old, remove all the old from the tray and @@ -2333,7 +2334,7 @@ model_getPlayersLastScore( ModelCtxt* model, XP_S16 player, switch ( entry.moveType ) { case MOVE_TYPE: scoreLastMove( model, &entry.u.move.moveInfo, - nEntries - which - 1, expl, explLen ); + nEntries - which, expl, explLen ); break; case TRADE_TYPE: nTiles = entry.u.trade.oldTiles.nTiles; @@ -2422,6 +2423,32 @@ writePlayerCtxt( const ModelCtxt* model, XWStreamCtxt* stream, } } /* writePlayerCtxt */ +static void +removeTile( TrayTileSet* tiles, XP_U16 index ) +{ + XP_U16 ii; + --tiles->nTiles; + for ( ii = index; ii < tiles->nTiles; ++ii ) { + tiles->tiles[ii] = tiles->tiles[ii+1]; + } +} + +static XP_S16 +setContains( const TrayTileSet* tiles, Tile tile ) +{ + XP_S16 result = -1; + XP_S16 ii; + /* search from top down so don't pull out of below divider */ + for ( ii = tiles->nTiles - 1; ii >= 0 ; --ii ) { + Tile playerTile = tiles->tiles[ii]; + if ( playerTile == tile ) { + result = ii; + break; + } + } + return result; +} + #ifdef CPLUS } #endif diff --git a/xwords4/common/model.h b/xwords4/common/model.h index 438a04a06..a5d62bba2 100644 --- a/xwords4/common/model.h +++ b/xwords4/common/model.h @@ -114,6 +114,7 @@ void model_writeToTextStream( const ModelCtxt* model, XWStreamCtxt* stream ); void model_setSize( ModelCtxt* model, XP_U16 boardSize ); void model_destroy( ModelCtxt* model ); +XP_U32 model_getHash( const ModelCtxt* model ); void model_setNPlayers( ModelCtxt* model, XP_U16 numPlayers ); XP_U16 model_getNPlayers( const ModelCtxt* model ); @@ -181,8 +182,8 @@ void model_getCurrentMoveTile( ModelCtxt* model, XP_S16 turn, XP_S16* index, Tile* tile, XP_U16* col, XP_U16* row, XP_Bool* isBlank ); -void model_commitTurn( ModelCtxt* model, XP_S16 player, - TrayTileSet* newTiles ); +XP_Bool model_commitTurn( ModelCtxt* model, XP_S16 player, + TrayTileSet* newTiles ); void model_commitRejectedPhony( ModelCtxt* model, XP_S16 player ); void model_makeTileTrade( ModelCtxt* model, XP_S16 player, const TrayTileSet* oldTiles, diff --git a/xwords4/common/mscore.c b/xwords4/common/mscore.c index 1131b984a..d9d1158ab 100644 --- a/xwords4/common/mscore.c +++ b/xwords4/common/mscore.c @@ -280,7 +280,7 @@ void normalizeMoves( ModelCtxt* model, XP_S16 turn, XP_Bool isHorizontal, MoveInfo* moveInfo ) { - XP_S16 lowCol, i, j, thisCol; /* unsigned is a problem on palm */ + XP_S16 lowCol, ii, jj, thisCol; /* unsigned is a problem on palm */ PlayerCtxt* player = &model->players[turn]; XP_U16 nTiles = player->nPending; XP_S16 lastTaken; @@ -291,27 +291,29 @@ normalizeMoves( ModelCtxt* model, XP_S16 turn, XP_Bool isHorizontal, moveInfo->nTiles = (XP_U8)nTiles; lastTaken = -1; - for ( i = 0; i < nTiles; ++i ) { + for ( ii = 0; ii < nTiles; ++ii ) { lowCol = 100; /* high enough to always be changed */ - for ( j = 0; j < nTiles; ++j ) { - pt = &player->pendingTiles[j]; + for ( jj = 0; jj < nTiles; ++jj ) { + pt = &player->pendingTiles[jj]; thisCol = isHorizontal? pt->col:pt->row; if (thisCol < lowCol && thisCol > lastTaken ) { lowCol = thisCol; - lowIndex = j; + lowIndex = jj; } } /* we've found the next to transfer (4 bytes smaller without a temp local ptr. */ pt = &player->pendingTiles[lowIndex]; lastTaken = lowCol; - moveInfo->tiles[i].varCoord = (XP_U8)lastTaken; + moveInfo->tiles[ii].varCoord = (XP_U8)lastTaken; - moveInfo->tiles[i].tile = pt->tile; + moveInfo->tiles[ii].tile = pt->tile; } - pt = &player->pendingTiles[0]; - moveInfo->commonCoord = isHorizontal? pt->row:pt->col; + if ( 0 < nTiles ) { + pt = &player->pendingTiles[0]; + moveInfo->commonCoord = isHorizontal? pt->row:pt->col; + } } /* normalizeMoves */ static XP_Bool diff --git a/xwords4/common/server.c b/xwords4/common/server.c index 05fcbaf0d..6a1a878af 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -1896,6 +1896,14 @@ sendMoveTo( ServerCtxt* server, XP_U16 devIndex, XP_U16 turn, stream = messageStreamWithHeader( server, devIndex, code ); +#ifdef STREAM_VERS_BIGBOARD + if ( STREAM_VERS_BIGBOARD <= stream_getVersion( stream ) ) { + XP_U32 hash = model_getHash( server->vol.model ); + // XP_LOGF( "%s: adding hash %x", __func__, (unsigned int)hash ); + stream_putU32( stream, hash ); + } +#endif + stream_putBits( stream, PLAYERNUM_NBITS, turn ); /* who made the move */ traySetToStream( stream, newTiles ); @@ -1934,33 +1942,44 @@ readMoveInfo( ServerCtxt* server, XWStreamCtxt* stream, TrayTileSet* newTiles, TrayTileSet* tradedTiles, XP_Bool* legalP ) { - XP_Bool success; - XP_U16 whoMoved = stream_getBits( stream, PLAYERNUM_NBITS ); + XP_Bool success = XP_TRUE; XP_Bool legalMove = XP_TRUE; XP_Bool isTrade; - traySetFromStream( stream, newTiles ); - success = pool_containsTiles( server->pool, newTiles ); - if ( success ) { - isTrade = stream_getBits( stream, 1 ); - - if ( isTrade ) { - traySetFromStream( stream, tradedTiles ); - XP_LOGF( "%s: got trade of %d tiles", __func__, - tradedTiles->nTiles ); - } else { - legalMove = stream_getBits( stream, 1 ); - success = model_makeTurnFromStream( server->vol.model, - whoMoved, stream ); - getPlayerTime( server, stream, whoMoved ); +#ifdef STREAM_VERS_BIGBOARD + if ( STREAM_VERS_BIGBOARD <= stream_getVersion( stream ) ) { + XP_U32 hashReceived = stream_getU32( stream ); + success = hashReceived == model_getHash( server->vol.model ); + if ( !success ) { + XP_LOGF( "%s: hash mismatch: dropping move", __func__ ); } - + } +#endif + if ( success ) { + XP_U16 whoMoved = stream_getBits( stream, PLAYERNUM_NBITS ); + traySetFromStream( stream, newTiles ); + success = pool_containsTiles( server->pool, newTiles ); if ( success ) { - pool_removeTiles( server->pool, newTiles ); + isTrade = stream_getBits( stream, 1 ); - *whoMovedP = whoMoved; - *isTradeP = isTrade; - *legalP = legalMove; + if ( isTrade ) { + traySetFromStream( stream, tradedTiles ); + XP_LOGF( "%s: got trade of %d tiles", __func__, + tradedTiles->nTiles ); + } else { + legalMove = stream_getBits( stream, 1 ); + success = model_makeTurnFromStream( server->vol.model, + whoMoved, stream ); + getPlayerTime( server, stream, whoMoved ); + } + + if ( success ) { + pool_removeTiles( server->pool, newTiles ); + + *whoMovedP = whoMoved; + *isTradeP = isTrade; + *legalP = legalMove; + } } } return success; @@ -2019,7 +2038,7 @@ makeMoveReportIf( ServerCtxt* server, XWStreamCtxt** wordsStream ) static XP_Bool reflectMoveAndInform( ServerCtxt* server, XWStreamCtxt* stream ) { - XP_Bool success = XP_TRUE; + XP_Bool success; ModelCtxt* model = server->vol.model; XP_U16 whoMoved; XP_U16 nTilesMoved = 0; /* trade case */ @@ -2068,11 +2087,11 @@ reflectMoveAndInform( ServerCtxt* server, XWStreamCtxt* stream ) server->vol.showPrevMove = XP_TRUE; mvStream = makeMoveReportIf( server, &wordsStream ); - model_commitTurn( model, whoMoved, &newTiles ); + success = model_commitTurn( model, whoMoved, &newTiles ); resetEngines( server ); } - if ( isLegalMove ) { + if ( success && isLegalMove ) { XP_U16 nTilesLeft = model_getNumTilesTotal( model, whoMoved ); if ( (gi->phoniesAction == PHONIES_DISALLOW) && (nTilesMoved > 0) ) { @@ -2106,7 +2125,6 @@ reflectMoveAndInform( ServerCtxt* server, XWStreamCtxt* stream ) util_requestTime( server->vol.util ); } } - LOG_RETURNF( "%d", success ); return success; } /* reflectMoveAndInform */ @@ -2483,7 +2501,8 @@ sendUndoToClientsExcept( ServerCtxt* server, XP_U16 skip, static XP_Bool reflectUndos( ServerCtxt* server, XWStreamCtxt* stream, XW_Proto code ) { - XP_U16 nUndone, lastUndone; + XP_U16 nUndone; + XP_S16 lastUndone; XP_U16 turn; ModelCtxt* model = server->vol.model; XP_Bool success = XP_TRUE; @@ -2492,7 +2511,7 @@ reflectUndos( ServerCtxt* server, XWStreamCtxt* stream, XW_Proto code ) lastUndone = stream_getU16( stream ); success = model_undoLatestMoves( model, server->pool, nUndone, &turn, - NULL ); + &lastUndone ); if ( success ) { if ( code == XWPROTO_UNDO_INFO_CLIENT ) { /* need to inform */