Topic: AGS Engine Coding Conventions - Proposal Draft 1.0

[AGA]

Should probably put "Copyright (C) 2000-2010, Chris Jones; 2010-2012 Whoever Else" or something, to be fair.

[Crimson Wizard]

Here's a funny question on naming.
Consider we have a variable which name contains abbreviation, e.g. MP3. How should it be named:
int Mp3Player;
or
int MP3Player;


BTW, monkey, how's the final convention going?

[monkey0506]

Generally I'm inclined to lean toward "MP3Player", because some abbreviations are quite ugly otherwise IMO (like "Gui").

Regarding the copyright question, 'tis an interesting and easily debatable one. Certainly wouldn't want to step on anybody's toes, but perhaps we could give some generic notation to the "AGS Engine Source Contributors" where appropriate and then we could notate the individual contributors of each version in the changelog, etc. I imagine that most of those who would be contributing to the source would be okay with something like this, vs. listing the name of every contributor to each individual file...?

Among other things, I am working on revising the conventions, trying for the most part to take note of everyone's feedback, but I didn't want to add dozens of intermittent revisions to the wiki either. I'll let you know when I've gotten it finished.

[JJS]

^^ I agree on both accounts. The source contributers could also be listed in a separate file. They should certainly not be all in the file header, which should be designed so that it ideally doesn't have to be changed at all in the future. Basically just the project name, a generic copyright notice and the license should do.

[Crimson Wizard]

I need some advice. I was investigating possible outcomes of using scope convention in practice. Consider following hypothetical situation:

Common/util/string.h

Code: C++

