Qol improvements

[JoshGatto]

What does this PR do?

• Adds Hypershift functionality where you can set any button as the hypershift activating button which will either work while held down or will toggle on and off it the button is clicked. When Hypershift is active, it allows you to map any of the buttons to a different mapped button
• Updates the design to avoid any card sizing inconsistencies, adds padding to items to give more spacing
• Adds "Learn Hardware Trigger" functionality for any unsupported razer mouses, to help potentially allow users with other mouses to click the triggers of the keys to which they would like to map each "Button" in the Naga Controller app (make sure the keys to the buttons on your Razer mouse are first set to simple keys like the default 1-12, including the DPI buttons otherwise there may be issues with learning hardware trigger.
• Adds various key shortcut options so users can select from a dropdown menu instead of having to press the keys
• Allows for modifier keys to be set as the buttons mappings alone
• Fixes hold button functionality

Screenshots

[DParent10]

Hey @JoshGatto — wow, this is a seriously impressive contribution! The Hypershift implementation is really well thought out, the media key support fills a big gap, and the UI redesign looks clean. A lot of care went into this and it shows.

I went through the changes carefully and have some feedback I'd love to work through together before we merge. Nothing here is a dealbreaker — I think we can clean this up collaboratively and get it into great shape.

Things to fix before merging

1. .DS_Store in the branch — I just removed it from main, so could you do the same on your branch? Just run git rm --cached .DS_Store, commit, and push.

2. Compiled binary committed dist/NagaController.app/Contents/MacOS/NagaController) — a built binary shouldn't live in source control. Can you add dist/ to .gitignore and remove it from the branch?

3. finishLearning() cooldown is a no-op — isLearningHardware is set to false on line 788 before the 0.3s asyncAfter block fires on line 803, so that block just re-sets it to false again. Trailing events from the same physical press aren't actually blocked during that window. A separate isCoolingDown flag that gets checked in capture(event:) would fix this.

4. Nil binding saved on timeout — If the user doesn't press anything during the 1-second learn window, finishLearning() still constructs and saves a HardwareBinding with all-nil fields. Worth adding a guard before saving:

   guard pendingUsage != nil || pendingKeyCode != nil else { return }

5. missionControl, appExpose, and appsGrid shell commands — open -a "Mission Control" is hit-or-miss on modern macOS, open -a "Mission Control" --args 2 won't trigger App Expose, and /System/Applications/Apps.app doesn't exist on a standard macOS install. Happy to help figure out the right approach for these — what was the intended behavior for each?

6. Hypershift "stuck on" state — If a user taps to toggle hypershift on and then holds the button, the long-hold release does nothing — there's currently no way to turn it off except tapping again. The code even has a comment acknowledging this // But for now, let's keep it simple.). Worth making this explicit or adding a fix before merging since it could really confuse users.

7. setHypershiftAction doesn't call saveUserProfiles() — Hypershift mapping changes won't persist if the app quits unexpectedly, unlike hardware bindings which save immediately via setHardwareBinding. The user can work around it by clicking "Save" manually, but it'd be more consistent to auto-save here like the other setters do.

Nice-to-haves (we can tackle together in follow-up commits)

• isModifier(_:) in ButtonMapper (line 268) is defined but never called — only modifierFlag(for:) is used directly. Can be removed.
• The hardcoded Phase 1 defaults (Copy/Paste) in ButtonMapper.mapping (lines 9-12) can be changed to [:] since real mappings come from ConfigManager now.
• vendor == 0x2442 added to isNagaDevice (line 279) with no comment — worth noting which device this covers.
• getHardwareBinding iterates a Dictionary with no guaranteed order — if two bindings share the same usage+page, the result is undefined.
• ProcessInfo.activeProcessorCount > 0 in sendMediaKey (lines 205, 216) always evaluates true, making the else branch dead code.
• Several verbose NSLog statements could be toned down for release builds — in particular the "NUCLEAR EVENT" logging on line 111 of HIDListener, which fires on every non-movement HID event from every device. That's clearly a debug artifact that should be removed or gated behind a flag before release.

What's great

• The hardware binding learn flow is a really solid UX concept.
• The dual-source HID + EventTap capture approach is clever.
• Profile switching actually works now — it was a stub before, great catch!
• The serialization/deserialization for hypershift and mediaKey types is correctly implemented.
  Really nice work overall. Once you've addressed these, leave a comment and I'll do a final pass before we merge. When you're happy with it, feel free to mark it Ready for review and I'll do a final pass.

[JoshGatto]

Hey @DParent10 glad you like the updates! Sorry have been a bit busy but have pushed through some fixes for that feedback you gave, awesome work on the project happy to be helping out!

[JoshGatto]

@DParent10 Good for review?

[DParent10]

Hey @JoshGatto — went through all of this carefully and you got every single point from my review. Really nice work.

Quick rundown on the must-fixes: .DS_Store and the dist/ binary are gone and gitignored, the nil-binding guard is in, and Hypershift persists now. The finishLearning() cooldown is the one I was most worried about and you nailed it — the dedicated isCoolingDown flag checked at the top of capture() is exactly right. Also glad you moved missionControl/showDesktop to CGEvent and dropped appExpose/appsGrid rather than trying to force an API that doesn't exist. The atomic setBothActions was a smart addition I wasn't even expecting.

Nice-to-haves are all in there too — dead code removed, defaults cleaned up, getHardwareBinding rewrite, and the NUCLEAR log gated behind #if DEBUG.

One thing I want to dig into before merging — the 0x2442 entry in isNagaDevice (the one labeled Cooler Master). Two parts:

