-
Notifications
You must be signed in to change notification settings - Fork 611
Development Guide
It is assumed you've read the Contributing Guide.
- The STL is your friend, don't be afraid to use it.
- Be careful with
auto
, it can mask important type details. - Be as
const
as you reasonably can. -
UpperCamelCase
for namespaced functions and classes. -
UPPER_SNAKE_CASE
for enums (exception for enum classes: style as classes). -
lowerCamelCase
for everything else. - You should use exceptions only in exceptional circumstances:
- If player/server data is at risk of being lost or corrupted, this is probably a good time to throw an exception and take down the server.
- If you're encountering a situation that shouldn't ever be happening, best to kill it with an exception.
- (Begrudgingly) If the STL, a third party lib, or legacy code relies on throwing exceptions as part of regular operation (LuaJIT does this) - wrap them in try/catch and move on with your life.
We keep a .clang-format
file in the root of the repo, but accept it can be difficult to set up for use on just your changes, as opposed to entire files that you're working with that might have legacy styling you don't want to mess with.
Here are the points from .clang-format
explained:
When in doubt, defaulting to WebKit style with Allman braces
is seemingly a safe option.
// Correct ✔️
class Classname
{
public:
Classname();
private:
int member;
}
// Wrong ❌
class Classname
{
public:
Classname();
private:
int member;
}
// Correct ✔️
void f()
{
foo();
}
// Wrong ❌
void f() { foo(); }
Braces should almost always be on a new line.
// Correct ✔️
if (x == 5)
{
function();
}
// Wrong ❌
if (x == 5) {
function();
}
// Correct ✔️
Constructor(int param0, int param1)
: member0(param0)
, member1(param1)
{
}
// Wrong ❌
Constructor(int param0, int param1)
: member0(param0), member1(param1){}
// Correct ✔️
namespace Foo
{
namespace Bar
{
}
}
// Wrong ❌
namespace Foo { namespace Bar {
}}
// Correct ✔️
std::vector<int> x{ 1, 2, 3, 4 };
// Wrong ❌
std::vector<int> x{1, 2, 3, 4};
// Correct ✔️
switch(x)
{
case 0:
{
break;
}
}
// Wrong ❌
switch(x)
{
case 0:
{
break;
}
}
-
Note: It doesn't matter if your
break;
is inside or outside the body of your case statement - as long as it's there (if you indend it to be).
// Correct ✔️
if (func())
{
reaction();
}
// Wrong ❌
if (func())
{
reaction();
}
// Correct ✔️
void function(int x)
{
doSomething(x);
}
// Wrong ❌
void function(int x)
{
doSomething(x);
}
Yup.
// Correct ✔️
void function(CBigType* type);
void function(CBigType& type);
// Wrong ❌
void function(CBigType *type);
void function(CBigType &type);
// Wrong ❌
void function(CBigType * type);
void function(CBigType & type);
- Try to keep your
include
andusing
statements organised alphabetically, in logical blocks.
// Correct ✔️
if (true)
{
doThing();
}
// Wrong ❌
if(true)
{
doThing();
}
// Correct ✔️
A<A<int>>
// Wrong ❌
A<A<int> >
// Correct ✔️
<4 spaces>
// Wrong ❌
<a tab>
// Wrong ❌
<a half-tab>
readability-braces-around-statements
// Correct ✔️
if (x == 5)
{
function();
}
// Wrong ❌
if (x == 5)
function();
// Wrong ❌
if (x == 5)
function(21);
else
{
function(42);
}
// Correct ✔️
if (thisThing())
{
}
else
{
}
if (thatThing())
{
}
// Correct ✔️
if (thisThing())
{
}
if (thatThing())
{
}
// Wrong ❌
if (thisThing())
{
}
else
{
}
if (thatThing())
{
}
// Wrong ❌
if (thisThing())
{
}
if (thatThing())
{
}
- There is no hard rule about how wide is too wide at this time.
- Use your best judgement, but nobody likes left/right scrolling.
// Correct ✔️
if (lotsOfStuff &&
evenMoreStuff &&
evenMoreStuff &&
evenMoreStuff &&
evenMoreStuff &&
evenMoreStuff)
{
}
// Wrong ❌
if (lotsOfStuff
&& evenMoreStuff
&& evenMoreStuff
&& evenMoreStuff
&& evenMoreStuff
&& evenMoreStuff)
{
}
// Wrong ❌ (this is 102 chars wide)
if (lotsOfStuff && evenMoreStuff && evenMoreStuff && evenMoreStuff && evenMoreStuff && evenMoreStuff)
{
}
cppcoreguidelines-pro-type-cstyle-cast
// Correct ✔️
uint32 param = static_cast<uint32>(input);
// Wrong ❌
uint32 param = (uint32)input;
cppcoreguidelines-pro-type-static-cast-downcast
NOTE: There is a good reason for this. If you use static_cast
and it fails, the returned pointer will NOT NECESSARILY be nullptr
and any blocks checking against for nullptr
might incorrectly succeed. dynamic_cast
will return a nullptr
if it fails.
// Correct ✔️
if (auto PChar = dynamic_cast<CCharEntity*>(baseEntity))
{
PChar->DoSomething()
}
// Wrong ❌
auto PChar = static_cast<CCharEntity*>(baseEntity)
PChar->DoSomething()
// Wrong ❌
if (auto PChar = static_cast<CCharEntity*>(baseEntity))
{
// The cast is forced, so PChar will _always_ be populated here....
PChar->DoSomething()
}
// Correct ✔️
auto f(0, 1, 2, 3, 4, 5, 6);
// Wrong ❌
auto f(0,1,2,3,4,5,6);
Formatting tools have a famously difficult time with lamdas, so we tend to wrap them in // clang-format on/off
to ensure we can format them how we'd like.
// Correct ✔️
// clang-format off
auto isEntityAlive = [&](CBigEntity* entity) -> bool
{
return entity->isAlive;
};
// clang-format on
// Correct ✔️
// clang-format off
static_cast<CCharEntity>(PPlayer)->ForParty([&](CBattleEntity* entity)
{
entity->doStuff();
});
// clang-format off
// Wrong ❌
auto isEntityAlive =
[&](CBigEntity* entity)
{
return entity->isAlive;
};
A lot of the styling rules from the C++ guide can and should be applied to Lua code. Here are the important points to remember when styling your Lua:
- Our lua functions and members are typically
lowerCamelCased
, with few exceptions. - Make sure you check out
scripts/globals/npc_util.lua
for useful tools and helpers. - If you're going to cache a long table entry into a var with a shorter name, make sure that name still conveys the original meaning.
-- Correct ✔️
local copCurrentMission = player:getCurrentMission(xi.mission.log_id.COP)
local copMissionStatus = player:getCharVar("PromathiaStatus")
local sandyQuests = xi.quest.id.sandoria
-- Wrong ❌
local currentMission = player:getCurrentMission(xi.mission.log_id.COP)
local missionStatus = player:getCharVar("PromathiaStatus")
local quests = xi.quest.id.sandoria
-- Correct ✔️
local var = 0
-- Wrong ❌
var = 0
-- Correct (Allman style for multi-line tables) ✔️
local table =
{
one = 1,
two = 2,
}
-- Correct (Oneliner style for small, single-line tables) ✔️
local table = { 1, 2, 3, 4 }
-- Wrong ❌
local table = {
content = 1,
}
-- Wrong ❌
local table =
{ one = 1,
two = 2,
}
NOTE: The final entry in a multi-line table should have a comma after it.
-- Correct ✔️
if condition1 == 1 then
trigger()
end
-- Correct ✔️
if condition1 and (condition2 or condition3) then
trigger()
end
-- Wrong ❌
if (condition1 == 1) then
trigger()
end
-- Correct ✔️
local x = 42
trigger(42)
-- Wrong ❌
local x = 42;
trigger(42);
-- Wrong ❌
local x = 42; trigger(42);
-- Short - Correct ✔️
if condition then
bla()
end
-- Long or many multiple conditions - Correct ✔️
if
condition1 and
condition2 or
not condition3
then
bla()
end
-- Wrong ❌
if condition1 then
bla()
end
-- Wrong ❌
if condition1 and condition2 and
not condition3 then
bla()
end
-- Wrong ❌
if condition1 and condition2 and
not condition3
then
bla()
end
-
not
before,and/or
after
-- Correct ✔️
if
condition1 and
condition2 or
not condition3
then
bla()
end
-- Wrong ❌
if
condition1
and condition2
or not condition3
then
bla()
end
-- Correct ✔️
if condition1 and (condition2 or condition3) then
trigger()
end
-- Wrong ❌
if condition1 and ( condition2 or condition3 ) then
trigger()
end
-- Wrong ❌
if
condition1 and
( condition2 or condition3 )
then
trigger()
end
-- Correct ✔️
local function myFunction(param1, param2, param3)
end
-- Wrong ❌
local function myFunction (param1,param2,param3)
end
-- Correct ✔️
if a == b then
a = b + c
end
-- Wrong ❌
if a==b then
a = b+c
end
-- Correct ✔️
if a == b then
a = b + c
end
return a
-- Wrong ❌
if a == b then
a = b + c
end
return a
-- Correct ✔️
local function myFunction(param1, param2, param3)
return param1 + param2
end
-- Wrong ❌
local function myFunction(param1, param2, param3)
return param1 + param2
end
local function myFunction(param1, param2, param3)
return param1 + param2
end
-- Correct ✔️
if a == b then
a = b + c
end
local function myFunction(param1, param2, param3)
return param1 + param2
end
-- Wrong ❌
if a == b then a = b + c end
local function myFunction(param1, param2, param3) return param1 + param2 end
-- Correct ✔️
xi.func({
entry = 1,
entry = 2,
})
-- Wrong ❌
xi.func(
{
entry = 1,
entry = 2,
}
)
-
Don't put single quotes around non string fields:
42,0
not:
'42','0'
-
No line breaks in the middle of a statement:
INSERT INTO table_name VALUES (a,b,c,x,y,z);
not:
INSERT INTO table_name VALUES (a,b,c, x,y,z);
-
Spaces in names get replaced with an underscore. Hyphens are allowed. Most other symbols are removed from item/mob/npc names except for polutils_name or packet_name columns, where they must be escaped.
-
Full lower case skill/spell/pet/ability things.
-
Don't change SQL keywords to lowercase:
INSERT INTO table_name
not:
insert into table_name
Our SQL tables are big and confusing, and they are also modified by hand. It can be very helpful to leave short comments on your additions and modifications to highlight what they are.
Without a comment, this entry is not easily human-readable:
INSERT INTO `mob_droplist` VALUES (504,0,0,1000,888,340);
So we instead store it as:
INSERT INTO `mob_droplist` VALUES (504,0,0,1000,888,340); -- (Colossal Calamari) seashell
Conversely, Combo
weaponskill doesn't need any additional comments because it has a name field:
INSERT INTO `weapon_skills` VALUES (1,'combo',0x02020000000200000000000002000000000202000000,1,5,0,16,2000,5,1,8,0,0,0,0);
The format of the comment isn't massively important, but it is preferred not to use ';' as a seperator in the middle of your comment. This is a little confusing, as it's the statement-terminator in SQL.
In general, it is preferred to comment out the entire row rather than only leaving a comment about still needing to capture it. Examples include item and NPC models. Use your best judgment and if you aren't sure you can always ask.
- If it "works" there is an unfortunate chance nobody will come back to finish it later, including the original author. Life happens, people come and go, take breaks etc.
- If its "wrong" we're likely to still get issue reports on it because of that partial implementation, more so than if it were completely missing.
- Experience has shown people don't often read those comments before complaining that "we" got it wrong.
- By having the partial data but commenting it out, it remains obviously incomplete and the next editor can more easily see what needs to happen.
- Example:
-- Missing model, looks like a fish. Just kidding this doesn't exist and is only a made up example
-- INSERT INTO `item_equipment` VALUES (65534,'holy_moly_mace',43,0,1048645,???,0,0,3,0,0);
And absolutely never substitute item modifiers!
- Going forward schema changes should be accompanied by a migration script.
Python is primarily used for support scripts.
- Python uses
lower_snake_case