Merge Prettified Corax+ Code

Timendus / chip8-test-suite

A collection of ROM images with tests that will aid you in developing your own CHIP-8, SUPER-CHIP or XO-CHIP interpreter (or "emulator")

GNU General Public License v3.0

359 stars 7 forks source link

Merge Prettified Corax+ Code #24

Closed GamingMadster closed 2 months ago

GamingMadster commented 2 months ago

Here's the code cleanup I mentioned doing in #22.

Added spacing for the code so the code was a bit easier to read through
Removed unnecessary comments, added a couple others
Made labels a bit more consistent throughout

I might try and do the same for some of the other source files, just so it's easier for someone looking to contribute to be able to read through the code.

Timendus commented 2 months ago

The Corax+ test is the worst offender of the bunch, I think. Primarily because I've initially tried to keep it just the "stock" Corax89 ROM with a few patches / bug fixes. I've added more and more changes over the years, without ever really taking the time to refactor.

I think your PR is an improvement overall, couple of remarks:

Your version is removing a newline at the end of the file. I'm guessing your editor is configured to remove it. Just like the tabs/spaces discussion, this is a very subjective thing, and I generally just conform to whatever the rest of the project I'm contributing to uses.
I like adding whitespace for readability. I'll go through later to see if I agree with your groupings.
I don't see any changes to labels..?

All in all: congratulations on a much cleaner and much easier to review pull request than the last one! Great to see you learning in real time :smile:

GamingMadster commented 2 months ago

To be honest, combing through the code wasn't all too bad, but some areas were sure a doosie.

In terms of the labels, I think I'm combining the labels and comments for the start of each test in my mind, oops. 😅 I do think I still modified atleast one though?
The whitespace i mainly did for the if statements that set i to image-no, although there are a couple other areas that I did it upon which you may deem unnecessary, unsure.
In terms of the newlines disappearing, I've just been using notepad to get the job done. I don't know why it would remove the newline at the end, since it's a simple text editor and it shouldn't auto format it. I guess I might have accidentally deleted it, oops x2. 😅

Oh, and thanks! Again, I'm glad to be contributing to the test suite!

GamingMadster commented 2 months ago

Alright, I've done a little bit more to cleanup stuff.

Did some small comment changes
Added a few comments of my own
Hopefully re-added the newline? (I just copied the line needed from the file before my commits ¯\_(ツ)_/¯)

Timendus commented 2 months ago

Thanks! I've removed many of the newlines again :P Sorry for that, but I do think it's a lot more readable now. Thanks for your help :)