Contributing to SourceMod - Revision history

BAILOPAN: Contibuting to SourceMod moved to Contributing to SourceMod

2009-02-19T07:46:58Z

Contibuting to SourceMod moved to Contributing to SourceMod

PRED* at 01:50, 21 September 2008

2008-09-21T01:50:32Z

PRED* at 01:48, 21 September 2008

2008-09-21T01:48:32Z

PRED*: New page: Rambling about why patches are good =Source Code Guidelines= ==Double-Check your commits, and check compilability== Don't commit something because it looks right -- while you don't need t...

2008-09-21T01:45:55Z

New page: Rambling about why patches are good =Source Code Guidelines= ==Double-Check your commits, and check compilability== Don't commit something because it looks right -- while you don't need t...

New page

Rambling about why patches are good

=Source Code Guidelines=
==Double-Check your commits, and check compilability==
Don't commit something because it looks right -- while you don't need to explicitly test every addition, it should at least compile. Breaking the build process for anything on either Windows and Linux causes the build server to send everyone an e-mail.

The builds are:
*<tt>Windows - Microsoft Visual Studio, 8.0 (required)</tt>
*<tt>Linux, 32bit - GCC, 4.1 (required)</tt>
==Submit one patch idea at a time==
Mercurial marks all changes in a set. For example, if you change 30 files, all 30 files will be marked as one revision set. Thus, it is best if you submit one "patch idea" at a time. If you add feature X to one file, submit it as a single patch before you add feature Y to another file, otherwise you risk committing both changes in one revision.
==ALWAYS MIND BACKWARDS COMPATIBILITY==
If you're about to change the functionality of a native, first consider every way a script could be using it. For example, pev() on a string will return the string length. Is it likely any script is actually using this? No. Can we remove this functionality? No.

It may suck, but the instant we break anything, someone will complain that the newer version is worse. All old plugins should work, especially without recompiling. This is called ABI compatibility, and it means that all old SourceMod plugin binaries should work with modification across updates.

Another example: if you are about to add parameters onto a native, you should make the new parameters have default arguments. Furthermore, you must make sure that old binary plugins passing in the old number of parameters will not break (easy, since Pawn tells you how many parameters were passed).

Remember: If it breaks something, don't do it, or else you'll find any number of people hounding you!
==Coding Style==
For SourceMod, we do have loose official coding style. You should use this style within reasonable limits. The most important rules are the ones about brace blocks and camel case, every thing else can be tweaked to your own style.

Always put braces on a new line, except for else cases
The following are bad:

<pre>
void Gaben() {
if (gaben) {
hello
}
else
{
ghello
}
}
</pre>
Always put the brace on a new line:

<pre>
void Gaben()
{
if (gaben)
{
hello
} else {
ghello
}
}
</pre>
OR

<pre>
void Gaben()
{
if (gaben)
{
hello
}
else
{
ghello
}
}
</pre>
(These are both allowed conventions)

Always use braces for block statements (no one liners!)
The following are bad:

<pre>
if (gaben) hacks
if (gaben)
hacks
if (gaben) {
hacks
}
if (gaben) { hacks }
</pre>
Proper:

<pre>
if (gaben)
{
hacks
}
</pre>
This adds more spaces but improves readability. It also makes it less annoying to add more into the block later.

Use Camel Case for API
No questions asked, use CamelCase! SourceMod does everything in a "Verb, Noun" or "Subject, Verb, Noun" form. I.e. SQL_Connect, or GetClientName. Some natives have the verb implied. For example, "StrEqual" is implicitly "IsStrEqual." This practice is kind of cheap and you should try to deprecate usage of it.

Space/parenthesize operators in math functions:
The following are not very readable:

<pre>
int mynum = (a/b+c)*d-e;
</pre>
It should be:

<pre>
int mynum = (((a / b) + c) * d) - e;
</pre>
Never add extra spaces to parenthetical listings
For example, this is silly:

<pre>
void Function ( a, b, c, d )
{
Function ( a, b, c, d );
}
</pre>
It should, of course, be:

<pre>
void Function(a, b, c, d)
{
Function(a, b, c, d)
}
</pre>
The expection to this rule is with complicated inner expressions. For example:

<pre>
Function(Calc(Reduce(((*iter)->GetFunc())())))
</pre>
Is impossible to read, and might look better as:

<pre>
Function(Calc(Reduce(
((*iter)->GetFunc()) ()
)));
</pre>
Don't "crunch" control statements
Bad:

