DemoProductions / shmup

3 stars 2 forks source link

Clean up code #51

Closed flip40 closed 8 years ago

flip40 commented 8 years ago

Since we are chilling on the code for a bit for discussion, I want to clean up some of the stuff we recently added in the meantime.

ghost commented 8 years ago

Not that important, but maybe we can establish the curly brace convention?

flip40 commented 8 years ago

Sure. If I recall from an earlier commit by you, you want

function()
{
  stuff
}

As opposed to

function() {
  stuff
}

correct? I'll also go fix any that don't use braces at all to have them.

if (true)
  stuff
if (true)
{
  stuff
}
ghost commented 8 years ago

Yeah that's correct. Sounds good

ghost commented 8 years ago

Monodevelop also seems to default to using tab characters instead of spaces. Lets make sure each source file has spaces that replace tabs

flip40 commented 8 years ago

Was looking over Weapon class, I have a proposed change (will make it an issue as well if you agree), I think we should remove the bool for canShoot and simply rely on the Shoot function to determine if it can instead of the player checking. The player doesn't necessarily care if it can shoot, but it will try, and the Weapon will then decide if it can or not. To still inform the player the method can return a bool (true for shot, false for didn't). It would also shorten update to only update the time, and shoot to then check the time to see if it can shoot.

Thoughts?

ghost commented 8 years ago

Was looking over Weapon class, I have a proposed change (will make it an issue as well if you agree), I think we should remove the bool for canShoot and simply rely on the Shoot function to determine if it can instead of the player checking. The player doesn't necessarily care if it can shoot, but it will try, and the Weapon will then decide if it can or not. To still inform the player the method can return a bool (true for shot, false for didn't). It would also shorten update to only update the time, and shoot to then check the time to see if it can shoot.

Makes more sense, lets make that change

Pull request for this issue merged: #53

ghost commented 8 years ago

tab issue and some inconsistent use of 'this' keyword

flip40 commented 8 years ago

If you are not using MonoDevelop on mac this may not apply, but...

preferences -> text editor -> behavior, uncheck "Enable on the fly coding", might help stop bad formatting curly braces. However, we can also set a policy so that this can be left checked (see below), just pointing out it's location.

Checking "format on save" might be nice too, but that might also really mess things up if we don't quite have this format finalized (again, see below).

Following this (http://stackoverflow.com/questions/10986435/how-can-i-over-ride-the-default-code-formatting-policy-in-monodevelop)

...import this policy and apply it to the project: (removed the link, just ask for it in Discord and I'll upload it... was making some revisions and I don't think Filedropper keeps those up permanently anyways).

It does most of the formatting I believe we are looking for. There are some cases the leave curly braces on the same line, but they are cases where doing that almost certainly adds clarity. For example, for some anonymous functions and allowing for while to be on the same line if reverse while ({} while();), etc. Normal functions, if statements, etc. follow the new line for the curly brace though.

Feel free to go through it, if you think we should make some changes we can set up another format policy, but we should make sure we use the same ones.

flip40 commented 8 years ago

101