Skip to content

Address PR review feedback: extract duplicated code, fix Vue reactivity violation, prioritize user config#6

Merged
w1010tdev merged 6 commits into
copilot/unify-axis-mapping-frontendfrom
copilot/sub-pr-5
Jan 24, 2026
Merged

Address PR review feedback: extract duplicated code, fix Vue reactivity violation, prioritize user config#6
w1010tdev merged 6 commits into
copilot/unify-axis-mapping-frontendfrom
copilot/sub-pr-5

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 18, 2026

Addresses unresolved review comments from PR #5 regarding code duplication, Vue best practices violations, and configuration priority.

Changes

  • Extracted duplicated axis_codes list (server/joystick_manager.py)

    • Created UINPUT_AXIS_CODES class constant with thread-safe lazy initialization
    • Eliminated duplication between _init_gamepad() and set_axis() methods
  • Fixed Vue computed property mutation (static/js/main.js)

    • Moved axis mapping initialization to dedicated initializeCustomAxisMapping() function
    • Added watch() on customJoystickAxisCount to handle dynamic axis count changes
    • Computed property now returns new objects via spread operator instead of mutating reactive state
  • Prioritized buttons.json over config.py (server/joystick_manager.py, server/app.py)

    • VirtualJoystick.__init__() now accepts optional joystick_config parameter from user settings
    • Falls back to config.py defaults only when user config unavailable
    • Updated initialization flow to pass user config from buttons.json
  • Documented configuration priority (README.md)

    • Added section clarifying buttons.json (Web UI) takes precedence over config.py defaults
    • Noted server restart requirement for joystick type/axis count changes

Example

Before (computed property mutating reactive state):

const customAxisConfigList = computed(() => {
    for (let i = 0; i < customJoystickAxisCount.value; i++) {
        if (!customAxisMapping[i]) {
            customAxisMapping[i] = { /* defaults */ };  // ❌ Mutation
        }
        customAxisMapping[i].axisIndex = i;  // ❌ Mutation
    }
});

After (pure computed with separate initialization):

const initializeCustomAxisMapping = () => {
    for (let i = 0; i < customJoystickAxisCount.value; i++) {
        if (!customAxisMapping[i]) {
            customAxisMapping[i] = { /* defaults */ };
        }
    }
};

const customAxisConfigList = computed(() => {
    return list.map(cfg => ({ ...cfg, axisIndex: i }));  // ✓ No mutation
});

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits January 18, 2026 07:08
…d mutation, use buttons.json config priority

Co-authored-by: w1010tdev <246258262+w1010tdev@users.noreply.github.com>
Co-authored-by: w1010tdev <246258262+w1010tdev@users.noreply.github.com>
Co-authored-by: w1010tdev <246258262+w1010tdev@users.noreply.github.com>
Copilot AI changed the title [WIP] Add custom virtual joystick functionality for flight simulation Address PR review feedback: extract duplicated code, fix Vue reactivity violation, prioritize user config Jan 18, 2026
Copilot AI requested a review from w1010tdev January 18, 2026 07:12
@w1010tdev w1010tdev marked this pull request as ready for review January 24, 2026 01:18
@w1010tdev w1010tdev requested a review from Copilot January 24, 2026 01:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses prior review feedback by removing duplicated Linux uinput axis-code lists, fixing a Vue reactivity anti-pattern around computed mutation, and ensuring user-saved joystick configuration in buttons.json takes precedence over config.py.

Changes:

  • Extracted and lazily initialized a shared UINPUT_AXIS_CODES mapping for Linux uinput in CustomVirtualJoystick.
  • Refactored custom-axis mapping initialization in the Vue UI to avoid mutating reactive state inside a computed property.
  • Updated virtual joystick initialization to pass user joystick config from buttons.json, falling back to config.py defaults only if absent.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
static/js/main.js Refactors custom axis mapping initialization and computed output to avoid computed mutations.
server/joystick_manager.py Deduplicates Linux uinput axis code list; adds user-config override support to VirtualJoystick.
server/app.py Builds/passes joystick config from buttons.json into VirtualJoystick initialization.
README.md Documents configuration precedence and restart requirements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread static/js/main.js Outdated
Comment thread static/js/main.js Outdated
w1010tdev and others added 2 commits January 24, 2026 09:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@w1010tdev w1010tdev merged commit 0881da0 into copilot/unify-axis-mapping-frontend Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants