feature: All-in-one PS/2 micro-controller and keyboard emulation once and for all

[maximilien-noal]

Fixes the most glaring issues in the current keyboard implementation.

The keyboard did not have an internal buffer.
It did not have a delay to ignore key

It did not support being disabled/renabled (which is a keyboard command)

The IRQ1 handler didn't call the keyboard intercept from INT15H first before processing.

The corresponding INT15H interrupt was not implemented.

INT9H did not read the keyboard through the Data port.

We could not read the keyboard through the Data port while an emulated program read it at the same time.

I still have a lot to implement (a proper Keyboard Controller, which also links to the mouse and aux port, typematic stuff, keyboard ScanCode sets, support all keyboard commands, move the scanCode translation elsewhere, support DOS keyboard layouts on top...)

But at least the DOS Console stuff (like in text menus, for example the one in Oh No! More Lemmings, or the DUNE 2 SETUP program), are useable now.

Monkey Island 1 can also properly be paused with the space bar thanks to the delay.

I tested a lot of games without issues (Krondor, Dune, Dune II, Wolf3D, Shangai II, ...)

[kevinferrare]

Shouldnt this logic to disable keyboard, read data and enable again be in keyboard class? Overrides might need it.
I wonder why doing so though, to ensure no other key gets added to the buffer? Wouldn't that solve the problem partially considering between time keyboard request irq1 and time this actually gets called, some time could already have passed? (Think about masked irq or other irq being processed)

[kevinferrare]

Ushort savedAX = State.AX since you use both al and ah?

[kevinferrare]

Why cant this be done on ui thread that would put events in a queue in each device? Wouldn't this be less impactful than checking that every cycle?

[kevinferrare]

Can't this be checked in one common class that would be used to enquête events for the devices? UI would push events thrre and it would ignore them if paused

The issue occurs in src/Spice86.Core/Emulator/Devices/Input/Keyboard/Intel8042Controller.cs at lines 630–633. The nested if statements should be merged into a single if statement using the && operator, so that both guards are combined and the block is only executed if both conditions are true. No change to the functionality will result, but the code will be easier to read.

No imports or method definitions are required for this change. Only the lines containing the nested if statements and the logging statement should be changed.

---

The fix is to collapse the nested if statements at lines 667–669 into a single conditional using the logical && operator, following C# operator precedence (which is well-defined for &&). The new line will join the inner and outer conditions:
if (code == 0xf0 && (lines & 0b0001) == 0 && _loggerService.IsEnabled(LogEventLevel.Warning)) { ... }
Only lines 667–670 are affected. No new imports or definitions are required.

---

The best solution is to encapsulate the four byte values (flags1, flags2, flags3, and leds) into a new dedicated struct or class, for example, KeyboardState. The ProcessScancode method signature should then take a single KeyboardState object (ideally by reference if mutation is needed for performance, or as a class reference type if acceptable). All internal code that previously referenced the ref bytes should now access the properties of KeyboardState. This change improves code readability, maintainability, and reduces the method’s complexity. Only the region encompassing the relevant method declaration and its usages within this file need updating; any usages outside this file must be disregarded as per the instructions.

---

To fix the problem, we should replace the if-else assignment to asciiscan on line 454–455 with a single ternary assignment, reducing code duplication and improving clarity. The correct change is to replace:

if ((flags1 & 0x08) != 0) asciiscan = 0xa600;
else asciiscan = (ushort)((asciiscan & 0xff) | 0xe000);

with:

asciiscan = ((flags1 & 0x08) != 0) ? (ushort)0xa600 : (ushort)((asciiscan & 0xff) | 0xe000);

This preserves the original assignment logic without altering any other part of the function. All changes should be within src/Spice86.Core/Emulator/InterruptHandlers/Input/Keyboard/BiosKeyboardInt9Handler.cs in the given lines.

---

To fix this problem, replace the conditional assignment to _status.IsDataFromAux with a single-line ternary assignment.

• Original region: lines 312-315.
• Change: Replace the if ... else ... block assigning to _status.IsDataFromAux with a single line: _status.IsDataFromAux = _buffer[idx].IsFromAux ? true : false;
• The ternary is strictly correct (though just assigning _buffer[idx].IsFromAux would suffice since it is already a boolean), as per the recommendation ("use ternary operator"), but for maximum clarity and minimal change you can use the ternary as suggested.

No imports or other definitions are needed; only edit the assignment block.

To fix the issue, replace the if-else block currently used to assign a value to _buffer[idx].Data with a single line that uses the ternary operator to assign either GetTranslated(value) or value based on the condition isFromKbd && _config.TranslationEnabled. The change should be made within the method BufferAdd on lines 337–341 in Intel8042Controller.cs. No new imports, methods, or definitions are necessary; simply replace the block with the ternary assignment.