Funkschy / nand-to-browser

A Nand to Tetris Emulator implementation that can run in the browser
MIT License
5 stars 0 forks source link

Possible Bug, Keyboard.readInt() #1

Closed temoCF closed 1 year ago

temoCF commented 1 year ago

Hello, I might have found a bug in your emulator. When using the Keyboard.readInt("any string") function and typing in backspaces, the Emulator shutsdown with an error "String is empty" while the standard virtual machine does not collapse. The Jack OS API says that the user backspaces are handled by the readInt function.

Funkschy commented 1 year ago

Thanks for the report. This one is actually kinda tricky. When you use the official emulator with the official bytecode implementation (from the OS folder in the tools directory) it also prints an ERR18 So according to the official bytecode implementation this should actually throw an error. The book does not mention what should happen in the case of an underflow, so i decided to keep the behavior consistent between the bytecode and the rust implementation. I'm not sure if that's the correct thing to do, but it seemed better than being inconsistent like the official implementation :/ I'm not totally opposed to changing this behavior, since it might cause confusion, but I'll need to sleep on it first :)

temoCF commented 1 year ago

Thanks, for the reply. Interesting when I used the OS from the folder it did create ERR18 in the offical emulator as well. I thought that the emulator uses the same vms(in the OS folder) as built in OS (and just links them together). Why are the vms in the OS folder if the VMEmulator doesn't use them in the first place?

The problem was just that there is no exception handling to deal with the error and you can't really prevent a user from typing in a backspace even though an int was required. The "readInt" locks until a number is read, so you can't add additional checking of the pressed keys. It just felt weird to use a function that could easily create a fatal error. To be fair jack is mostly for educational purpose and in most projects of nand2tetris we required correct input and neglected error handling anyway.

Funkschy commented 1 year ago

The official emulator actually has implementations for all the stdlib in java, it works basically the same way as in this emulator. If a function is used which has no implementation in the loaded program we try and find that function in the stdlib implementation. Unfortunately the java implementation is not perfectly compatible with the official bytecode implementation. I'm not actually sure if the bytecode files from the OS folder are used in any way. The rust emulator can actually use the bytecode implementation if enabled by a flag (exactly for compatibility reasons), but this only works in the desktop build, not the web version. If this was a trivial fix, I'd just do it, but it's actually a lot more involved than one would expect due to the async way in which the standard library had to be implemented. Since this is not actually a bug I'll close this for now. But I'd be perfectly willing to merge a PR that fixes this behavior in a way that does not allow the user to delete the prompt in front of the input (this happened when i just tried to implement this in the obvious way)

Funkschy commented 1 year ago

I might come back to this in the future when i have more free time, but since as you said no one writes any serious software for the hack architecture, I don't think this is urgent :)