From 0d8fef5c56a6afbcdecbf9fc62b2d2a06781725b Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 17 Aug 2015 08:01:36 -0700 Subject: [PATCH 1/6] cleanup --- .../XWords4/src/org/eehouse/android/xw4/DelegateBase.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/DelegateBase.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/DelegateBase.java index f278985cb..d61882fa7 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/DelegateBase.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/DelegateBase.java @@ -528,10 +528,9 @@ public class DelegateBase implements DlgClickNotify, m_dlgDelegate.eventOccurred( event, args ); break; default: - if ( BuildConfig.DEBUG ) { - DbgUtils.logf( "DelegateBase.eventOccurred(event=%s) (DROPPED)", - event.toString() ); - } + DbgUtils.logdf( "DelegateBase.eventOccurred(event=%s) (DROPPED)", + event.toString() ); + break; } if ( 0 != fmtId ) { From ff3d5871793b574b50e82537daac7840b86f018c Mon Sep 17 00:00:00 2001 From: Eric House Date: Tue, 18 Aug 2015 06:40:31 -0700 Subject: [PATCH 2/6] really only post messages sent toast when user chose the menuitem --- .../XWords4/src/org/eehouse/android/xw4/jni/JNIThread.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/JNIThread.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/JNIThread.java index f7b41574c..79d0de388 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/JNIThread.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/JNIThread.java @@ -523,11 +523,11 @@ public class JNIThread extends Thread { break; case CMD_RESEND: - boolean force = ((Boolean)args[0]).booleanValue(); int nSent = - XwJNI.comms_resendAll( m_jniGamePtr, force, + XwJNI.comms_resendAll( m_jniGamePtr, + ((Boolean)args[0]).booleanValue(), ((Boolean)args[1]).booleanValue() ); - if ( force ) { + if ( ((Boolean)args[2]).booleanValue() ) { Message.obtain(m_handler, MSGS_SENT, nSent).sendToTarget(); } break; From a11e5beded805eb4555e2c070a4ec83e9e08f4a4 Mon Sep 17 00:00:00 2001 From: Eric House Date: Tue, 18 Aug 2015 06:40:50 -0700 Subject: [PATCH 3/6] goes with prev commit --- .../XWords4/src/org/eehouse/android/xw4/BoardDelegate.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardDelegate.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardDelegate.java index 385c60fa6..cde6b27fc 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardDelegate.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardDelegate.java @@ -906,7 +906,7 @@ public class BoardDelegate extends DelegateBase break; case R.id.board_menu_game_resend: - m_jniThread.handle( JNICmd.CMD_RESEND, true, false ); + m_jniThread.handle( JNICmd.CMD_RESEND, true, false, true ); break; case R.id.gamel_menu_checkmoves: @@ -2104,7 +2104,8 @@ public class BoardDelegate extends DelegateBase } if ( 0 < m_connTypes.size() ) { - m_jniThread.handle( JNIThread.JNICmd.CMD_RESEND, force, true ); + m_jniThread.handle( JNIThread.JNICmd.CMD_RESEND, force, true, + false ); } } From 884eda06032970ba86126085d464aba5d65e8a35 Mon Sep 17 00:00:00 2001 From: Eric House Date: Tue, 18 Aug 2015 08:00:22 -0700 Subject: [PATCH 4/6] cleanup: use temp ptr to avoid too many dereferences --- xwords4/common/engine.c | 86 +++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/xwords4/common/engine.c b/xwords4/common/engine.c index b1aa40c62..1b3ab0763 100644 --- a/xwords4/common/engine.c +++ b/xwords4/common/engine.c @@ -1146,23 +1146,24 @@ considerScoreWordHasBlanks( EngineCtxt* engine, XP_U16 blanksLeft, static void saveMoveIfQualifies( EngineCtxt* engine, PossibleMove* posmove ) { - XP_S16 mostest = 0; + XP_S16 mostest; XP_S16 cmpVal; XP_Bool usePrev = engine->usePrev; XP_Bool foundEmpty = XP_FALSE; + MoveIterationData* miData = &engine->miData; if ( 1 == engine->nMovesToSave ) { /* only saving one */ mostest = 0; } else { mostest = -1; /* we're not interested if we've seen this */ - cmpVal = CMPMOVES( posmove, &engine->miData.lastSeenMove ); + cmpVal = CMPMOVES( posmove, &miData->lastSeenMove ); if ( !usePrev && cmpVal >= 0 ) { - /* XP_LOGF( "%s: dropping %d: higher than %d", __func__, */ - /* posmove->score, engine->miData.lastSeenMove.score ); */ + /* XP_LOGF( "%s: dropping %d: >= %d", __func__, */ + /* posmove->score, miData->lastSeenMove.score ); */ } else if ( usePrev && cmpVal <= 0 ) { - /* XP_LOGF( "%s: dropping %d: lower than %d", __func__, */ - /* posmove->score, engine->miData.lastSeenMove.score ); */ + /* XP_LOGF( "%s: dropping %d: <= %d", __func__, */ + /* posmove->score, miData->lastSeenMove.score ); */ } else { XP_S16 ii; /* terminate i at 1 because mostest starts at 0 */ @@ -1178,19 +1179,19 @@ saveMoveIfQualifies( EngineCtxt* engine, PossibleMove* posmove ) /* 1/20/2001 I don't see that this assertion is valid. I simply don't understand why it isn't tripped all the time in the old crosswords. */ - /* XP_ASSERT( (engine->miData.lastSeenMove.score == 0x7fff) */ - /* || (engine->miData.savedMoves[i].score */ + /* XP_ASSERT( (miData->lastSeenMove.score == 0x7fff) */ + /* || (miData->savedMoves[i].score */ /* <= posmove->score) ); */ - if ( 0 == engine->miData.savedMoves[ii].score ) { + if ( 0 == miData->savedMoves[ii].score ) { foundEmpty = XP_TRUE; mostest = ii; break; } else if ( -1 == mostest ) { mostest = ii; } else { - cmpVal = CMPMOVES( &engine->miData.savedMoves[mostest], - &engine->miData.savedMoves[ii] ); + cmpVal = CMPMOVES( &miData->savedMoves[mostest], + &miData->savedMoves[ii] ); if ( !usePrev && cmpVal > 0 ) { mostest = ii; } else if ( usePrev && cmpVal < 0 ) { @@ -1204,14 +1205,14 @@ saveMoveIfQualifies( EngineCtxt* engine, PossibleMove* posmove ) while ( mostest >= 0 ) { /* while: so we can break */ /* record the score we're dumping. No point in considering any scores lower than this for the rest of this round. */ - /* engine->miData.lowestSavedScore = */ - /* engine->miData.savedMoves[lowest].score; */ + /* miData->lowestSavedScore = */ + /* miData->savedMoves[lowest].score; */ /* XP_DEBUGF( "lowestSavedScore now %d\n", */ - /* engine->miData.lowestSavedScore ); */ + /* miData->lowestSavedScore ); */ if ( foundEmpty ) { /* we're good */ } else { - cmpVal = CMPMOVES( posmove, &engine->miData.savedMoves[mostest]); + cmpVal = CMPMOVES( posmove, &miData->savedMoves[mostest]); if ( !usePrev && cmpVal <= 0 ) { break; } else if ( usePrev && cmpVal >= 0 ) { @@ -1219,10 +1220,10 @@ saveMoveIfQualifies( EngineCtxt* engine, PossibleMove* posmove ) } } /* XP_LOGF( "saving move with score %d at %d (replacing %d)\n", */ - /* posmove->score, mostest, */ - /* engine->miData.savedMoves[mostest].score ); */ - XP_MEMCPY( &engine->miData.savedMoves[mostest], posmove, - sizeof(engine->miData.savedMoves[mostest]) ); + /* posmove->score, mostest, */ + /* miData->savedMoves[mostest].score ); */ + XP_MEMCPY( &miData->savedMoves[mostest], posmove, + sizeof(miData->savedMoves[mostest]) ); break; } } /* saveMoveIfQualifies */ @@ -1230,15 +1231,16 @@ saveMoveIfQualifies( EngineCtxt* engine, PossibleMove* posmove ) static void set_search_limits( EngineCtxt* engine ) { + MoveIterationData* miData = &engine->miData; /* If we're going to be searching backwards we want our highest cached move as the limit; otherwise the lowest */ - if ( 0 < engine->miData.nInMoveCache ) { + if ( 0 < miData->nInMoveCache ) { XP_U16 srcIndx = engine->usePrev - ? engine->nMovesToSave-1 : engine->miData.bottom; - XP_MEMCPY( &engine->miData.lastSeenMove, - &engine->miData.savedMoves[srcIndx], - sizeof(engine->miData.lastSeenMove) ); - //engine->miData.lowestSavedScore = 0; + ? engine->nMovesToSave-1 : miData->bottom; + XP_MEMCPY( &miData->lastSeenMove, + &miData->savedMoves[srcIndx], + sizeof(miData->lastSeenMove) ); + //miData->lowestSavedScore = 0; } else { /* we're doing this for first time */ engine_reset( engine ); @@ -1249,41 +1251,40 @@ static void init_move_cache( EngineCtxt* engine ) { XP_U16 nInMoveCache = engine->nMovesToSave; + MoveIterationData* miData = &engine->miData; XP_U16 ii; XP_ASSERT( engine->nMovesToSave == NUM_SAVED_ENGINE_MOVES ); for ( ii = 0; ii < NUM_SAVED_ENGINE_MOVES; ++ii ) { - if ( 0 == engine->miData.savedMoves[ii].score ) { + if ( 0 == miData->savedMoves[ii].score ) { --nInMoveCache; } else { break; } } - engine->miData.nInMoveCache = nInMoveCache; - engine->miData.bottom = NUM_SAVED_ENGINE_MOVES - nInMoveCache; + miData->nInMoveCache = nInMoveCache; + miData->bottom = NUM_SAVED_ENGINE_MOVES - nInMoveCache; - if ( engine->usePrev ) { - engine->miData.curCacheIndex = - NUM_SAVED_ENGINE_MOVES - nInMoveCache - 1; - } else { - engine->miData.curCacheIndex = NUM_SAVED_ENGINE_MOVES; - } + miData->curCacheIndex = engine->usePrev + ? NUM_SAVED_ENGINE_MOVES - nInMoveCache - 1 + : NUM_SAVED_ENGINE_MOVES; } static PossibleMove* next_from_cache( EngineCtxt* engine ) { + MoveIterationData* miData = &engine->miData; PossibleMove* move; if ( move_cache_empty( engine ) ) { move = NULL; } else { if ( engine->usePrev ) { - ++engine->miData.curCacheIndex; + ++miData->curCacheIndex; } else { - --engine->miData.curCacheIndex; + --miData->curCacheIndex; } - move = &engine->miData.savedMoves[engine->miData.curCacheIndex]; + move = &miData->savedMoves[miData->curCacheIndex]; } return move; } @@ -1309,21 +1310,22 @@ scoreQualifies( EngineCtxt* engine, XP_U16 score ) { XP_Bool qualifies = XP_FALSE; XP_Bool usePrev = engine->usePrev; + MoveIterationData* miData = &engine->miData; - if ( usePrev && score < engine->miData.lastSeenMove.score ) { + if ( usePrev && score < miData->lastSeenMove.score ) { /* drop it */ - } else if ( !usePrev && score > engine->miData.lastSeenMove.score - /* || (score < engine->miData.lowestSavedScore) */ ) { + } else if ( !usePrev && score > miData->lastSeenMove.score + /* || (score < miData->lowestSavedScore) */ ) { /* drop it */ } else { XP_S16 ii; - PossibleMove* savedMoves = engine->miData.savedMoves; + PossibleMove* savedMoves = miData->savedMoves; /* Look at each saved score, and return true as soon as one's found with a lower or equal score to this. As an optimization, consider remembering what the lowest score is *once there are NUM_SAVED_ENGINE_MOVES moves in here* and doing a quick test on that. Or better, keeping the list in sorted order. */ - for ( ii = 0, savedMoves = engine->miData.savedMoves; + for ( ii = 0, savedMoves = miData->savedMoves; ii < engine->nMovesToSave; ++ii, ++savedMoves ) { if ( savedMoves->score == 0 ) { /* empty slot */ qualifies = XP_TRUE; From 6d7123c27a2924084eec36e10728c159848d4065 Mon Sep 17 00:00:00 2001 From: Eric House Date: Wed, 19 Aug 2015 06:05:18 -0700 Subject: [PATCH 5/6] fix curses app for standalone games (at least to not crash) --- xwords4/linux/cursesmain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xwords4/linux/cursesmain.c b/xwords4/linux/cursesmain.c index 4a98eacae..3a538baaf 100644 --- a/xwords4/linux/cursesmain.c +++ b/xwords4/linux/cursesmain.c @@ -2119,7 +2119,7 @@ cursesmain( XP_Bool isServer, LaunchParams* params ) #ifndef XWFEATURE_STANDALONE_ONLY /* send any events that need to get off before the event loop begins */ - if ( !isServer ) { + if ( !!cGlobals->game.comms && !isServer ) { (void)server_initClientConnection( cGlobals->game.server, mem_stream_make( MEMPOOL params->vtMgr, From 255ca56ed73a1532db7ed66d5bdc05e4557de701 Mon Sep 17 00:00:00 2001 From: Eric House Date: Wed, 19 Aug 2015 07:43:00 -0700 Subject: [PATCH 6/6] when no moves found, reset engine and try again. Works around bug where you do "hint", then "prev hint" and get told there are no moves found only to have the next "prev hint" succeed. This is a hack, but the right fix is eluding me, and will certainly be riskier. --- xwords4/common/engine.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/xwords4/common/engine.c b/xwords4/common/engine.c index 1b3ab0763..334c69342 100644 --- a/xwords4/common/engine.c +++ b/xwords4/common/engine.c @@ -345,9 +345,6 @@ chooseMove( EngineCtxt* engine, PossibleMove** move ) result = (NULL != chosen) && (chosen->score > 0); - if ( !result ) { - engine_reset( engine ); - } return result; } /* chooseMove */ @@ -381,7 +378,7 @@ normalizeIQ( EngineCtxt* engine, XP_U16 iq ) XP_Bool engine_findMove( EngineCtxt* engine, const ModelCtxt* model, XP_U16 turn, const Tile* tiles, - XP_U16 nTiles, XP_Bool usePrev, + const XP_U16 nTiles, XP_Bool usePrev, #ifdef XWFEATURE_BONUSALL XP_U16 allTilesBonus, #endif @@ -394,7 +391,9 @@ engine_findMove( EngineCtxt* engine, const ModelCtxt* model, XP_Bool result = XP_TRUE; XP_U16 star_row; XP_Bool canMove = XP_FALSE; + XP_Bool isRetry = XP_FALSE; + retry: engine->nTilesMax = XP_MIN( MAX_TRAY_TILES, nTiles ); #ifdef XWFEATURE_BONUSALL engine->allTilesBonus = allTilesBonus; @@ -542,6 +541,22 @@ engine_findMove( EngineCtxt* engine, const ModelCtxt* model, newMove->nTiles = 0; } + /* Gross hack alert: there's an elusive bug in move cacheing that means + when we move forward or back from the highest-scoring move to the + lowest (or vice-versa) no move is found. But the next try succeeds, + because an engine_reset clears the state that makes that happen. So as + a workaround, try doing that when no moves are found. If none is found + for some other reason, e.g. no tiles, at least the search should be + quick. */ + if ( !canMove ) { + engine_reset( engine ); + if ( !isRetry ) { + isRetry = XP_TRUE; + XP_LOGF( "%s: no moves found so retrying", __func__ ); + goto retry; + } + } + *canMoveP = canMove; return result; } /* engine_findMove */