FNA Code Style ./I* - R*

[3vi1]

Flibitized for #147. All the usual, and removed unnecessary parenthesis from commutative additions.

[flibitijibibo]

Separate lines for all this stuff

[flibitijibibo]

Other than the two style things I pointed out above, seems great!

[3vi1]

Can't believe I missed those get/set's.

I guess I had done so many bool expressions that I got stuck in a groove on that last string concatenation.

Thanks for catching those.

[flibitijibibo]

Note that this formatting issue runs across most of these diffs, not just the one I pointed out.

[flibitijibibo]

Guessing this was borked in the retabbing.

[3vi1]

I had a problem with this file being corrupted (I believe I used sed on it while I had it open in another editor), so I copy/pasted over it from a backup inside of MonoDevelop (I was on my daily commuter van, so couldn't just checkout the old version). I just re-tested now, and sure enough Monodevelop will screw up the licensing tabbing when doing that. Good to know.

[flibitijibibo]

Will look at this one more time in detail tomorrow... you can do another passthrough of the P-R section one more time if you want. The new stuff all looks good (but I haven't checked Matrix yet).

For Matrix, the only thing I'd check right off the bat is the file results. In upstream it complains about very long lines, so if it still does that, see what's needed to fix that...

Hoping to finally merge this tomorrow or Sunday.

[3vi1]

[mono] ~/src/fna-jl/MonoGame.Framework @ file Ma*
MathHelper.cs: ASCII text
Matrix.cs:     ASCII text

:)

Yeah, Matrix was a bear. One line was even 467 characters wide. I can see where the original coder might have found it hard to break up since he nested so many of the operations, but most of that nesting was unnecessary (it was controlling the order of addition of floats, which is totally commutative) so I was able to simplify it and multi-line it all.

I'll go comment on a couple of other items I think you might want to offer feedback on, since I apparently have insomnia this morning. :)

[3vi1]

Hmmmm... looks like I can't do inline comments on Matrix. I guess it's hidden because it's so long.

Anyway, the two items I was thinking of are:

• Overloaded operators. There was redundant code for all the operators, I simplified this to have the operators (ex. operator-+) hand off to the named method (.Add), eliminating the second implementation of the same function.
• Matrix as a parameter. At least one function (FindDeterminants) passes a 4x4 matrices as 12 floats. Instead of putting each float parameter on a separate line of the function header, I put 4 to a line to make it more clear what it was doing.

[flibitijibibo]

Turns out the same can be said of Quaternion regarding obnoxious copypasted code...

This should be considered an additional commit on top of this merge: 24f1eb3

Matrix seemed okay, but I gave it a very lazy glance... the only thing that leaves me unsure about this commit is the possible change of behavior from changing nests/parens in order of operations, as that can be super super hard to detect if something goes wrong. I tested this with Capsized, which uses these bits extensively, and it seemed okay, but if we get a report about weird maths somewhere down the line, we should probably look to these two sets of changes.

[flibitijibibo]

Sure enough:

8c452e3

a671673#diff-5eecd4f30a1b3dd7256f6b1d0695e716L644
a671673#diff-5eecd4f30a1b3dd7256f6b1d0695e716R709

Definitely do a second pass through these files and be sure more of those didn't slip through...

[3vi1]

the only thing that leaves me unsure about this commit is the possible change of behavior from changing nests/parens in order of operations

It should be fine. I don't think I changed any nesting that affects anything but the additions, which should be totally commutative for the vectors and float operations I modified.

If we do get any reports, I'll be happy to go back and investigate/fix anything I @%^!'d up.