Skip to content

Latest commit

 

History

History
820 lines (611 loc) · 16.2 KB

Development-Guide.md

File metadata and controls

820 lines (611 loc) · 16.2 KB

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 and using 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;
    };

Back to Top

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,
    }
)

Back to Top

  • 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.

Back to Top

Python is primarily used for support scripts.

  • Python uses lower_snake_case

Back to Top