Viewer importer: key binding import wipes keybinds (WASD etc.) -- overwrite + resetBindings, no merge/validation #232

Closed
opened 2026-10-01 22:33:04 +00:00 by sky_line · 2 comments
Owner

Report

Importing settings from another viewer (Firestorm) wiped the keybinds: WASD movement and similar no longer work.

Likely cause (from code reading; not yet reproduced)

SLFloaterViewerImport::onImport() (key bindings checkbox):

  1. Copies the source viewer's key_bindings.xml over ours (fs::copy_file(..., overwrite_existing)).
  2. Then calls gViewerInput.loadBindingsXML(dst).

LLViewerInput::loadBindingsXML() starts with resetBindings() (clears every binding). It then loads only what parses and validates from the file. So:

  • If the FS file fails keys.validateBlock() (FS-specific attributes / commands / modes), nothing loads and every mode is left empty.
  • If the file lacks a mode, or uses command names we don't have, those bindings are silently dropped.
  • Because the file was already copied over ours, the broken bindings persist across restarts. The .pre-import backup exists but nothing restores it.

Fix

  1. Never overwrite blindly:
    • parse and validate the source file first (in memory);
    • if it fails, report it and leave ours untouched.
  2. Merge, don't replace:
    • start from our current bindings (or defaults for modes the user never customised);
    • overlay only (mode, key, mask, mouse) -> command entries whose command exists in our command set;
    • keep every mode the source file doesn't define.
  3. Verify after load:
    • if a mode ends up empty, or core movement (forward / back / left / right, jump, fly) has no binding, restore the backup automatically and report it.
  4. Report what was imported, skipped (unknown commands) and kept.
  5. Recovery for affected users:
    • a "Restore key bindings" action (the .pre-import backup, or our defaults) in the importer and in Preferences > Controls;
    • consider an automatic check at startup: if movement keys are unbound in every mode, offer to restore.

Repro to confirm

  1. Fresh profile.
  2. Firestorm profile with a customised or stock key_bindings.xml.
  3. Run the importer with the key bindings option ticked.
  4. Check WASD in third person, then restart and check again.
## Report Importing settings from another viewer (Firestorm) **wiped the keybinds**: WASD movement and similar no longer work. ## Likely cause (from code reading; not yet reproduced) `SLFloaterViewerImport::onImport()` (key bindings checkbox): 1. **Copies the source viewer's `key_bindings.xml` over ours** (`fs::copy_file(..., overwrite_existing)`). 2. Then calls `gViewerInput.loadBindingsXML(dst)`. `LLViewerInput::loadBindingsXML()` starts with **`resetBindings()`** (clears every binding). It then loads only what parses and validates from the file. So: - If the FS file fails `keys.validateBlock()` (FS-specific attributes / commands / modes), nothing loads and **every mode is left empty**. - If the file lacks a mode, or uses command names we don't have, those bindings are silently dropped. - Because the file was already copied over ours, the broken bindings **persist across restarts**. The `.pre-import` backup exists but nothing restores it. ## Fix 1. **Never overwrite blindly:** - parse and validate the source file first (in memory); - if it fails, report it and leave ours untouched. 2. **Merge, don't replace:** - start from our current bindings (or defaults for modes the user never customised); - overlay only `(mode, key, mask, mouse) -> command` entries whose command exists in our command set; - keep every mode the source file doesn't define. 3. **Verify after load:** - if a mode ends up empty, or core movement (forward / back / left / right, jump, fly) has no binding, restore the backup automatically and report it. 4. Report what was imported, skipped (unknown commands) and kept. 5. **Recovery for affected users:** - a "Restore key bindings" action (the `.pre-import` backup, or our defaults) in the importer and in Preferences > Controls; - consider an automatic check at startup: if movement keys are unbound in every mode, offer to restore. ## Repro to confirm 1. Fresh profile. 2. Firestorm profile with a customised or stock `key_bindings.xml`. 3. Run the importer with the key bindings option ticked. 4. Check WASD in third person, then restart and check again.
Author
Owner

Fixed on branch vimport (commits dfc44c53ca and bafa583924). Not built yet; syntax-checked only.

