search

ripose-jp / Memento

An mpv-based video player for studying Japanese
https://ripose-jp.github.io/Memento/
GNU General Public License v2.0
489 stars 21 forks source link

Add to Anki delay results in incorrect context when switching #81

Closed Calvin-Xu closed 2 years ago

Calvin-Xu commented 2 years ago

During the interval between clicking on the add button for a term and the term is actually added to Anki (which can last seconds depending on media), changes to context can change the selection. This affects both {context} and {audio-context}, although I have not tested the others. Ideally, the context when the term is added should be used.

The delay also seems to show that when attempting to add another term before the previous term has finished adding, the new term would be added and previous term(s) would be dropped. I suppose this is not exactly a bug because Memento does not guarantee any behavior in this situation and to solve it might require queuing the add actions. It only happens in rare and specific circumstances that I think it might be low priority right now.

ripose-jp commented 2 years ago

I'm pretty certain this is a bug that was introduced since adding Forvo support. Currently there's a delay between fetching the sources and actually saving the values. Should be an easy fix.

Fortunately, this shouldn't be possible on v0.5.4-1 since everything is done synchronously.

Thanks for the report.

Calvin-Xu commented 2 years ago

Thank you. This is good to know.

On a completely unrelated note, are there any plans for https://github.com/ripose-jp/Memento/issues/65 in the near future?

ripose-jp commented 2 years ago

This should be fixed by 68b69fec8f64779a17f601c6d6096b0383007a06. Because it's a race condition, it's hard to verify for certain that everything is fixed, but the behavior I was able to cause on the version prior to this commit is not reproducible on this commit. I'll let you have the final say on when to close this.

https://github.com/ripose-jp/Memento/issues/65 in the near future?

This feature isn't difficult to implement at all, but honestly I don't really like it. It seems pointless to me when manual search works just fine for these cases. If stuff gets so bad, you can also always just manually edit your cards as well. I think the idea of subtitle being read only is much more intuitive.

Calvin-Xu commented 2 years ago

Thank you. I'll test some more and report anything or close this.

It seems pointless to me when manual search works just fine for these cases. If stuff gets so bad, you can also always just manually edit your cards as well. I think the idea of subtitle being read only is much more intuitive.

The manual search has worked great, but the obvious catch is that cloze cannot be populated, and it can end up being a lot of editing on the Anki side, especially if the sentences themselves are incorrect and if you need to fix each field for every card. Fixing the text first can propagate the changes all at once. Another point is that if the user is using any bitmap subs as reference when editing, it is easier to stay in Memento.

I agree that making subtitles editable can be troublesome and lead to feature creep like exporting edited subs, etc. As a middle ground, perhaps the edit can be confined entirely to the subtitlewidget, and is not persisted once the current subtitle changes? This obviously does not extend edits to context, but might be a okay compromise.