mortbopet / Ripes

A graphical processor simulator and assembly editor for the RISC-V ISA
https://ripes.me/
MIT License
2.49k stars 270 forks source link

Fix Issue: WASM, Reading console input does not work #360

Closed federicovilla55 closed 1 week ago

federicovilla55 commented 3 months ago

In the current WebAssembly (WASM) version of Ripes there's an issue preventing input reading from the console. This because pressing Enter does not return control to the processor, as stated in issue #304.

The problems stems from the behaviour of the Console::keyPressEvent(QKeyEvent *e) method, which requires a QKeyEvent parameter, that appears to behave differently in the binary executable compared to the WASM version. In the binary executable, when pressing Enter or Return, the QKeyEvent associated with the keys Qt::Key_Return or Qt::Key_Enter has a non empty text field (e->text()). However, in the WASM version that field is empty and therefore the check if (!e->text().isEmpty()) always returns false when pressing Enter, resulting in the already reported error. The same happens with the Backspace Key.

Proposed fix

I propose altering the handling of these events: rather than mandating a non-empty text field in the QKeyEvent, I suggest to verify only the key associated with each command. Text inspection should only occur for the events where the key is not checked.

As stated in the Qt documentation of the QKeyEvent Class of key():

Returns the code of the key that was pressed or released. See Qt::Key for the list of keyboard codes. These codes are independent of the underlying window system.

The key() of an event is independent from the underlying system while the text() may not.

In the modified code, all types of QKeyEvent are processed using the same switch-case structure, as currently happens for the directional keys. When a key event is triggered, the program checks if the key corresponds to any of the arrow keys, the Enter key, the Return key or the Backspace key. If the key does not match any of these, the event is handled as it used to: first, it checks that the content of the key event is not empty, ensuring that modifier keys such as Shift, Control, or Alt are not considered. Then, it adds the content of the event to the buffer and calls the putData() method.

mortbopet commented 2 months ago

sorry for the delay; been on vacation for the past few weeks.

Thank you for looking into this - awesome that we're going to have a fix for this in WASM! have you tested this by running a local WASM build and (Linux | Windows) build to ensure that it works in both scenarios?

federicovilla55 commented 2 weeks ago

I've tested the PR with both local WASM and Linux/Windows builds, and it works in all cases. Let me know if there are any other scenarious I should check.

mortbopet commented 1 week ago

@federicovilla55 sorry for the delay (feel free to ping me if i'm not replying)... awesome - we'll merge it :)