feat: add adaptive symbol typing trainer - #144
Conversation
kissetfall
left a comment
There was a problem hiding this comment.
The trainer mechanics and tests are a solid start, but the generated pool does not yet represent what the loaded keyboard actually types.
typing_trainer_symbols::direct_and_shifted_symbols duplicates a hard-coded US/QWERTY output map. With a Russian active layout, for example, KC_A produces ф while the trainer expects a / A, so the exercise becomes incorrect or impossible. printable_symbols_from_layout also accepts only KeyBinding::Vial and skips every KeyBinding::Rmk, including native Universal Symbols.
Please move printable-output derivation into the existing keycode/legend owner (or add a shared helper there), make it use the selected key legend/input mapping, and handle supported RMK-native printable actions. Add regression coverage for at least Russian output and native Universal Symbols, alongside English/QWERTY.
Please also avoid rebuilding the pool from every layer, sorting it, and cloning the complete adaptive stats map on every UI frame. Recompute/cache this data when the layout or relevant setting changes. Finally, clarify the user-facing material definition: the current “Symbols” pool includes letters and digits as well as punctuation; either make that explicit in the UI or filter it to the intended material.
A synthetic merge onto current main passed 570/570 tests, i18n, and Clippy, so these are behavior/ownership blockers rather than build failures.
|
Reworked all four points, then reviewed the result again locally and fixed what that review turned up. The branch is rebased onto current Printable output moved to the keycode owner. RMK-native actions handled. Input mapping is user-selectable. The language dropdown is shown in symbol training as well and doubles as the input mapping the pool is derived from, so an exercise never asks for characters the active layout cannot type. Switching material also moves the count to the nearest preset the new material offers — otherwise the dropdown showed 50 while the run generated 10. No per-frame work. The pool is rebuilt only when the keymap or the selected language changed. I kept that as a comparison against a cached snapshot rather than invalidating at each of the ~20 places that assign or mutate Material wording. The pool is deliberately every printable key across the layers, letters and digits included, so the toggle is labelled "layer keys" with a tooltip saying exactly that. Its width was also missing from the controls container, which clipped the toggle away entirely — symbol training could not be enabled at all. One fix outside the feature came along: Regression coverage: Russian output (letters, shifted digits, Verification: |
aeb57d6 to
d04c1cd
Compare
kissetfall
left a comment
There was a problem hiding this comment.
Thanks — the previous ownership, layout-mapping, RMK-action, caching, persistence, and material-copy blockers are addressed. I rechecked the current head against main: 592/592 tests pass, i18n passes, and Clippy completes without new warnings. Two behavior gaps remain before merge.
-
Symbol training advertises both time and fixed-count sessions, but
typing_trainer_focus_statusreturns the character counter unconditionally whenever symbol material is selected. In Time mode the selected 15/30/60/120-second duration is therefore no longer visible; the UI shows a per-chunk0..word_countcounter which resets as the timed run advances. Please keep the remaining-time display forTypingTrainerMode::Timeand use character progress only for the fixed-count mode, with regression coverage for both combinations. -
The material is described as every printable character across all layers, but the shared output decoder still drops printable keys already represented in Entropy's keycode catalog. At minimum,
KC_NONUS_HASH/KC_NONUS_BSLASHand numpad arithmetic keys such as/,*,-, and+never enter the pool. Please extend the owning output decoder with the correct input-layout semantics and add coverage, or narrow the user-facing promise if those keys are intentionally unsupported.
The screenshot in the PR is also from the earlier UI (it omits the language/input-layout selector and uses the old symbols label), so please refresh it with the final state.
The current GitHub Build workflow is still action_required; after these fixes, please run the full matrix before merge.
The trainer built its pool from a private US/QWERTY table and only looked at Vial bindings: with a Russian layout KC_A demanded 'a' instead of 'ф', and native Universal Symbols never reached the exercise at all. The key-to-typed-characters mapping now lives with the legend owner (keycode.rs) and follows the selected input mapping; binding decoding sits next to label decoding and covers RMK actions. The pool is rebuilt when the keymap or the language changes instead of every frame, and statistics are no longer cloned on every keystroke. The language stays available in symbol training because it selects the input mapping, and the material is named after layer keys, stating that letters and digits are included.
The argument of then_some is always evaluated, so the tap dance index was computed for keycodes below the range as well — the subtraction overflowed and brought down debug builds on the very first keypress.
Mod-tap and layer-tap keys type their tap keycode, yet the pool dropped them entirely: a letter living only on a home-row mod disappeared from the exercise. Native Universal Symbols inside tap and tap-hold actions were lost for the same reason. The firmware types the Universal Symbols Russian letters only while the Russian layout is active, so they do not belong in the English pool.
The controls container width was computed without the material toggle, so it was clipped away and symbol training could not be enabled at all. Switching material moves the count to the nearest preset of the new set; otherwise the dropdown showed one value while the run used another. Layers arrive from the device in the background and every new layer rebuilt the pool, wiping the typed text and the timer: a running exercise is no longer restarted. Statistics were persisted only for a finished run, so an abandoned session lost the weights it had gathered.
The name english was left over from the first, English-only table and read wrong in the Russian arms.
d04c1cd to
0609309
Compare
`printable_symbols_from_layout` draws `№` into the symbol pool for the Russian layout (KC_3 with Shift), but `typing_trainer_accepts_char` refused it: U+2116 is neither alphanumeric nor ASCII punctuation, so `type_char` returned at its guard and the caret never advanced. The exercise looked frozen and could only be moved on by typing a wrong character. Accept the character instead of dropping it from the pool — the layout really does type it. The new pool-wide test walks every keycode and Universal Symbol in both input layouts, so a future addition that the trainer cannot accept fails the suite instead of stalling a session.
The input handler recorded an attempt before calling `type_char`, which bails on a finished run, on a character the trainer does not accept and on an empty pool. In Time mode the timer expires inside the draw pass, so every character a fast typist commits on the following frames was counted against the same un-advanced expected character. That symbol's weight saturated at the 200 cap in `weighted_symbol_text`, it was then over-sampled in every later session, and the skew was persisted to `typing_trainer_symbol_stats.json`. Move the recording into `type_char`, past the guards and under the bounds check that appends the character, so statistics can only count keystrokes the run really consumed.
The fallback that keeps a generated text from being one repeated character popped the last character and pushed `symbols[1]`. When the repeated character already was `symbols[1]` — half the runs of a two-symbol pool — the text stayed uniform and the guard did nothing. At `count == 1` the text is trivially uniform, so the fixup also overrode the weighted draw with a fixed pool entry. Pick a symbol that differs from the repeated one and skip the fixup for single-character texts.
`(0x0032, _) => typed('#', '~')` claimed the ISO key types `#`/`~` in
the Russian layout too, where neither character exists. The neighbouring
ISO key at 0x0064 is already layout-aware, so this was an oversight: on
an ISO board `#` and `~` entered the Russian symbol pool but could never
be produced, every occurrence was a guaranteed error, and the adaptive
weighting then sampled them about twice as often as anything else.
Give the key the Russian output of its ANSI sibling KC_BSLS.
Problem
The typing trainer only uses language word lists, which makes it hard to practise the symbols placed across custom keyboard layers.
Solution
symbolsmaterial toggle with time or fixed-count sessions (25, 50, 100);~/.config/entropy/typing_trainer_symbol_stats.json;Screenshot
Verification
cargo test --all-targetspython3 scripts/check_i18n.pyrustfmt --checkfor changed Rust files