[FNA] [HARD] FNA Code Style: Tabbing, etc.

[flibitijibibo]

Difficulty: This task is considered a HARD task. If MEDIUM tasks separate the men from the boys, HARD tasks separate Dark Souls fans from actual serial killers that wear proper armor for reasons yet to be explained.

Ordering: This task should occur AFTER removing unused defs! See #145 before taking a folder here.

One of my favorite features of the MonoGame source is that the tabbing/formatting is fucked. I hesitate to blame any specific dev for this, not because of a mass of incompetent MonoGame devs, but because of a mass of incompetent IDE developers. I swear, I have yet to understand the black magic that decides how MonoDevelop tabs your files.

So while we're fixing this problem, we're going to establish a strict text format that should help make the source significantly more readable.

Here are the rules:

• Tabs: Actual '\t' tabs, NOT SPACES!
• Blank Lines: NO TRAILING SPACE! Blank lines should be, you know, blank.
• Characters per line: ~100. When a line gets too long, start splitting it up into multiple lines. The number should only be considered a maximum value; if you find that you're doing a lot of extensive left-to-right reading rather than top-to-bottom, start splitting the lines up.
• i++/i--: Do i += 1 and i -= 1 instead unless the increment is genuinely being used to its advantage. If it's that painful to do this, consider foreach instead.
• Do not use var! Use the actual type name!
• someMethod() rather than someMethod (), someArray[x] rather than someArray [x], etc.
• (Type) cast rather than (Type)cast.
• Use braces everywhere! I don't care if the if block is one line, braces! Use them!
• Single line comments: // This code is derp rather than //this code is derp.
• Multi-line comments: Use /* */ blocks, rather than multiple lines of //.

How I split up lines:

public static void StupidMethod(
    int with,
    int lots,
    int of,
    string stupid
    IntPtr parameters
) {
    if (    stupidCheck.HasLotsOfBools() &&
        (   theresNothingWe.CanDo() ||
            xnaIsJustThatDumb   )   )
    {
        UInt32 bitFlags = (
            0x000000FF |
            0x0000FF00 |
            0x00FF0000
        );
        throw new Exception(
            "People probably hate my style, I'm sure.\n" +
            "But y'all gotta do it or else this will get worse.\n" +
            "Seriously, have you seen this code?" +
            bitFlags
        );
    }
}

When in doubt, look at a folder that's been completed.

When tabs start getting crazy, you're likely going to want to check with git diff or various text editors to be sure that what you've got is correct. MonoDevelop in particular likes to be totally insane about '\t' tabbing sometimes, so while it might look like tabs are aligned in MonoDevelop, a sane text editor will probably tell you otherwise.

In addition to fixing the style, you're going to find that you're cleaning up a lot of extra stuff on the way... bizarre trailing whitespace will likely become your personal favorite, if interleaved tabbing styles does not win your heart. That said, try to keep the actual code the same. Only change the actual code when you're 100% sure that what you're changing is essentially a no-op change.

Remember, this is the hardest task on the FNA transition list. Assume the absolute worst before you start.

But, note that this is all that you have to change. Everyone else can deal with license headers, line endings, and all that other stuff, assuming you don't do this yourself.

Now, on to the files in question.

The following folders need work:
M - Content/ - Completed by @meklu 93fd738
M - Content/ContentReaders/ - Completed by @meklu 93fd738
H - Graphics/ - Completed by @extrahotchilipowder de44018 and flibit 9569acb 252b6b9
M - Graphics/PackedVector/ - Completed by flibit cb8157c
M - Graphics/Vertices/ - Completed by flibit c9ecd31
E - Input/ - Completed by @khbecker d42d412 and flibit 6d87b6d ea25e90
E - Input/Touch/ - Completed by @3vi1 09c633d with additions by flibit 3ecd0ad
E - Media/ - Completed by @3vi1 741baf3 with additions by flibit c740616
E - Storage/ - Completed by @3vi1 38edaa5 with additions by flibit bd7d2d2
H - ./ - Commits: 5e8b527 c902210 cb7fd61 c69880c 0a4ecb2 a671673 24f1eb3 37ba458 be42238 80b47e7 7aa9e27

E - Easy. Probably still work, but nothing deadly.
M - Medium. Things start to get ugly here.
H - Hard. I hope you're proudly unemployed while you're doing this.

The following folders have already been completed, and should be used as references:
Audio/
Graphics/States/
SDL2/

The following folders are NOT to be touched. This is because we need to keep MonoGame compatibility, or because it will be dealt with in a separate project:
GamerServices/
Graphics/Effect/
Graphics/Shaders/
Net/
Properties/
Utilities/

If you decide to take a folder, please post here and let us know you're working on it. Because we're doing so much work on so much code, we need to be sure we're not conflicting with other branches. Be sure to check ALL [FNA] issues before taking on a folder!

[meklu]

Taking Content/*, as I've done the initial sweep.

[meklu]

To anyone else willing to take a look at the other directories listed here, I've brewed a few one-liners that may help the effort.
https://gist.github.com/meklu/c6d1a7561ed315fbbdd5

[3vi1]

Thanks, meklu - I'll check those out before I jump in here.

[3vi1]

I'll take Storage

[3vi1]

Quickie:

Kill trailing whitespace, or whitespace in blank lines: sed -i 's/[\t ]*$//g' *.

Hopefully I'll get time to actually work on this tomorrow, today was very busy - and tonight just as much.

[khbecker]

A good check for line length is to run the file command on all the files and make sure they show up as ASCII text, not ASCII text, with very long lines.

You can use this bash script to do it for the current directory:
for f in *.cs; do file $f; done
or this to do it recursively
git ls-files . | grep '\.cs$' | xargs file $1

[3vi1]

You can use this bash script to do it for the current directory:

Why not just file *.cs?

[khbecker]

I see that works too. I never tried file with a wildcard.

[3vi1]

Shells in *nix expand wildcards before passing them to the command, so wildcards will work with any program that can take a list of files as input.

[khbecker]

Thanks. I didn't realize the shell handled that part of the wildcards.

[3vi1]

You're welcome. Shell expansion is an interesting and powerful feature, but it's good to be aware of what it's doing automagically because it means that programs/commands never see the wildcards (which is why you can't just do a mass rename with the mv command).

Now... if only I knew 1/10th as much about how git branches are supposed to work. :) Guess I'll pick it up as we go.

[3vi1]

I'll take on Media now, if you please.

[flibitijibibo]

Will take Graphics/Vertices/ when I get back tonight, while merging the region changes.

[3vi1]

I'll continue working my way up the list and take Input/Touch/.

[flibitijibibo]

Also going to be taking Graphics/ and Graphics/PackedVector/.

[flibitijibibo]

Just pushed a large chunk of cleanup for Graphics/: 9569acb

This cleans up all files except for the following:

• ModelBone.cs
• ModelBoneCollection.cs
• ModelEffectCollection.cs
• ModelMesh.cs
• ModelMeshCollection.cs
• ModelMeshPart.cs
• ModelMeshPartCollection.cs
• SpriteBatch.cs
• SpriteBatcher.cs
• SpriteBatchItem.cs
• SpriteFont.cs

If anyone wants to take some of these files, feel free. Everything else should be nice and clean now.

[3vi1]

I can knock out Input sometime in the next few days, if no one else wants it.

[flibitijibibo]

I'll leave Input/ unassigned for now, let me know if/when you start it and I'll mark it as assigned.

[khbecker]

If I'm not too busy, I might grab Input or the rest of Graphics tomorrow night. But if I haven't responded again by the time you're ready to start it, then go for it @3vi1 . I've been really busy this week, so I might not get a chance to contribute this week.

[khbecker]

Since no one else has claimed it yet, I'll take Input now.

[3vi1]

That's cool. I'm going to be really tied up with my job at least until Sunday. I'll see what I can help with at that point.

[extrahotchilipowder]

I'll take what's left of Graphics/

[extrahotchilipowder]

What's the official stance on stuff like this:

// Summary:
// Gets the name of this bone.
public string Name
{
get;
set;
}


Kill the comment or let it be? And also, if it stays, change it to /* */ style?

[flibitijibibo]

I believe those are actually supposed to be XMLdoc summaries. So it should probably be this:

// <summary>Gets the name of this bone.</summary>

Though I don't know if that's 100% correct... look around the source (or possibly SDL2#'s source) and see how the XML tags are meant to work. Don't remember off the top of my head.

[ghost]

XML tags require three /// before them: http://msdn.microsoft.com/en-us/library/b2s063f7.aspx

[flibitijibibo]

The only unassigned task left is ./. For this one, since the folder is so unbelievably massive, devs can pick out a small handful of files at a time, rather than having to take the entire folder. That might make this last goal a bit more appealing...

[3vi1]

I'll start at the bottom and take:

TextInputEventArgs.cs
Threading.cs
TitleContainer.cs
Vector2.cs
Vector3.cs
Vector4.cs

[flibitijibibo]

Up until now I've been swerving a bit on this, but thinking about it, you're right, we probably should remove those.

I'll double check all the old stuff when I do my second FNA pass, but from now on, go ahead and remove those bits.

[flibitijibibo]

Getting ContainmentType and CurveContinuity. Just a couple small enum files.

[3vi1]

I'll continue working my way up and do ./I* - M* tonight.

IDrawable.cs
IGameComponent.cs
IGraphicsDeviceManager.cs
IUpdateable.cs
LaunchParameters.cs
MathHelper.cs
Matrix.cs

[flibitijibibo]

If you want you can add this to your current PR while you fix the current issues in that one, I'd be cool with that. Looking at that list, only Matrix seems to be the real beast in there.

[3vi1]

Will do. It will probably be sometime tomorrow before I add them to the pull. I only got about halfway through Matrix as I spent half the night distracted with troubleshooting unrelated issues (Today's Ubuntu Trusty repo updates killed sound and broke KDE's 3D compositing... yippee).

[extrahotchilipowder]

I'll start these today. (4 of them will be really easy.)

CurveKey
CurveKeyCollection
CurveLoopType
CurveTangent
DisplayOrientation
DrawableGameComponent
FrameworkDispatcher
Game


[flibitijibibo]

We've done two passes through Matrix.cs (3vi1 and myself), if someone wants to do a third/fourth pass through that file, consider that one perpetually open. It's probably the largest file in the MonoGame project in the worst way possible.

You probably want to do a side-by-side comparison with the original while you're doing it:

https://github.com/mono/MonoGame/blob/develop/MonoGame.Framework/Matrix.cs

[3vi1]

I guess all that's left is for me to take:

GameComponentCollection.cs
GameComponentCollectionEventArgs.cs
GameComponent.cs
GamePlatform.cs
GameServiceContainer.cs
GameTime.cs
GameWindow.cs
GraphicsDeviceInformation.cs
GraphicsDeviceManager.cs

I'll see if I can't knock these out this morning before taking the son to see the new Captain America movie.

Edit: missed the GameComponent files at first due to ls sorting != MonoDevelop sorting.

[3vi1]

Well those went pretty quick. Once EHCP finishes what he has in progress, the root will be done!

[flibitijibibo]

We're down to the very last file in our first pass of style cleanup, and hilariously enough, it's Game.cs. Yup!

Joe's done the pass, but I've not reviewed it yet: https://github.com/flibitijibibo/MonoGame/pull/192/files#diff-7

Will try to review this today/tomorrow, but if you want to take a look at it yourself, feel free to do so. Like Matrix, it's kind of a massive file for lots of nasty reasons (many of which I intend to fix as soon as we're done with this task).

We're so close to being done with the first wave of FNA transition tasks!

[flibitijibibo]

We finished the last task! While the code is certainly not perfect, it should be much easier to understand and edit now.

I will be adding the second wave of tasks this week.