Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error with Commodore Key. #629

Closed
rauno-palosaari opened this issue Jan 25, 2021 · 7 comments
Closed

Error with Commodore Key. #629

rauno-palosaari opened this issue Jan 25, 2021 · 7 comments

Comments

@rauno-palosaari
Copy link

Virtual C64
4.0b3

Error with Commodore Key. Release without press

Commodore + key1 -> Nothing

_pressRowCol(7,5)
releaseRowCol(7,0)
releaseRowCol(7,5)

Commodore + key2 -> _

_pressRowCol(7,5)
_pressRowCol(5,6)
releaseRowCol(5,6)
releaseRowCol(7,5)

Commondore key have other strage keys -> small cap, 4

_pressRowCol(7,5)
_pressRowCol(1,3)
_pressRowCol(1,7)
releaseRowCol(1,3)
releaseRowCol(1,7)
releaseRowCol(7,5)

Control ok

Control + key1 -> Black

_pressRowCol(7,2)
_pressRowCol(7,0)
releaseRowCol(7,0)
releaseRowCol(7,2)

Regard,

Rauno Palosaari

@rauno-palosaari
Copy link
Author

Commondore ok with 3.4

@dirkwhoffmann
Copy link
Owner

Hi Rauno. You pressed RunStop on the MacBook Pro Touch Bar, right?

@rauno-palosaari
Copy link
Author

rauno-palosaari commented Jan 26, 2021 via email

@rauno-palosaari
Copy link
Author

Strange.
Option + 1 works with the Magic Keyboard with Numeric Keypad.

Option + Numeric Keypad 4

_pressRowCol(7,5)
_pressRowCol(7,0)
releaseRowCol(7,0)
releaseRowCol(7,5)

Option + 4

_pressRowCol(7,5)
releaseRowCol(7,0)
releaseRowCol(7,5)

@dirkwhoffmann
Copy link
Owner

I can confirm that there is a bug. After adding some more debug messages, I can see the following after pressing OPT + 4 + 4 in positional key mapping mode:

_press(49)
_pressRowCol(7,5)
KeyboardController.82::keyDown(with:): keyDown: Optional("¢") 21
KeyboardController.88::keyUp(with:): keyUp: Optional("¢") 21
KeyboardController.82::keyDown(with:): keyDown: Optional("¢") 21
KeyboardController.88::keyUp(with:): keyUp: Optional("¢") 21
_release(49)
_releaseRowCol(7,5)

The Commodore key (7,5) is pressed and released on the C64 side which is good. But the '4' gets lost (it is recognized on the Swift side, but there are no corresponding key events on the C64 side). I think it's because I use the MacKey class as the lookup key for the positional key map in Swift. The modifier flags (which are also stored in the MacKey class) are likely to interfere:

 let joyKeyMap1: [MacKey: Int]

If I remember correctly, I had a similar issue in vAmiga. I'll look into it in more detail tomorrow...

@dirkwhoffmann
Copy link
Owner

I’ve identified two (maybe three) issues:

  1. As expected, the emulator failed to find the mapped C64 key in the positional key map if a modifier flag was present.

    I’ve fixed that by providing a custom hash function:

    func hash(into hasher: inout Hasher) {
        
        hasher.combine(keyCode)
    }

    An even easier method would be to use the key code as map index and not the object itself.

  2. The standard positional key map had duplicates (two distinct Mac keys assigned to the same C64 key)

    MacKey.curRight: C64Key.curLeftRight,
    MacKey.curLeft: C64Key.curLeftRight,
    MacKey.curDown: C64Key.curUpDown,
    MacKey.curUp: C64Key.curUpDown,

    As a result, pressing „cursor left“ on the Mac pressed the horizontal cursor key on the C64 keyboard. This moved the cursor into the wrong direction if the shift key was not pressed at the same time. I agree that this is totally confusing to the user. To cope with this issue, I’ve removed the double mappings from the default key map. Furthermore, when the key map is being loaded from the user defaults, the emulator checks for double mappings and filters them out. This has two implications though:

    • Pressing "cursor left" or "cursor up" on the Mac keyboard has no effect in positional key mapping mode any more. To move the cursor up, you need to press Shift+Cursor down, just like on the original C64 keyboard.
    • The key map in the user defaults storage might not work as expected any more (because all duplicates are filtered out). This can be solved by restoring the standard settings in the Keyboard preferences dialog (it’ll save the new standard positional key map to the user defaults storage which has no duplicates any more).
  3. One of the logs above shows a key release event, but no key press event for the option key. I was not able to reproduce this on my machine, but I remember that I had similar problems with older macOS releases (many years ago actually). To cope with this, I used the following code at that time:

   /* Checks if the internal values are consistent with the provides flags.
     * There should never be an insonsistency. But if there is, we release the
     * suspicous key. Otherwise, we risk to permanently block the C64's
     * keyboard matrix
     */
    func checkConsistency(withEvent event: NSEvent) {
        
        let flags = event.modifierFlags
        
        if (leftShift || rightShift) != flags.contains(NSEvent.ModifierFlags.shift) {
            keyUp(with: MacKey.shift)
            keyUp(with: MacKey.rightShift)
            Swift.print("*** SHIFT inconsistency detected *** \(leftShift) \(rightShift)")
        }
        if control != flags.contains(NSEvent.ModifierFlags.control) {
            keyUp(with: MacKey.control)
            Swift.print("*** CONTROL inconsistency *** \(control)")
        }
        if option != flags.contains(NSEvent.ModifierFlags.option) {
            keyUp(with: MacKey.option)
            Swift.print("*** OPTION inconsistency *** \(option)")
        }
    }

At some point, I stopped calling this function, because the issue went away from some OS upgrade to another (on my machine). If the problem still shows up in the next release, I might have to resurrect this function.

@dirkwhoffmann
Copy link
Owner

Hopefully fixed in v4.0b5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants