Get first card quicker during reviews

AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.java

@@ -26,13 +26,16 @@
 import android.database.Cursor;
 import android.net.Uri;
 import androidx.test.rule.GrantPermissionRule;
+import timber.log.Timber;
+
 import android.util.Log;
 
 import com.ichi2.anki.AbstractFlashcardViewer;
 import com.ichi2.anki.AnkiDroidApp;
 import com.ichi2.anki.CollectionHelper;
 import com.ichi2.anki.FlashCardsContract;
 import com.ichi2.anki.exception.ConfirmModSchemaException;
+import com.ichi2.async.CollectionTask;
 import com.ichi2.libanki.Card;
 import com.ichi2.libanki.Collection;
 import com.ichi2.libanki.Consts;
@@ -790,7 +793,10 @@ public void testQueryNextCard() {
         for(int i = 0; i < 10; i++) {//minimizing fails, when sched.reset() randomly chooses between multiple cards
             col.reset();
             nextCard = sched.getCard();
+            CollectionTask.waitToFinish();
             if(nextCard.note().getId() == noteID && nextCard.getOrd() == cardOrd)break;
+            CollectionTask.waitToFinish();
+
         }
         assertNotNull("Check that there actually is a next scheduled card", nextCard);
         assertEquals("Check that received card and actual card have same note id", nextCard.note().getId(), noteID);
@@ -802,7 +808,7 @@ public void testQueryNextCard() {
      * Test that query for the next card in the schedule returns a valid result WITH a deck selector
      */
     @Test
-    public void testQueryCardFromCertainDeck(){
+    public synchronized void testQueryCardFromCertainDeck(){
         long deckToTest = mTestDeckIds.get(0);
         String deckSelector = "deckID=?";
         String[] deckArguments = {Long.toString(deckToTest)};
@@ -827,6 +833,7 @@ public void testQueryCardFromCertainDeck(){
                 col.reset();
                 nextCard = sched.getCard();
                 if(nextCard.note().getId() == noteID && nextCard.getOrd() == cardOrd)break;
+                try { Thread.sleep(500); } catch (Exception e) { Timber.e(e); } // Reset counts is executed in background.
             }
             assertNotNull("Check that there actually is a next scheduled card", nextCard);
             assertEquals("Check that received card and actual card have same note id", nextCard.note().getId(), noteID);

AnkiDroid/src/main/java/com/ichi2/async/CollectionTask.java

@@ -130,7 +130,8 @@ public enum TASK_TYPE {
         FIND_EMPTY_CARDS,
         CHECK_CARD_SELECTION,
         LOAD_COLLECTION_COMPLETE,
-        PRELOAD_NEXT_CARD
+        PRELOAD_NEXT_CARD,
+        RESET,
     }
 
     /**
@@ -488,6 +489,10 @@ protected TaskData actualDoInBackground(TaskData param) {
                 doInBackgroundPreloadNextCard();
                 break;
 
+            case RESET:
+                doInBackgroundReset();
+                break;
+
             default:
                 Timber.e("unknown task type: %s", mType);
         }
@@ -1928,6 +1933,10 @@ public void doInBackgroundLoadCollectionComplete() {
         }
     }
 
+    public void doInBackgroundReset() {
+        getCol().getSched().reset();
+    }
+
 
     /**
      * Helper class for allowing inner function to publish progress of an AsyncTask.

AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.java

@@ -50,9 +50,12 @@ public abstract class AbstractSched {
      */
     public abstract @Nullable Card getCard();
 
-    /** Let the scheduler knows that the selected deck potentially changed and all pre-computed data (queue and counts)
-     * should be discarded. Should be called after getCard if the card is not answered. */
-    protected abstract void reset();
+    /**
+     * The collection saves some numbers such as counts, queues of cards to review, queues of decks potentially having some cards.
+     * Reset all of this and compute from scratch. This occurs because anything else than the sequence of getCard/answerCard did occur.
+     */
+    // Should ideally be protected. It's public only because CollectionTask should call it when the scheduler planned this task
+    public abstract void reset();
 
     /** Check whether we are a new day, and update if so. */
     public abstract void _updateCutoff();
@@ -438,11 +441,6 @@ public enum UnburyType {
      */
     public abstract void setContext(@Nullable WeakReference<Activity> contextReference);
 
-    /**
-     * @return The counts, after having reseted them
-     */
-    public abstract @NonNull int[] recalculateCounts();
-
     /**
      * Change the maximal number shown in counts.
      * @param reportLimit A maximal number of cards added in the queue at once.

AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java

@@ -258,7 +258,7 @@ private void unburyCardsForDeck(@NonNull List<Long> allDecks) {
     @Override
     protected @Nullable Card _getCard() {
         // learning card due?
-        @Nullable Card c = _getLrnCard();
+        @Nullable Card c = _getLrnCard(false);
         if (c != null) {
             return c;
         }
@@ -348,7 +348,7 @@ protected void _resetLrnQueue() {
     // sub-day learning
     @Override
     protected boolean _fillLrn() {
-        if (mLrnCount == 0) {
+        if (mHaveCounts && mLrnCount == 0) {
             return false;
         }
         if (!mLrnQueue.isEmpty()) {
@@ -672,7 +672,7 @@ protected boolean _fillRev(boolean allowSibling) {
         if (!mRevQueue.isEmpty()) {
             return true;
         }
-        if (mRevCount == 0) {
+        if (mHaveCounts && mRevCount == 0) {
             return false;
         }
         while (!mRevDids.isEmpty()) {
@@ -722,11 +722,14 @@ protected boolean _fillRev(boolean allowSibling) {
             // nothing left in the deck; move to next
             mRevDids.remove();
         }
-        // Since we didn't get a card and the count is non-zero, we
-        // need to check again for any cards that were removed from
-        // the queue but not buried
-        _resetRev();
-        return _fillRev(true);
+        if (mHaveCounts && mRevCount != 0) {
+            // if we didn't get a card but the count is non-zero,
+            // we need to check again for any cards that were
+            // removed from the queue but not buried
+            _resetRev();
+            return _fillRev(true);
+        }
+        return false;
     }
 
 

AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java

@@ -33,6 +33,7 @@
 
 import com.ichi2.anki.R;
 import com.ichi2.async.CancelListener;
+import com.ichi2.async.CollectionTask;
 import com.ichi2.libanki.Card;
 import com.ichi2.libanki.Collection;
 import com.ichi2.libanki.Consts;
@@ -69,6 +70,7 @@
 import static com.ichi2.libanki.sched.Counts.Queue.*;
 import static com.ichi2.libanki.sched.Counts.Queue;
 import static com.ichi2.libanki.stats.Stats.SECONDS_PER_DAY;
+import static com.ichi2.async.CollectionTask.TASK_TYPE.*;
 
 @SuppressWarnings({"PMD.ExcessiveClassLength", "PMD.AvoidThrowingRawExceptionTypes","PMD.AvoidReassigningParameters",
                     "PMD.NPathComplexity","PMD.MethodNamingConventions","PMD.AvoidBranchingStatementAsLastInLoop",
@@ -159,14 +161,17 @@ public SchedV2(@NonNull Collection col) {
      */
     public @Nullable Card getCard() {
         _checkDay();
-        // check day deal with cutoff if required. No need to do it in resets
-        if (!mHaveCounts) {
-            resetCounts(false);
-        }
         if (!mHaveQueues) {
             resetQueues(false);
         }
         @Nullable Card card = _getCard();
+        if (card == null && !mHaveCounts) {
+            // maybe we didn't refill queues because counts were not
+            // set. This could only occur if the only card is a buried
+            // sibling. So let's try to set counts and check again.
+            reset();
+            card = _getCard();
+        }
         if (card != null) {
             mCol.log(card);
             incrReps();
@@ -182,6 +187,10 @@ public SchedV2(@NonNull Collection col) {
         } else {
             discardCurrentCard();
         }
+        if (!mHaveCounts) {
+            // Need to reset queues once counts are reset
+            CollectionTask.launchCollectionTask(RESET);
+        }
         return card;
     }
 
@@ -198,7 +207,7 @@ public void deferReset() {
         discardCurrentCard();
     }
 
-    protected void reset() {
+    public void reset() {
         _updateCutoff();
         resetCounts(false);
         resetQueues(false);
@@ -616,7 +625,7 @@ public List<DeckDueTreeNode> deckDueTree(@Nullable CancelListener cancelListener
      */
     protected @Nullable Card _getCard() {
         // learning card due?
-        @Nullable Card c = _getLrnCard();
+        @Nullable Card c = _getLrnCard(false);
         if (c != null) {
             return c;
         }
@@ -783,7 +792,7 @@ private boolean _fillNew(boolean allowSibling) {
         if (!mNewQueue.isEmpty()) {
             return true;
         }
-        if (mNewCount == 0) {
+        if (mHaveCounts && mNewCount == 0) {
             return false;
         }
         while (!mNewDids.isEmpty()) {
@@ -818,11 +827,14 @@ private boolean _fillNew(boolean allowSibling) {
             // nothing left in the deck; move to next
             mNewDids.remove();
         }
-        // if we didn't get a card, since the count is non-zero, we
-        // need to check again for any cards that were removed
-        // from the queue but not buried
-        _resetNew();
-        return _fillNew(true);
+        if (mHaveCounts && mNewCount != 0) {
+            // if we didn't get a card but the count is non-zero,
+            // we need to check again for any cards that were
+            // removed from the queue but not buried
+            _resetNew();
+            return _fillNew(true);
+        }
+        return false;
     }
 
 
@@ -854,7 +866,7 @@ private void _updateNewCardRatio() {
      * @return True if it's time to display a new card when distributing.
      */
     protected boolean _timeForNewCard() {
-        if (mNewCount == 0) {
+        if (mHaveCounts && mNewCount == 0) {
             return false;
         }
         @Consts.NEW_CARD_ORDER int spread = mCol.getConf().getInt("newSpread");
@@ -863,6 +875,8 @@ protected boolean _timeForNewCard() {
         } else if (spread == Consts.NEW_CARDS_FIRST) {
             return true;
         } else if (mNewCardModulus != 0) {
+            // if the counter has not yet been resetted, this value is
+            // random. This will occur only for the first card of review.
             return (mReps != 0 && (mReps % mNewCardModulus == 0));
         } else {
             return false;
@@ -1001,7 +1015,7 @@ protected void _resetLrnQueue() {
     // sub-day learning
     // Overridden: a single kind of queue in V1
     protected boolean _fillLrn() {
-        if (mLrnCount == 0) {
+        if (mHaveCounts && mLrnCount == 0) {
             return false;
         }
         if (!mLrnQueue.isEmpty()) {
@@ -1031,11 +1045,6 @@ protected boolean _fillLrn() {
     }
 
 
-    protected @Nullable Card _getLrnCard() {
-        return _getLrnCard(false);
-    }
-
-
     // Overidden: no _maybeResetLrn in V1
     protected @Nullable Card _getLrnCard(boolean collapse) {
         _maybeResetLrn(collapse && mLrnCount == 0);
@@ -1069,7 +1078,7 @@ protected boolean _preloadLrnCard(boolean collapse) {
 
     // daily learning
     protected boolean _fillLrnDay() {
-        if (mLrnCount == 0) {
+        if (mHaveCounts && mLrnCount == 0) {
             return false;
         }
         if (!mLrnDayQueue.isEmpty()) {
@@ -1528,7 +1537,7 @@ protected boolean _fillRev(boolean allowSibling) {
         if (!mRevQueue.isEmpty()) {
             return true;
         }
-        if (mRevCount == 0) {
+        if (mHaveCounts && mRevCount == 0) {
             return false;
         }
         int lim = Math.min(mQueueLimit, _currentRevLimit(true));
@@ -1562,11 +1571,14 @@ protected boolean _fillRev(boolean allowSibling) {
                 return true;
             }
         }
-        // since we didn't get a card and the count is non-zero, we
-        // need to check again for any cards that were removed from
-        // the queue but not buried
-        _resetRev();
-        return _fillRev(true);
+        if (mHaveCounts && mRevCount != 0) {
+            // if we didn't get a card but the count is non-zero,
+            // we need to check again for any cards that were
+            // removed from the queue but not buried
+            _resetRev();
+            return _fillRev(true);
+        }
+        return false;
     }
 
 
@@ -2982,14 +2994,6 @@ public void setContext(@Nullable WeakReference<Activity> contextReference) {
     }
 
     /** not in libAnki. Added due to #5666: inconsistent selected deck card counts on sync */
-    @Override
-    public @NonNull int[] recalculateCounts() {
-        _resetLrnCount();
-        _resetNewCount();
-        _resetRevCount();
-        return new int[] { mNewCount, mLrnCount, mRevCount };
-    }
-
     @Override
     public void setReportLimit(int reportLimit) {
         this.mReportLimit = reportLimit;

AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java

@@ -36,6 +36,7 @@
 
 import com.ichi2.libanki.Deck;
 import com.ichi2.libanki.DeckConfig;
+import com.ichi2.libanki.sched.Counts;
 import com.ichi2.utils.JSONArray;
 import com.ichi2.utils.JSONException;
 import com.ichi2.utils.JSONObject;
@@ -414,9 +415,11 @@ public JSONObject sanityCheck() {
             //#5666 - not in libAnki
             //We modified mReportLimit inside the scheduler, and this causes issues syncing dynamic decks.
             AbstractSched syncScheduler = mCol.createScheduler(SYNC_SCHEDULER_REPORT_LIMIT);
-            for (int c : syncScheduler.recalculateCounts()) {
-                counts.put(c);
-            }
+            syncScheduler.resetCounts();
+            Counts counts_ = syncScheduler.counts();
+            counts.put(counts_.getNew());
+            counts.put(counts_.getLrn());
+            counts.put(counts_.getRev());
             check.put(counts);
             check.put(mCol.getDb().queryScalar("SELECT count() FROM cards"));
             check.put(mCol.getDb().queryScalar("SELECT count() FROM notes"));

AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.java

@@ -441,4 +441,10 @@ protected FragmentTestActivity openDialogFragmentUsingActivity(DialogFragment me
         return activity;
     }
 
+    protected Card getCard() {
+        Card card = getCol().getSched().getCard();
+        advanceRobolectricLooperWithSleep();
+        return card;
+    }
+
 }

AnkiDroid/src/test/java/com/ichi2/libanki/FinderTest.java

@@ -300,11 +300,11 @@ public void test_findCards() {
         if (!isNearCutoff(col)) {
             assertEquals(0, col.findCards("rated:1:1").size());
             assertEquals(0, col.findCards("rated:1:2").size());
-            c = col.getSched().getCard();
+            c = getCard();
             col.getSched().answerCard(c, 2);
             assertEquals(0, col.findCards("rated:1:1").size());
             assertEquals(1, col.findCards("rated:1:2").size());
-            c = col.getSched().getCard();
+            c = getCard();
             col.getSched().answerCard(c, 1);
             assertEquals(1, col.findCards("rated:1:1").size());
             assertEquals(1, col.findCards("rated:1:2").size());