fix: Use fsrs config bool for set due date#4792
fix: Use fsrs config bool for set due date#4792Luc-Mcgrady wants to merge 2 commits intoankitects:mainfrom
Conversation
A detailed explanation why this occurs(it took me a lot of effort to figure it out) With the previous code, when memory states are absent, fsrs_enabled is false. So, the interval is calculated from let new_interval = …
} else if force_reset || !matches!(self.ctype, CardType::Review | CardType::Relearn) {
days_from_today.max(1)
} else {
self.interval.max(1)
};So, for a new card, the interval becomes If you use SDD on the same card again (before doing a review), memory states are still absent but it is a review card now. So, if If When this SDD'ed card is reviewed, the card has no usable revlog and a non-zero interval. So, memory_state_from_sm2 is used to determine the memory states (the long standing issue - open-spaced-repetition/fsrs4anki#675) anki/rslib/src/scheduler/fsrs/memory_state.rs Lines 410 to 420 in 5a9b54e The First FixUse FSRS config bool instead of memory states to know whether FSRS is enabled.This seems to be good change, not only for new cards but for any card that lacks memory states due to any reason. (#4035 (comment)) The Second FixHard-code the interval to 0 when SDD is used on a new cardThis is slightly controversial and may require discussion. Ideally, users should not use SDD on a new card because if they do, FSRS can't know the best way to schedule the cards. The users should either use Grade Now (if they did active recall outside Anki and want to reflect that in Anki) or use Learn now (if they want to do their first active recall within Anki). Learn Now isn't implemented in Anki yet, but it is available as an add-on (https://github.com/Ajatt-Tools/learn-now-button). I have also requested for its native implementation in Anki. By hard-coding the interval to 0, SDD behaves more like
The main problem is that users will continue to use SDD on new cards even when they should actually use Grade Now because they don't know that they shouldn't use SDD. So, what if we disable SDD on new cards and implement a feature similar to Learn now and then ask users to use either Grade now or Learn now if they try to use SDD on a new card? If we do this, we will need a new version of the Learn now (instead of just porting the add-on's version) that also allows scheduling the card to any day so that there is no loss of functionality when migrating from SDD to Learn now. |
|
To summarize, this seems to be mergeable in the current state if we are not worried about the users who continue to misuse SDD on new cards when they should actually use Grade Now. |
Linked issue (required)
fixes #4791
Summary / motivation (required)
Previously, using set due date on a new card was broken for FSRS because new cards didn't have a memory state which was how the set due date function determined if FSRS is enabled or not. See #4035 (comment).
Steps to reproduce (required, use N/A if not applicable)
How to test (required)
I followed the reproduction steps and instead, after using set due date the interval is always set to 0; as to not affect grade now.
Checklist (minimum)
./ninja checkor an equivalent relevant check locally.Details
Before / after behavior (optional)
Risk / compatibility / migration (optional)
If there are any ways I have not foreseen that an interval can be set to 0, then it may cause problems with this approach.
UI evidence (required for visual changes; otherwise N/A)
Scope