Scripting Standards

[kisuka 178]

Since the beginning of scripting in the Athena project, I don't believe, or can remember of, a set of guidelines / standards which have ever been released. Standards are created as a way to develop a style which remains consistent among all script releases.

 

Due to the lack of standards, most athena scripts are styled depending on the scripter / author's personal preference. This has resulted in a mix of styles, which have become hard to read, and even harder to contribute to in the future (due to ugly script styling).

 

Because of this, I would like to propose a set of Scripting Standards, to finally get us on track to clean scripts in the Athena Project.

 

Attached are the standards I have developed, as well as an example script following the standards. Please review them and be sure to let me know what you think.

 

This is by no means accepted into the community yet, I would like feedback first before this is even considered to be official.

 

Thank You.

 

(Files best viewed in : Sublime Text 2 or Notepad++)

alberta.txt

AS Standards.txt

[jaBote 438]

I like the standards, but maybe this could break that:

 

announce "this is such a great and big announcement that can maybe be inside of an official script, how will you cut it out?",bc_all;

 

And only thing I don't like is that I'd prefer to see this on an if-else statement because I feel it's more clear:

 

if (this) {}else if (that) {}else {}

 

P.S.: I can't seem to download the alberta text?

[kisuka 178]

I like the standards, but maybe this could break that:

 

announce "this is such a great and big announcement that can maybe be inside of an official script, how will you cut it out?",bc_all;

 

And only thing I don't like is that I'd prefer to see this on an if-else statement because I feel it's more clear:

 

if (this) {}else if (that) {}else {}

 

P.S.: I can't seem to download the alberta text?

else if would indeed look like that using the K&R styling. I'll specify an else-if statement example in the standards.

 

hmm... that's a good question regarding the long announce command. Perhaps multi-line solution? like...

 

announce "	this is such a great and big announcement	that can maybe be inside of an	official script, how will you cut	it out?",bc_all;

The longest announcement command currently in official is about 217 characters.

 

The 80-character limit is something should be given lots of feedback about. I only specified 80 characters due to the fact that it fits somewhat nicely in a terminal window, and is an old standard. PHP for example allows for a limit of 120 characters per line. So this rule should be given a limit which everyone agrees is the best 'standard' limit that also fits all commands nicely. Basically the limit would impose that a 'good script' doesnt have an announce command that has more than 200+ characters.

 

One purposed suggestion could be that the line limit be 255 characters and that message commands be on 1 line up until 255 line limit then terminated. This would organize the scripts better in-game anyways in my opinion as they'd be cut off at the 35 character limits automatically, rather than manually specifying them in the script.

 

There could also just be no line limit if people agree on this.

[Haru 290]

Nice, I like this. Here's my feedback:

 

- I believe 'else' and 'else if' statements are less confusing if they're placed on the same line as the closing curly brace of the preceding 'if' block. To say, I prefer '} else if (something) {'. This is just personal preference though.

 

- Maximum line length should be either 80 or 120 characters. Any more than that is annoying.

 

- We already have a way to split strings across lines. It's almost as you suggested, but with one difference: you need to terminate the string with " after each line (the script parser detects them and joins them while loading the script, just like it happens in c):

announce "this is such a great and big announcement"         " that can maybe be inside of an"         " official script, how will you cut"         " it out?", bc_all;

After being parsed, the string will contain no extra spaces, and won't cause any concatenation operations at runtime, just as if it was written on one line.

 

- One line if-else statements are fine without braces. But if either the if or the else needs braces, then both should have them (Kernel-style).

 

- 'switch', 'for', 'while', should be treated the same way as 'if' (a space between the keyword and the parentheses) to better differentiate them from function calls.

 

- No camel case please... it's ugly... (personal_preference). Regardless of the camel-case preference, though, it might be worth to recommend to avoid capitalizing the first letter of variable names, so that they won't conflict with item name constants.

 

- Considering how frequently we use nested 'switch' statements, I think the 'case' labels should be indented with the enclosing block (Kernel-style), so that each switch statements takes up one indentation level rather than two. I'm suggesting this because it'd cause line-length issues (and horizontal scrollbars) in many scripts.

 

- I'd add a note about using tabs to indent (at the beginning of the line), and spaces to align (at mid-line, i.e. to align comments or values)

 

- I'd add a note about the naming of functions (perhaps F_name?) and labels (perhaps OnSomething?)

[kisuka 178]

- So basically, change K&R to Kernel Style instead? Anyone have any input on this? I'm fine with it. I just want us to decide on a standard.

 

Kernel : https://www.kernel.org/doc/Documentation/CodingStyle

K&R : http://en.wikipedia.org/wiki/1_true_brace_style#K.26R_style

 

- I'd vote for 80 characters, for the fact that the limit per NPC message line is 35 characters. Also organizes the script header nicely (we had it at 64 characters previously for the header, so we get a bit more room now, 120 seems overkill for the header.)

 

- I'm fine with the underscore method for variable names. If it's underscore method there should be no uppercases. Should be all lowercase with _ representing a space between words.

 

Completely agree with your changes Haru.

[pan 87]

I don't know why nobody ever says anything about using Allman style, in my opinion it's the easiest to read and to maintain.

if (condition){}else{}

Or even a variation that I personally prefer to read:

if( condition ){}else{}

And also I think sometimes shorthanding is a good way to save time and still make good code, other times you could use the ternary operator... And why don't use set to define variables? If it's not all right to use this syntax we should "remove" it from our documentation and just leave the entry in our source to keep backwards-compatibility.

[Haru 290]

I don't know why nobody ever says anything about using Allman style, in my opinion it's the easiest to read and to maintain.

if (condition){}else{}

Or even a variation that I personally prefer to read:

if( condition ){}else{}

 

 

I disagree. It makes it unclear at first sight that the block is related to that specific if statement, and it uses too much vertical space unnecessarily. (For the same reason why I dislike excessive amounts of empty lines inside functions, in general. The less lines of code you can fit in your screens, the harder it becomes to understand what a function does.)

And why don't use set to define variables? If it's not all right to use this syntax we should "remove" it from our documentation and just leave the entry in our source to keep backwards-compatibility.

In some (very few) cases it's necessary to use it, so it needs to be kept in the documentation (perhaps also noting that it is deprecated and it should not be used unless strictly necessary.)

 

An example where it is necessary is when you want to set a variable from another npc:

set getvariableofnpc(.foo, "My Other NPC"), 1;

[pan 87]

I disagree. It makes it unclear at first sight that the block is related to that specific if statement, and it uses too much vertical space unnecessarily. (For the same reason why I dislike excessive amounts of empty lines inside functions, in general. The less lines of code you can fit in your screens, the harder it becomes to understand what a function does.)

And why don't use set to define variables? If it's not all right to use this syntax we should "remove" it from our documentation and just leave the entry in our source to keep backwards-compatibility.

In some (very few) cases it's necessary to use it, so it needs to be kept in the documentation (perhaps also noting that it is deprecated and it should not be used unless strictly necessary.)

 

An example where it is necessary is when you want to set a variable from another npc:

set getvariableofnpc(.foo, "My Other NPC"), 1;

 

 

Why not rewrite getvariableofnpc to behave like an identifier and work just like a regular variable would?

getvariableofnpc(.foo, "My Other NPC") = 1;

I prefer Allman's because it 'highlights' each block, making it easier to read, I also think that when functions are too crowded they are harder to read, but it's just a personal preference.

[Haru 290]

Why not rewrite getvariableofnpc to behave like an identifier and work just like a regular variable would?

getvariableofnpc(.foo, "My Other NPC") = 1;

 

 

I can't think of a proper way to do so... The language syntax doesn't let you assign a value to the result of a function, probably because functions don't have a return type (and even when they in fact normally return a reference, you can't know that at parse time, because they could return void at parse time)

If you have any good ideas... I don't :x

[kisuka 178]

Can you explain what you mean when you say it 'highlights' the block of script pan? Cuz I have my own syntax highlighter in Sublime Text 2 specific for scripts, so my blocks are always highlighted if I select one of the braces of that block.

[pan 87]

I can't think of a proper way to do so... The language syntax doesn't let you assign a value to the result of a function, probably because functions don't have a return type (and even when they in fact normally return a reference, you can't know that at parse time, because they could return void at parse time)

If you have any good ideas... I don't :x

Well, we can think of something, I'll try to study our current parsing engine and see what can be done c:

Can you explain what you mean when you say it 'highlights' the block of script pan? Cuz I have my own syntax highlighter in Sublime Text 2 specific for scripts, so my blocks are always highlighted if I select one of the braces of that block.

It "separates" a block from the rest of the code, and also I think it makes easier to identify quickly different parts of the code, as it becomes less crowded.

[saithis 0]

Personally, I think the indent is enough to visually separate the block.

And Allman style means more scrolling and less code fitting on the screen, which is bad.

[KeyWorld 87]

Allman style can't be used because currently we can't do:

-	script	test	-1,{	}

Or:

function	script	test{	}

Or:

-	script	test	-1, {	function test	{	}	test();}

 

(Except if i miss an update).

[jaBote 438]

Main problem about this, is that I think everybody will try to defend their current scripting style more than reach to an agreement

[pan 87]

Main problem about this, is that I think everybody will try to defend their current scripting style more than reach to an agreement

That's why we need to show pros and cons and then make a poll to know which one is most accepted, I'm already using K&R in our source code, I don't mind switching styles in different projects, I just mentioned Allman's identation because nobody ever mentions it, and I, personally, find it better to read and to write than other styles.

Allman style can't be used because currently we can't do:

-	script	test	-1,{	}

Or:

function	script	test{	}

Or:

-	script	test	-1, {	function test	{	}	test();}

 

(Except if i miss an update).

 

 

We can always change our scripting engine to make comply with other standards, that's not really an issue... But I see that it could be a waste of time. I always thought that our headers are not very good, but changing such a 'basic' thing would make a lot of people confuse :x

[Haru 290]

Yeah, I don't think it's wise to change the NPC header at this point, no matter which standard we agree on. The only affected things would be what's inside, not the "top level commands".

 

In any case, here's one more reason for me to prefer K&R/Kernel style to Allman. Look at how they fold:

 

(screenshots taken in vim, with :set foldmethod=indent)

 

 

 

First screenshot only has as many lines as it needs to show the structure, one element per line. Second one seems like an excess of wasted space (two or three extra lines for each fold), and on long scripts it makes it much harder to understand at a glance, since unless you have a large screen, you won't be able to see the entire structure at once.

 

I'm not sure if it's just me, but, by looking at those screenshots, I also notice how, in the first one, the first two if conditions are separated while all the others are connected by if-else. In the second screenshot, that's not so immediately evident unless you actually read the code. And enforcing a blank line between if blocks if they're not related to each others would feel like rubbing salt on the wound (producing extra, unnecessary vertical whitespace.)

[kisuka 178]

My vote is for Kernel style, since majority of scripts already use this style. Some small tweaks would need to be made still but for the most part, most scripts already are in a form of kernel style.

 

Once we agree on the style we should figure out the final decision on this max line length thing.

 

Typically with kernel style, the max length is 80 characters per line.

[dudemelo 3]

Kernel Style.. with up to 120 characters per line!

Edited March 28, 2014 by dudemelo

[pan 87]

My vote is for Kernel style, since majority of scripts already use this style. Some small tweaks would need to be made still but for the most part, most scripts already are in a form of kernel style.

 

Once we agree on the style we should figure out the final decision on this max line length thing.

 

Typically with kernel style, the max length is 80 characters per line.

As everybody seems to hate Allman's and most scripts already follow this standard ): I agree. I think 80 characters per line is a good limit see this answer: http://stackoverflow.com/a/578318/929395

 

@Haru

I think when folding it can make a little more difficult, but when everything is unfolded it just seems better, but that's just my opinion. That's why standards need to be polled before being implemented, some things are "more personal" than others

[jaBote 438]

I'd also go for teaching these code standards in our scripting documentations or in another file, but making reference to it since they're good practice =)

[Haru 290]

Before deciding on the line length (I'm personally for 80, but I'm still fine with 120), I think we should check if limiting it to 80 would cause issues with some scripts (too many indentation levels, or too many NPC headers too wide to fit, since those can't be split into multiple lines, and need to be an exception to the line-length rule, and would cause horizontal scrolling regardless)

[kisuka 178]

Before deciding on the line length (I'm personally for 80, but I'm still fine with 120), I think we should check if limiting it to 80 would cause issues with some scripts (too many indentation levels, or too many NPC headers too wide to fit, since those can't be split into multiple lines, and need to be an exception to the line-length rule, and would cause horizontal scrolling regardless)

 

Agreed. What script do we have with the most indents? Figure that'd be the best thing to test it out on.

[Haru 290]

npc/quests/newgears/2005_headgears.txt has a block with twelve indents, but it's inside several switch() blocks, so it'll decrease to just 7 if you use single-indentation for switch/case. npc/quests/seals/megingard_seal.txt has a line with eleven indents, and a block with ten (and those aren't heavily nested inside switch/case)

[dudemelo 3]

The longest line has exactly 120 characters.

[Haru 290]

*throws note*

 

The script header should contain a reference to what license the script uses ("GNU General Public License version 3 or later" for all the currently-shipping scripts, unless otherwise specified). The GPL encourages us to do so, because it makes it clear what the license is, even if the file is distributed separately from the emulator.