1. namespace AGS
2. {
3. namespace Common
4. {
5. namespace Util
6. {
7.  
8. class CString
9. {
10. ...
11. };
12.  

Engine/main/wootclass.h

Code: C++

1. #include "Common/util/string.h"
2.  
3. namespace AGS
4. {
5. namespace Engine
6. {
7. namespace Main
8. {
9.  
10. class CWoot
11. {
12. public:
13.    CString s;
14. };
15.  

So. Here in this example the code obviously won't compile, because there's no *.cpp file CString is out of scope.
Convention suggests that we don't use "using namespace" directive in header. Instead we may use using-declaration like:

Code: C++

1. ...namespaces...
2. {
3.  
4. using AGS::Common::Util::CString;
5.  
6. class CWoot...
7.  

Will this be ok to add "using" lines like that for every referenced type declared in separate scope in every header we have?
If not, what could be better option?

[RickJ]

I would like to suggest that the conventions also include a file naming convention for editor and engine distributions so that when people post their latest and greatest AGS version the zip or rar files will be named consistently so that the origin and lineage of a specific version can be determined by it's zip file name. 

For example a monkey alpha version of the editor and a beta version of the engine, derived from AGS V2.2.1,  would perhaps look something like the following:

AGSEDIT-V2.2.1-MKY-A00.01.ZIP   
AGSENG-V2.2.1-MKY-B01.01.ZIP

The format isn't really important as long as it contains enough information to identify the distribution and  doesn't break the internet.  So feel free to modify or invent something that serves the above stated purpose.  If we can agree on something it would be beneficial to all.

I would also like to suggest that there be a standard format or convention for release threads similar to how CJ used to do it.  First post to contain synopsis of distribution, change log for various versions, and link to latest version.  Subsequent posts to contain bug reports, support, feature/testing discussions, etc.

Sorry for sort of hijacking the thread as this isn't exactly "Engine Coding Conventions" but it is intimately related and will be decided and used by the same group of people.

Rick

[Crimson Wizard]

The link given in the first post does not work.
Should be: http://www.adventuregamestudio.co.uk/wiki/AGS_Engine_Coding_Conventions

[Crimson Wizard]

This should have been done long long ago, but better later than never .
I must admit that, although I tried, I failed to precisely match this convention because of "false memory" effect, and maybe also simple forgetfullness.

I propose few updates to the convention:
1. Change level of indentation to at least 4 spaces.
I don't want to start tabs vs spaces war again, but it seems that it will make somewhat easier for tab-users if the indentation was bigger. At least one person claimed that 2 spaces made it impossible for his brain to process the code (AFAIK he wanted 8-space tabs).

2. Explicitly mention if and when it is allowed (if it is) to omit curly braces, like if there's only one line after "if".
I had certain confusion regarding this rule, and kept reminding myself to check the convention, but never did. Now I see that there's no such rule at all (unless I occasionally skipped it).

3. Abbreviations in class etc naming. There are two ways to write abbreviations: always caps (GUI), CamelCase (Gui), or depending on some factor. Personally I think there's some reason I like caps more in one case and CamelCase in another, but cannot yet formulate this precisely.
I can tell I prefer to use CamelCase when the name starts with abbreviation, e.g. GuiLabel instead of GUILabel, perhaps because in the second case the abbreviation blends with the first letter of the second word.
I know monkey_05_06 has opposite opinion .

4. Expand "Use of const" paragraph adding the use of const local variables as the way to improve readability and prevent yourself from occasional mistakes (changing precalculated variable that should not be altered later).

5. Explicitly state that it is allowed (if not preffered) to use the existing code style when making minor changes in the code which does not yet uses our convention. The idea is to keep style consistent, so, unless you are rewriting bunch of old functions, you can use the "old" AGS style.
This does not mean you can write "dirty" code ofc.

Maybe there was something else, I will post again if I remember.

UPD: Also, dumb question, but is it that literally anyone registered on forums can edit the AGS Wiki without any extra permissions and such?

[monkey0506]

Yeah, I guess this sort of fell by the wayside, but it does need to be updated. Again, the majority vote should ideally be given sway here. Anything more than 4 spaces of indentation is just bad style though, I don't care what anyone says. My entire programming career has been based around an indentation size of 2 spaces, and that is enough to create a clear distinction.

[Calin Leafshade]

4 spaces the most common now I think.

Most editors have 4 spaces as the default tab width.

[Alan v.Drake]

I vote for 4 spaces.
2 spaces were a trend back then when coders were forced to write cramped code inside 80 columns terminals, nowaday we have bigger monitors and editors that can wrap code automagically. I and my colleagues only use 2 spaces for html and xml, those are the only places where it makes sense.


- Alan

[Crosco]

1. Change level of indentation to at least 4 spaces.

I personally favour 4 spaces and Alan's explanation is fine.
But at the end I think 2 or 4 are the same. It is more important that everyone sticks to the final amount of spaces.

2. Explicitly mention if and when it is allowed (if it is) to omit curly braces, like if there's only one line after "if".

It should NEVER be allowed to omit braces! It's simply to dangerous!
It's a kind of habituation, if you do it always you will never forget to do it.

Also, do you remember Apple's security bug at the beginning of this week? Omitted braces were one reason why the code failed. 

3. Abbreviations in class etc naming. There are two ways to write abbreviations: always caps (GUI), CamelCase (Gui), or depending on some factor.

I like "always caps" because it's an abbreviation (and naturally used in this way). 

4. Expand "Use of const" paragraph adding the use of const local variables as the way to improve readability and prevent yourself from occasional mistakes (changing precalculated variable that should not be altered later).

"Maximum constness" is excellent! 

5. Explicitly state that it is allowed (if not preffered) to use the existing code style when making minor changes in the code which does not yet uses our convention. The idea is to keep style consistent, so, unless you are rewriting bunch of old functions, you can use the "old" AGS style.
This does not mean you can write "dirty" code ofc.

I agree.

[Crimson Wizard]

2. Explicitly mention if and when it is allowed (if it is) to omit curly braces, like if there's only one line after "if".

It should NEVER be allowed to omit braces! It's simply to dangerous!
It's a kind of habituation, if you do it always you will never forget to do it.

I actually disagree with this. If you use identation then it is very difficult to not notice that you are missing a brace, or forget to add ones if you add more lines.
Besides, if you code for a long time you begin to see logical blocks of code in your mind before you type them, so I will always know if I need one line or a block after the condition. This comes so natural, I don't even know how one can not put braces really...

And the fact that this convention suggests putting each brace on its own line means that you will have 3 lines where you could have 1. Considering also that single line after condition is so often a "return", "break" or "continue", or other simple command.
10 years ago I would not bother, but today I am getting more capricious over this.

[Crimson Wizard]

I would like to know, if everyone is okay with using aliases for std::abcdefg<T> STL types?

Something like:

Code: C++

1. typedef std::vector<bool>     BoolArray;
2. typedef std::vector<int>      IntArray;
3. typedef std::vector<char*>    CharPtrArray;
4.  

Reason: easier to type, usually shorter. There may also be similar aliases for iterators and stuff.

Code: C++

1. typedef IntArray::iterator       IntArrIter;
2. typedef IntArray::const_iterator IntArrCIter;
3.  

Does this look good to you? Too weird? Any thoughts?

[Calin Leafshade]

A vectorised list is not an array.

maybe:

Code: C

1. typedef std::vector<bool>     BoolList:
2.  

[Crimson Wizard]

A vectorised list is not an array.

Hmm, unless I am mistaken, in C++ world array is sequential data storage and list is not.

EDIT:

Alternatively, we may use container type as a prefix:

Code: C++

1. typedef std::vector<bool>    VBool;
2. typedef std::vector<int>     VInt;
3. typedef std::list<int>       LInt;
4.  

Even shorter and better grouped, but weirdly looking .

Another:

Code: C++

1. typedef std::vector<bool>   VecBool;
2.  

This way we see it is vector, and keep names grouped alphabetically.