<pre>
if(a)
for(;;)
while(gaben)
</pre>
Good:

<pre>
if (a)
for (;;)
while (gaben)
</pre>
Don't unnecessarily expand iterator expressions
Example of unnecessary spacing:

<pre>
if ( (fp=fopen("file", "rt")) == NULL )
if ( (a + b) )
for (int i = 1; i <= 1; i++)
</pre>
Read better as:

<pre>
if ((fp=fopen("file", "rt") == NULL)
if ((a+b) != 0)
for (int i=1; i<=1; i++)
</pre>
Make conditional statements clear
This is unclear, and may have random bugs:

<pre>
if (a & *b = c + d == t * d+++k || b -= 5)
</pre>
Better:

<pre>
if ( ((a & *b) = (c+d)) == (t * (d + ++k))
|| (b -= 5) != 0 )
</pre>
Use #if 0 to disable code, not /* */
Block comments are annoying, and tend to be messy when they're nested. Use #if 0 / #endif to disable code.

Do not use Hungarian notation.
Use g_, m_, b, and f where appropriate. If you find yourself using variable names like m_pszString, you probably need to stop.

=Steps=

==Obtaining the Source Code==
Download the latest copy of the SourceMod source code using [http://wiki.alliedmods.net/Mercurial_Tutorial Mercurial]. If you are going to be submitting changes for c++ parts of SourceMod make sure you can [http://wiki.alliedmods.net/Compiling_SourceMod compile SourceMod].
==Making and submitting a patch==
Use the Mercurial command:

<pre>hg diff file1.txt file2.txt > outputfile.patch</pre>

To generate a patch file for your changes. This can then be attached to the appropriate [https://bugs.alliedmods.net/ Bugzilla] bug report using the Add an attachment link (don't forget to tick the patch box).
==Why hasn't my patch been committed?==
See Bailopan's blog regarding patch submitting guidelines: [http://www.bailopan.net/blog/?p=9 Blag]

← Older revision	Revision as of 07:46, 19 February 2009
(No difference)

@@ Line 90: / Line 90: @@
 No questions asked, use CamelCase! SourceMod does everything in a "Verb, Noun" or "Subject, Verb, Noun" form. I.e. SQL_Connect, or GetClientName. Some natives have the verb implied. For example, "StrEqual" is implicitly "IsStrEqual." This practice is kind of cheap and you should try to deprecate usage of it.
-Space/parenthesize operators in math functions:
+==Space/parenthesize operators in math functions==
 The following are not very readable:
@@ Line 101: / Line 101: @@
 int mynum = (((a / b) + c) * d) - e;
 </pre>
-Never add extra spaces to parenthetical listings
+==Never add extra spaces to parenthetical listings==
 For example, this is silly:
@@ Line 130: / Line 130: @@
 		)));
 </pre>
-Don't "crunch" control statements
+==Don't "crunch" control statements==
 Bad:
@@ Line 145: / Line 145: @@
 while (gaben)
 </pre>
-Don't unnecessarily expand iterator expressions
+==Don't unnecessarily expand iterator expressions==
 Example of unnecessary spacing:
@@ Line 160: / Line 160: @@
 for (int i=1; i<=1; i++)
 </pre>
-Make conditional statements clear
+==Make conditional statements clear==
 This is unclear, and may have random bugs:
@@ Line 172: / Line 172: @@
 	|| (b -= 5) != 0 )
 </pre>
-Use #if 0 to disable code, not /* */
+==Use #if 0 to disable code, not /* */==
 Block comments are annoying, and tend to be messy when they're nested. Use #if 0 / #endif to disable code.

@@ Line 87: / Line 87: @@
 This adds more spaces but improves readability. It also makes it less annoying to add more into the block later.
-Use Camel Case for API
+==Use Camel Case for API==
 No questions asked, use CamelCase! SourceMod does everything in a "Verb, Noun" or "Subject, Verb, Noun" form. I.e. SQL_Connect, or GetClientName. Some natives have the verb implied. For example, "StrEqual" is implicitly "IsStrEqual." This practice is kind of cheap and you should try to deprecate usage of it.
@@ Line 175: / Line 175: @@
 Block comments are annoying, and tend to be messy when they're nested. Use #if 0 / #endif to disable code.
-Do not use Hungarian notation.
+==Do not use Hungarian notation==
 Use g_, m_, b, and f where appropriate. If you find yourself using variable names like m_pszString, you probably need to stop.