Contributing to SourceMod

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:

• Windows - Microsoft Visual Studio, 8.0 (required)
• Linux, 32bit - GCC, 4.1 (required)

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:

void Gaben() {
	if (gaben) {
	  hello
	} 
	else 
	{
	  ghello
	}
}

Always put the brace on a new line:

void Gaben()
{
	if (gaben)
	{
		hello
	} else {
		ghello
	}
}

OR

void Gaben()
{
	if (gaben)
	{
		hello
	}
	else
	{
		ghello
	}
}

(These are both allowed conventions)

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

if (gaben) hacks
if (gaben)
	hacks
if (gaben) {
	hacks
}
if (gaben) { hacks }

Proper:

if (gaben)
{
	hacks
}

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:

int mynum = (a/b+c)*d-e;

It should be:

int mynum = (((a / b) + c) * d) - e;

Never add extra spaces to parenthetical listings For example, this is silly:

void Function ( a, b, c, d )
{
	Function ( a, b, c, d );
}

It should, of course, be:

void Function(a, b, c, d)
{
	Function(a, b, c, d)
}

The expection to this rule is with complicated inner expressions. For example:

Function(Calc(Reduce(((*iter)->GetFunc())())))

Is impossible to read, and might look better as:

Function(Calc(Reduce(
		((*iter)->GetFunc()) ()
		)));

Don't "crunch" control statements Bad:

if(a)
for(;;)
while(gaben)

Good:

if (a)
for (;;)
while (gaben)

Don't unnecessarily expand iterator expressions Example of unnecessary spacing:

if ( (fp=fopen("file", "rt")) == NULL )
if ( (a + b) )
for (int i = 1; i <= 1; i++)

Read better as:

if ((fp=fopen("file", "rt") == NULL)
if ((a+b) != 0)
for (int i=1; i<=1; i++)

Make conditional statements clear This is unclear, and may have random bugs:

if (a & *b = c + d == t * d+++k || b -= 5)

Better:

if ( ((a & *b) = (c+d)) == (t * (d + ++k))
	|| (b -= 5) != 0 )

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 Mercurial. If you are going to be submitting changes for c++ parts of SourceMod make sure you can compile SourceMod.

Making and submitting a patch

Use the Mercurial command:

hg diff file1.txt file2.txt > outputfile.patch

To generate a patch file for your changes. This can then be attached to the appropriate 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: Blag