-
Notifications
You must be signed in to change notification settings - Fork 7
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
Ideas for further improvement #5
Comments
Things resolved in upcoming version 4:
|
Something else that might be nice to add (now that program memory won't be a problem going forwards with the stand alone binaries) is a sound test. I don't think the test suite will be able to automatically check and report on the implementation in the same way it does with flags, quirks etc, but it could be nice to have a screen on which a message is displayed stating an audio pattern (e.g. something like a 1 second beep following by 1 second of silence then 5 beeps of half a second each separated by half a second of silence) and then the test suite plays the corresponding audio. The user can listen and easily check that the audio they hear being played by the interpreter matches the stated pattern in the test suite. I would certainly have found this useful recently for verifying my own interpreter audio playback. Instead I had to randomly pick existing ROMs then run them side-by-side in my emulator and other established emulators to ensure the audio matched; having a specific test would make this easier. |
For the subtraction opcodes, the test currently doesn't check how the interpreter sets the flag if the subtracted values are equal, which is apparently an annoying edge case for some people. Could be nice to add a test for that. |
Just a small note: This edge case makes the quirks test report "LOW" for the display wait quirk for an emulator that correctly implements the quirk, making the devs hunt in the wrong direction, so if other tests depend on the edge case it should be tested in the flags test, I think. |
Things resolved in upcoming version 4.1:
|
This was resolved by #22 (but not yet released)
|
Things resolved in version 4.2:
|
CHIP-8 beginner here! First of all thank you for your test suite, it has been an invaluable source so far in my journey! Now to the suggestion: Not sure what kind of problems you might encounter if you forget the mask, but would be nice to check for it, no? |
Well, you have to use the (P)RNG part to test for the mask, so the test needs to loop to make sure that enough bits come together to check for the effect of the mask. I do this in my unit tests, where I execute Cxnn with fixed masks a bunch of times and AND/OR together the returns and check that after enough iterations the AND result is equal zero (to check there are no stuck bits in the RNG) and the OR result is equal to the mask, showing that each mask bit was set at least in one result. Without a "working" RNG I see no way to validate the working of the mask from the perspective of the CHIP-8 program. |
Btw. even the not really good random generator in the original COSMAC VIP manages to succeed in that test after 16 iterations, so it should not need too many of them, so a test could run in a loop and exit if the AND result (starting at 0xFF) and the OR result (starting as 0x00) have reached 0/mask or a max tries counter ran down (255 should be easily enough and would not run that long), making the reach of the end of the counting a fail for Cxnn and the early due to both values being correct a success. Haven't done this as a CHIP-8 program yet, to validate the quality of the HP-48SX implementations yet though. |
Thank you! I have improved my unit test with your suggestion. Still would be fun to have this kind of test as part of the suite. I might try making this after I'm done with everything else. |
That does sound like it could be a valuable addition to the test suite. And Gulrak's solution is also how I would approach this problem. I would like to add that it would need to run this test several times, cycling through different masks, to make sure the mask actually works like a mask. We also have to be very explicit in the manual that this test can in some cases produce false negative results. |
The text was updated successfully, but these errors were encountered: