April 20, 2024, 02:10:43 am

The Gang Garrison 2 Forum

Please login or register.

Login with username, password and session length
Advanced search  

News:

NOTICE: Wondering where all the forums have gone?

Join the community Discord server!

Pages: [1]

Author Topic: Code standards (again)  (Read 32295 times)

Phantom Brave

  • All Hail Classicwell
  • Moderator
  • *****
  • Karma: 70
  • Offline Offline
  • Posts: 12532
  • Another one --
Code standards (again)
« on: August 02, 2013, 11:36:57 pm »

Because I seem to be the only one who remembers the old docs I decided to try to rewrite them. If anyone has any objections just argue here. The boolean operator thing and braces being on newlines are essential though.


Semicolons are *always* required at the ends of statements.
x = x + hspeed;


Parens are *always* required around conditionals [and their sister blocks: with(), switch(), and repeat()].
if (mode == 2)
    draw_sprite_ext(TimerOutlineS, 0, xoffset+xsize/2, yoffset, 2, 2, 0, c_white, 1);



Spacing between parens, if, and conditionals is so inconsistent in the codebase that we don't have a standard for it. Go with what looks good. Same with operations, go with what makes the expression easily readable. However, it must always be symmetrical.
if(variable);
if( variable );
if (variable);



Always space out the set statement sides from eachother.
x = 5/7;


When crossing order-of-operations boundries *without* parens, space the later operation away from the earlier operation.
x = 3 + 4/8;
y = 5*(x+2);



Block braces should be on a new line and indentation is four spaces.
if(something_is_true)
{
    werunablock = true;
    something();
}



You may skip using braces on single-line statements under certain situations.
if(variable)
    thing;
else
    other_thing;



You may NOT skip braces on conditions with other conditions inside of them.
while(1)
{
    if(x > n)
        break;
    else
        x += 1;
}

if (x/y > 5)
{
    if(amOnFire)
        hp -= damage;
    else
        . . .
}



You may skip braces with very simple series-es of single blocks, but not ones with multiple children blocks.
with (BladeB)
    if (ownerPlayer == other.ownerPlayer)
        instance_destroy();

loopsoundstop(MedigunSnd);
if(instance_exists(healTarget))
    if(healTarget.object != noone)
        healTarget.object.healer = noone;

with (Player)
{
    if (object == global.myself)
        . . .
    else
        . . .
}



Don't indent switch blocks; indent case blocks.
switch(player.commandReceiveCommand)
{
case PLAYER_LEAVE:
    socket_destroy(player.socket);
    player.socket = -1;
    break;
   
. . .
}



*Always* use english versions of boolean operators. The symbol versions play well with neither gmksplit nor GM's build in code editor.
if (intel and (ubered or global.mapchanging) and global.isHost)
    . . .



Use newlines to seperate logically distinct segments of code.
// drop intel if ubered or round is over
if (intel and (ubered or global.mapchanging) and global.isHost)
{
    sendEventDropIntel(player);
    doEventDropIntel(player);
}

//gotta regenerate some nuts
nutsNBolts = min(nutsNBolts+0.1, maxNutsNBolts);



Boolean expressions should always be encased in parentheses.
if (global.myself != -1)
{
    teamoffset = (global.myself.team == TEAM_BLUE);
    . . .



Variable names should start with a lowercase letter, with subsequent words capitalized (intercapping). Shorthand words in non-state variables (that is, adhoc logic variables) can fuzz this rule a little, and so can variables which pretend to be part of a GM variable family, but there's a limit.
thisIsAVariable
teamoffset
yoffset
timerPos
menu_script_back
stepHasRun


Object names should always start with a capital letter and be intercapped after that.
DeathCam

Sprite names should, when applicable, be the name of their associate object with a capital S "kind" suffix. Otherwise, they still need the S.
ScoutRedS

Script names should start with a lowercase letter and be intercapped after that, unless they emulate being in a "family" of GM functions.
setChatBubble
gotoInternalMapRoom
createGib
ds_list_sorted_insert
game_init
draw_roundtimer


Constants are always all capital letters, with underscores to separate words.
PROTOCOL_UUID
CHANGE_MAP
« Last Edit: August 06, 2013, 11:57:03 pm by Phantom Brave »
Logged

http://steamcommunity.com/id/wareya/
ladies and gentlemen i would like to announce that the fact of the matter is up that the fact of the matter is a fact and it matters

cspotcode

  • Coder
  • Administrator
  • *****
  • Karma: 134
  • Offline Offline
  • Posts: 412
Re: Code standards (again)
« Reply #1 on: August 05, 2013, 02:38:56 pm »

Good idea.  Once this is finalized it should go in Github in the Documentation folder.  The old docs were lost because they're on this forum and not preserved in the source repository.

Can anyone find the old coding standards thread?

Because I seem to be the only one who remembers the old docs I decided to try to rewrite them. If anyone has any objections just argue here. The boolean operator thing and braces being on newlines are essential though.


Spacing between parens, if, and conditionals is so inconsistent in the codebase that we don't have a standard for it. Go with what looks good. Same with operations, go with what makes the expression easily readable.
if(variable);
if( variable );
if (variable);



We should require parentheses around the expression.

// can not do this
if variable
// do this
if(variable)


I propose that we standardize the spacing even though it's currently inconsistent.  Below is my personal style, but I'm ok with doing something else as long as it's standardized.

// Zero spacing between the statement, parentheses, and expression
if(variable)
// do not do any of these
if (variable)
if( variable ) // <-- I would be ok with this style, too
if ( variable )


Always space out the set statement sides from eachother.
x = 5/7;


When crossing order-of-operations boundries *without* parens, space the later operation away from the earlier operation.
x = 3 + 4/8;
y = 5*(x+2);



I propose that we require a single space between all operators, but not on the inside of parentheses.

// Like this
x = 3 + 4 / 8;
y = 5 * (x + 2);


Block braces should be on a new line and indentation is four spaces.
if(something_is_true)
{
    werunablock = true;
    something();
}



You may skip using braces on single-line statements under certain situations.
if(variable)
    thing;
else
    other_thing;



You may NOT skip braces on conditions with other conditions inside of them.
while(1)
{
    if(x > n)
        break;
    else
        x += 1;
}

if (x/y > 5)
{
    if(amOnFire)
        hp -= damage;
    else
        . . .
}



You may skip braces with very simple series-es of single blocks, but not ones with multiple children blocks.
with (BladeB)
    if (ownerPlayer == other.ownerPlayer)
        instance_destroy();

loopsoundstop(MedigunSnd);
if(instance_exists(healTarget))
    if(healTarget.object != noone)
        healTarget.object.healer = noone;

with (Player)
{
    if (object == global.myself)
        . . .
    else
        . . .
}



Don't indent switch blocks; indent case blocks.
switch(player.commandReceiveCommand)
{
case PLAYER_LEAVE:
    socket_destroy(player.socket);
    player.socket = -1;
    break;
    
. . .
}



Works for me.



*Always* use english versions of boolean operators. The symbol versions play well with neither gmksplit nor GM's build in code editor.
if (intel and (ubered or global.mapchanging) and global.isHost)
    . . .



How do the symbol versions cause trouble with gmksplit?  I'm very much in favor of using the symbol versions exclusively because I think it creates more readable code.

Use newlines to seperate logically distinct segments of code.
// drop intel if ubered or round is over
if (intel and (ubered or global.mapchanging) and global.isHost)
{
    sendEventDropIntel(player);
    doEventDropIntel(player);
}

//gotta regenerate some nuts
nutsNBolts = min(nutsNBolts+0.1, maxNutsNBolts);




Yes!  Blank lines spacing out the code make me so happy.

Finally, we should require semicolons at the end of every statement, just for consistency.
« Last Edit: August 05, 2013, 02:40:26 pm by cspotcode »
Logged
"The knack of flying is learning how to throw yourself at the ground and miss."
- Douglas Adams

cspotcode

  • Coder
  • Administrator
  • *****
  • Karma: 134
  • Offline Offline
  • Posts: 412
Re: Code standards (again)
« Reply #2 on: August 05, 2013, 02:46:00 pm »

Logged
"The knack of flying is learning how to throw yourself at the ground and miss."
- Douglas Adams

Phantom Brave

  • All Hail Classicwell
  • Moderator
  • *****
  • Karma: 70
  • Offline Offline
  • Posts: 12532
  • Another one --
Re: Code standards (again)
« Reply #3 on: August 05, 2013, 07:29:32 pm »

Added semicolon and paren reqs. I forgot about them because they're so basic, hahaha.

>I propose that we standardize the spacing even though it's currently inconsistent.  Below is my personal style, but I'm ok with doing something else as long as it's standardized.

I think it would be too much of a hassle to do so. For one, we would have to change existing code over eventually, for negligible benefit. For two, I'm of the strong opinion that different expressions and contexts make different spacings more readable or pretty than others.

>I propose that we require a single space between all operators, but not on the inside of parentheses.

Well, forcing spacing all the time on expressions does have some annoying effects. For one, that means we can't do this anymore:
draw_generictimer(xoffset+xshift, yoffset+yshift, xsize, ysize, timer, teamoffset, 0);
...where the xoffset+xshift are part of the same contiguous block, setting them off as a single small argument to the function. There's a bunch of small things like that where having no spacing may be a good thing, and unless you have a totally one track mind, parsing spacing when you know what you're looking at in general shouldn't be a speedbump, or am I wrong?
« Last Edit: August 05, 2013, 10:30:09 pm by Phantom Brave »
Logged

http://steamcommunity.com/id/wareya/
ladies and gentlemen i would like to announce that the fact of the matter is up that the fact of the matter is a fact and it matters

cspotcode

  • Coder
  • Administrator
  • *****
  • Karma: 134
  • Offline Offline
  • Posts: 412
Re: Code standards (again)
« Reply #4 on: August 06, 2013, 11:27:33 am »

>I think it would be too much of a hassle to do so. For one, we would have to change existing code over eventually, for negligible benefit. For two, I'm of the strong opinion that different expressions and contexts make different spacings more readable or pretty than others.

We'll have to change existing code to add missing semicolons and parentheses anyway so I don't think this is too terrible.

>There's a bunch of small things like that where having no spacing may be a good thing, and unless you have a totally one track mind, parsing spacing when you know what you're looking at in general shouldn't be a speedbump, or am I wrong?


Agreed that parsing different forms of spacing is not a speedbump, but I still think that spaces around all operators is overall more readable and consistent.  It's also more enforceable by a linting tool, although I know we don't have one for gmk-splitter.

Which quotes should we use for strings?  Single or double?  My vote is for single.

Should we allow end-of-line comments?  I say no.

You should also incorporate the decisions made in the old style thread: http://www.ganggarrison.com/forums/index.php?topic=25052.0
The stuff about variable, object, and constant naming convention is important.
Logged
"The knack of flying is learning how to throw yourself at the ground and miss."
- Douglas Adams

Orpheon

  • Moderator
  • *****
  • Karma: 15
  • Offline Offline
  • Posts: 6409
  • Developer
Re: Code standards (again)
« Reply #5 on: August 06, 2013, 03:53:47 pm »

I agree totally on the original wareya suggestion and less on cspots changes.

Parantheses around expressions are good and proper when there's an actual expression involved (x+y>2), or at least some form of comparision, but in cases like
Code: [Select]
if cloaking
{
   return;
}
I find them pretty unnecessary and less aesthetic than leaving them out.

Same goes for spacing, this really depends on the context.
Even in the case of mathematical operations, being able to group non-important operations by giving them no spaces and as such accentuating the important ones helps readability imo.
Code: [Select]
area = x+width * y+height;I find is more readable than
Code: [Select]
area = x + width * y + height;
The symbol versions cause problems with gmksplit because xml uses symbols like "<" and such they are replaced to "&lt;" or "&gt;", which are catastrophes to reading.
I don't really see how || is more readable than "or" anyways.
Code: [Select]
if (key[UP] and (place_free(x, y+1) or canDoubleJump) and not taunting
{
   vspeed -= jumpSpeed;
}
Code: [Select]
if (key[UP] && (place_free(x, y+1) || canDoubleJump) && !taunting
{
   vspeed -= jumpSpeed;
}
It is just slightly more compact and not at all more readable (!taunting can easily be confused with taunting at a glance).

I agree on semicolons though, it's a habit that is really hard to break and has very little reason for or against it. The majority of people here put semicolons by habit, so making it obligatory wouldn't be dumb.
« Last Edit: August 06, 2013, 03:54:21 pm by Orpheon »
Logged

Phantom Brave

  • All Hail Classicwell
  • Moderator
  • *****
  • Karma: 70
  • Offline Offline
  • Posts: 12532
  • Another one --
Re: Code standards (again)
« Reply #6 on: August 06, 2013, 10:24:31 pm »

>We'll have to change existing code to add missing semicolons and parentheses anyway so I don't think this is too terrible.

The thing about semicolons is that they're an "absolute standard" among c-like languages. On the other hand, "people" haven't agreed on a way to space out parens yet, and it affects readability in a complicated way. That is, these two blocks both have perfectly readable styles, and switching them wouldn't help that:
    for (i = 0; i < items; i += 1)
    {
        itemy = view_yview[0]+ybegin+i*spacing;
        if(drawbg)
        {
    . . . . .


>Which quotes should we use for strings?  Single or double?

We use double quotes pretty extensively, and I think we should stick with that. As well, there's trouble when you have to nest strings, ie in plugins:
    object_event_add(InGameMenuController,ev_create,0,'
        menu_addlink("Plugin Options", "
            instance_destroy();
            instance_create(0,0,global.pluginOptions);
        ");
    ');

In that case, people seem to have "agreed" that the outer string should be single quotes, and the inner string (which happens to be inside the more significant bit of code) should be double quotes.

>Should we allow end-of-line comments?

I say yes. Example of how I use them:
    do // bluh I really want goto right here but I have to use a loop instead
    {
        if(onground and step_free and step_there and vspeed >= 0) // valid downward slope
        {
            remainingy = 0;
            y = round(y);
            y += slopesize;
            break;
        }
       
        floor_free = place_free(x, y+1); // makeshift caching for collision operations
        here_free = place_free(x, y);
        ongroundnew = here_free and !floor_free;
        generally_free = place_free(x+hspeedpart, y+vspeedpart);
    . . .


>I find them pretty unnecessary and less aesthetic than leaving them out.

I think that's because you're used to languages where they're not necessary, but to me, skipping parens is just as bad as skipping semicolons.

The symbol versions cause problems with gmksplit because xml uses symbols like "<" and such they are replaced to "&lt;" or "&gt;", which are catastrophes to reading.

The bigger reasons for me are that GM's code editor doesn't highlight them or recognize them as a "word", so you can't see them at a glance, distinguish them without parsing the texture, or ctrl+arrows over them.

>(!taunting can easily be confused with taunting at a glance)

"Not" is the one boolean operator which I believe should be used in symbol form. It's a single character, so there's no advantage to it not/being registered as a "word" within GM, and you can place it directly adjecant to whatever it is operating on, which makes absolute obvious that it's only operating on one value in the expression, and you can't do that with the actual word version. I just wish they were highlighted. Also, I disagree that it can easily be confused more than the word version.

Will look at some other things later tonight.
« Last Edit: August 06, 2013, 11:27:53 pm by Phantom Brave »
Logged

http://steamcommunity.com/id/wareya/
ladies and gentlemen i would like to announce that the fact of the matter is up that the fact of the matter is a fact and it matters

Phantom Brave

  • All Hail Classicwell
  • Moderator
  • *****
  • Karma: 70
  • Offline Offline
  • Posts: 12532
  • Another one --
Re: Code standards (again)
« Reply #7 on: August 06, 2013, 11:55:07 pm »

Added several sections. Boolean expression parens, and identifiers.
Logged

http://steamcommunity.com/id/wareya/
ladies and gentlemen i would like to announce that the fact of the matter is up that the fact of the matter is a fact and it matters

Orpheon

  • Moderator
  • *****
  • Karma: 15
  • Offline Offline
  • Posts: 6409
  • Developer
Re: Code standards (again)
« Reply #8 on: August 07, 2013, 07:14:18 am »


I think that's because you're used to languages where they're not necessary, but to me, skipping parens is just as bad as skipping semicolons.

Semicolons too are unnecessary, but they don't hurt readability for people who aren't used to them. Parentheses do.


The bigger reasons for me are that GM's code editor doesn't highlight them or recognize them as a "word", so you can't see them at a glance, distinguish them without parsing the texture, or ctrl+arrows over them.

This hasn't really bothered me but I can understand it, was mostly responding to cspotcode as he asked why it caused problems with gmksplit specifically.

"Not" is the one boolean operator which I believe should be used in symbol form. It's a single character, so there's no advantage to it not/being registered as a "word" within GM, and you can place it directly adjecant to whatever it is operating on, which makes absolute obvious that it's only operating on one value in the expression, and you can't do that with the actual word version. I just wish they were highlighted. Also, I disagree that it can easily be confused more than the word version.
I don't like it, but sure. The logic is sound.
« Last Edit: August 07, 2013, 07:14:45 am by Orpheon »
Logged

Phantom Brave

  • All Hail Classicwell
  • Moderator
  • *****
  • Karma: 70
  • Offline Offline
  • Posts: 12532
  • Another one --
Re: Code standards (again)
« Reply #9 on: August 07, 2013, 07:28:57 am »

>Semicolons too are unnecessary, but they don't hurt readability for people who aren't used to them. Parentheses do.

A lack of parenthesis also hurts readability for people who *are* used to them. We just need to pick a side and hold it.
Logged

http://steamcommunity.com/id/wareya/
ladies and gentlemen i would like to announce that the fact of the matter is up that the fact of the matter is a fact and it matters

ajf

  • (Ex-?)Developer and forum/web admin
  • *****
  • Karma: 7
  • Offline Offline
  • Posts: 3421
  • she's never quite as dead as you think
Re: Code standards (again)
« Reply #10 on: August 09, 2013, 09:32:16 pm »

Shouldn't this be stickied?

Eh, it's pointless once my pull request is merged, since it'll be in the repo.
Logged
did you know that spinning stars work like this???

I've seen things you people wouldn't believe. execute_strings on fire off the shoulder of Overmars. I watched object-beams glitter in the dark near the room_goto_fix. All those moments will be lost in time, like tears...in...rain. Time to die.

MedO

  • Owns this place
  • *****
  • Karma: 151
  • Offline Offline
  • Posts: 1752
Re: Code standards (again)
« Reply #11 on: August 11, 2013, 01:49:15 pm »

I reviewed this now as part of the pull request and my take is that the coding guidelines as proposed here go a bit too far. Indentation and brace placement affect readablilty quite a lot, but do we really want to waste time fighting over where to place spaces in an expression? I say focus on the most important readability points and just cope with the rest instead of imploring new contributors to learn a larger set of rules. And I'd prefer to have people thinking about the code design than about some minor layout issue.
Logged
Quote from: Alfred North Whitehead
It is the business of the future to be dangerous; and it is among the merits of science that it equips the future for its duties.

Quote from: John Carmack
[...] if you have a large enough codebase, any class of error that is syntactically legal probably exists there.

ajf

  • (Ex-?)Developer and forum/web admin
  • *****
  • Karma: 7
  • Offline Offline
  • Posts: 3421
  • she's never quite as dead as you think
Re: Code standards (again)
« Reply #12 on: August 11, 2013, 04:39:50 pm »

I reviewed this now as part of the pull request and my take is that the coding guidelines as proposed here go a bit too far. Indentation and brace placement affect readablilty quite a lot, but do we really want to waste time fighting over where to place spaces in an expression? I say focus on the most important readability points and just cope with the rest instead of imploring new contributors to learn a larger set of rules. And I'd prefer to have people thinking about the code design than about some minor layout issue.
True, but we need common standards. Arctic made a pull request earlier which was unreadable.
Logged
did you know that spinning stars work like this???

I've seen things you people wouldn't believe. execute_strings on fire off the shoulder of Overmars. I watched object-beams glitter in the dark near the room_goto_fix. All those moments will be lost in time, like tears...in...rain. Time to die.
Pages: [1]
 

Page created in 0.034 seconds with 36 queries.