1. What device is that actually? Cooler Master's vendor ID is usually 0x2516, so I'm not sure 0x2442 is them — could be an OEM/rebrand reporting something odd. Your NUCLEAR EVENT log prints prod= for every device, so you should have the real product string from your testing — mind grabbing that so we can label it correctly?

2. More of a design question. Since the manager matches everything (matches = [[:]]), events from every device already reach the callback — isNagaDevice is really just the software gate at guard isNaga || isLearning that decides what we act on. The asymmetry is: learning mode opens that gate for any device, so anyone can learn a trigger — but at runtime the gate needs isNaga, so a learned binding only fires if the mouse's VID is on the whitelist. Off-list mice learn fine and then silently do nothing, which is what pushed 0x2442 onto the list in the first place.

   What if we made the gate binding-aware instead? Rough shape:

   let hasBinding = ConfigManager.shared.getHardwareBinding(
       forUsage: usage, usagePage: usagePage, cookie: UInt32(cookie), value: value) != nil
   guard isNaga || isLearning || hasBinding else { return }

   It's actually a tighter filter (only the usages people explicitly mapped, not all of a device's traffic), and it drops the need to hardcode VIDs. To be clear about scope, it wouldn't make every mouse work — it makes Learn Hardware Trigger deliver on any mouse whose buttons emit a distinct, learnable HID signal, instead of only whitelisted ones. Buttons that just send keystrokes (the keyboard 0x07 page) are the fiddly case to watch so a binding can't shadow real typing, and plain/firmware-only buttons still won't surface anything to map. Curious what you think.

The Hypershift toggle escape looks right in the code too, I just want to feel it out on hardware before we ship.

I've been away from home without my Mac, so I can't do the final swift build and hardware pass until I'm back — once I am, I'll run it and merge. Shouldn't be long. Thanks for seeing this through, and for your patience on the review — it's a big step up for the app.

[JoshGatto]

@DParent10 Thanks, I think 0x2442 might have been my old Naga Pro with the dongle, however I can't test anymore so i've removed it from the code, the cooler master comment would have been added in from AI without me realising my bad.

I've attempted refactoring the code to better support the hardware learning like you suggested so its not razer specific, althought have kept the whitelisted id's. Let me know your thoughts

[JoshGatto]

Hey @DParent10 haven't heard from you, what are your thoughts on this?

[DParent10]

Hey @JoshGatto — sorry for the slow reply, and thanks for sticking with this. Went through the whole diff. The cleanup is much appreciated (dropping the .DS_Store and the checked-in .dmg, the atomic setBothActions, and the missing saveUserProfiles() in setAction were all real fixes). Hypershift and the media keys are a great addition.

I found two bugs and pushed fixes to your branch. Fair warning that I'm working off my Windows machine at the moment, so none of this is built or tested — it's all from reading the diff.

1. return inside queue.sync doesn't return from the enclosing function

In handle(value:) and again in triggerVirtualButton:

queue.sync {
    if let callback = learningCallback {
        callback(usagePage, usage, cookie, activeVal, vendor, productID)
        return   // exits the closure only
    }
}

Execution fell through to the buttonIndex(forUsage:) lookup below, so learning a non-keyboard button both trained the binding and fired whatever action was currently bound to it. The keyboard path already handled this correctly with a separate if isLearning { return } after the sync block — I've applied the same shape to both other sites.

One judgment call I'd like your read on: I put the guard outside the if pressed block, so releases during learning also return early. The alternative keeps synthetic releases firing, which better protects against a stuck synthetic key if a press lands just before learning starts and its release lands during. Narrow window either way and syntheticStates should clear on the next real release — but you know this code better, so move it if you'd rather.

2. Cancelling learn mode could leave the keyboard blocked

EventTapManager.eventCallback returns nil for every keyDown and flagsChanged while learningCallback is set, and ActionEditorViewController.viewWillDisappear restarted the tap without clearing it. Since the callback lives on a singleton, it survived the view controller.

Repro: click "Learn Hardware Trigger…", then Cancel or Esc without pressing a mouse button. With remapping on, the callback stays armed and the next keystroke is swallowed. If the VC has been deallocated, [weak self] makes the closure a no-op, finishLearning() never runs, and the keyboard stays dead until the app restarts. Fixed by tearing down both callbacks in viewWillDisappear when dismissed mid-learn.

A question rather than a change:

// TEMP: Match EVERYTHING to find the missing Naga interfaces
let matches: [[String: Any]] = [[:]]

I was going to ask you to restore the vendor matching, then realised the isLearning and hasBinding branches of your whitelist guard are only reachable if non-Razer devices actually deliver events — so the broad match looks load-bearing for learning arbitrary hardware, not leftover debugging. Is that the intent? If so the TEMP comment above it is misleading and worth rewording to explain why it's deliberate. If you'd rather scope it down, matching on usage page for mice/keyboards instead of [[:]] might be a middle ground.

Minor, non-blocking:

• Some diagnostic logging is still unconditional — DIAGNOSTIC_VERSION, the DISCOVERY enumeration, the Mapped press / No lookup lines. The NUCLEAR EVENT one is nicely #if DEBUG-gated; worth giving the rest the same treatment.
• sendMediaKey uses virtualKey: 160 for Mission Control — not a standard virtual keycode. Does it actually work for you? Just want to confirm before it ships.

If I get to my Mac this week I'll test properly and either follow up here or just go ahead and merge. If you get to it before I do, let me know how my two commits behave — and push back on any of the above if I've misread something.