Import no longer overwrites; it merges

  • Their file is parsed leniently and in memory: FS / Alchemy / SL key_bindings.xml with LLXMLNode (attributes only, no LLInitParam validation), and Black Dragon's controls.xml. For BD, mode 2 (its build mode) is dropped and toggle_crouch becomes toggle_down. If their file can't be read, ours is not touched.
  • Each (mode, key / mouse, mask) -> command is compared with ours:
    • a command we don't have (LLViewerInput::isKnownCommand) is skipped;
    • an identical binding is "same";
    • a free key is added;
    • a key we bind to another command is a conflict. It is listed but unticked. Ticking it replaces only that one binding, and it is refused if it would leave a core command (movement, jump, fly, chat ...) with no key;
    • a command that already has 3 keys is skipped.
  • Nothing of ours is removed, and modes their file doesn't define are kept.
  • The merged file is written and reloaded, then verified with LLViewerInput::hasBinding. If any core command that had a binding before lost it, the previous file is restored (or removed, if there was none) and reloaded, and the report says "import reverted".
  • Before writing, the old file is backed up in user_settings/import_backups/<date_time>/, and Undo last import restores it.

Recovery for users already affected
LLAppViewer::loadKeyBindings checks the user key_bindings.xml. If it leaves push_forward and push_backward unbound in third person, the file is moved to key_bindings.xml.unusable, the defaults are loaded, and a notice (KeyBindingsRestoredAtStartup) says so.

Acceptance test (the repro from this ticket)

  1. Start from a fresh profile and an FS profile with a stock or customised key_bindings.xml.
  2. Import with Key bindings ticked.
  3. WASD / E / C / F still work in third and first person, before and after a restart.
  4. Import again: every key row reads "same".
  5. Undo restores the previous file.
  6. Negative case: hand-delete the W/S bindings from third_person in key_bindings.xml. The next launch should show the notice and leave key_bindings.xml.unusable behind.
Fixed on branch `vimport` (commits dfc44c53ca and bafa583924). Not built yet; syntax-checked only. **Import no longer overwrites; it merges** - Their file is parsed leniently and in memory: FS / Alchemy / SL `key_bindings.xml` with `LLXMLNode` (attributes only, no `LLInitParam` validation), and Black Dragon's `controls.xml`. For BD, mode 2 (its build mode) is dropped and `toggle_crouch` becomes `toggle_down`. If their file can't be read, ours is not touched. - Each `(mode, key / mouse, mask) -> command` is compared with ours: - a command we don't have (`LLViewerInput::isKnownCommand`) is skipped; - an identical binding is "same"; - a free key is **added**; - a key we bind to another command is a **conflict**. It is listed but unticked. Ticking it replaces only that one binding, and it is refused if it would leave a core command (movement, jump, fly, chat ...) with no key; - a command that already has 3 keys is skipped. - **Nothing of ours is removed**, and modes their file doesn't define are kept. - The merged file is written and reloaded, then verified with `LLViewerInput::hasBinding`. If any core command that had a binding before lost it, the previous file is restored (or removed, if there was none) and reloaded, and the report says "import reverted". - Before writing, the old file is backed up in `user_settings/import_backups/<date_time>/`, and **Undo last import** restores it. **Recovery for users already affected** `LLAppViewer::loadKeyBindings` checks the user `key_bindings.xml`. If it leaves `push_forward` and `push_backward` unbound in third person, the file is moved to `key_bindings.xml.unusable`, the defaults are loaded, and a notice (`KeyBindingsRestoredAtStartup`) says so. **Acceptance test (the repro from this ticket)** 1. Start from a fresh profile and an FS profile with a stock or customised `key_bindings.xml`. 2. Import with Key bindings ticked. 3. WASD / E / C / F still work in third and first person, before and after a restart. 4. Import again: every key row reads "same". 5. Undo restores the previous file. 6. Negative case: hand-delete the W/S bindings from `third_person` in `key_bindings.xml`. The next launch should show the notice and leave `key_bindings.xml.unusable` behind.
Author
Owner

Verified done: key-binding import no longer overwrites and resets; it merges in memory (unknown commands skipped, conflicts listed unticked, core commands never left unbound) and recovers from an unusable key_bindings.xml at startup. Commits: dfc44c53ca, bafa583924. Shipped in v1.0.0-beta.1.

Verified done: key-binding import no longer overwrites and resets; it merges in memory (unknown commands skipped, conflicts listed unticked, core commands never left unbound) and recovers from an unusable key_bindings.xml at startup. Commits: dfc44c53ca, bafa583924. Shipped in v1.0.0-beta.1.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
sky_line/Slipstream#232
No description provided.