namcos21: Some cleanup & modernization #12566
Conversation
FlykeSpice
commented
Jul 11, 2024
•
FlykeSpice
commented
- namcos21_3d.cpp: Removed some duplicated code, modernized it a bit to current MAME standards
- namco21*.cpp: Tidied them up a bit, removed some unnecessary identations and added better formatting.
|
You know what? Why not touch all the namcos21_* in a single PR, should get things moving more swiftly |
FlykeSpice
changed the title
|
What do you mean? |
Eventually I would need to cleanup the others namcos21*.cpp source files aswell. So I thought why not do it in a single PR? |
There was a problem hiding this comment.
There are a lot of changes here, but I’m not sure on the quality of it.
On just a quick read of the code, I noticed you’d broken a fairly simple bubble sort of three items. There may be other stuff that’s broken that I haven’t noticed.
You haven’t been particularly consistent with reformatting. There are some if statements that you’ve reformatted to if (condition) style, while others where you’ve changed the line still use if( condition ) or other styles. Similarly, you’ve added spaces around binary operators in some places but not others.
A lot of the places you’ve added blank lines aren’t separating logical blocks of code, and hence don’t aid readability.
| for (;;) | ||
| { | ||
| if (v0->y > v1->y) | ||
| { | ||
| SWAP(n21_vertex, v0, v1); | ||
| } | ||
| else if (v1->y > v2->y) | ||
| { | ||
| SWAP(n21_vertex, v1, v2); | ||
| } | ||
| else | ||
| { | ||
| break; | ||
| } | ||
| std::swap(v0, v1); | ||
|
|
||
| if (v1->y > v2->y) | ||
| std::swap(v1, v2); | ||
|
|
||
| if (v0->y > v1->y) | ||
| std::swap(v0, v1); | ||
| } |
There was a problem hiding this comment.
You’ve broken this – consider what happens if you get (3 2 1). The first swap makes it (3 1 2), and the second swap makes it (1 3 2). You need three comparisons to bubble sort three items.
There was a problem hiding this comment.
Did you really proof read it? There are three comparisons.
Removed duplicate code and modernized it to current MAME standards
FlykeSpice
force-pushed
the
namcos21_3d
branch
from
ae34fa1
to
ec7b9fd
Compare
|
....hello? |
