- General Guidelines
- License
- Workflow Guide
- Issue Report Contributions
- Pull Request Contributions
- Style Guide
- By contributing to Topaz, either through issues or pull requests, you are expected to abide by the rules laid out here in this Contributing Guide.
- We do not support out-of-date clients or client modification.
- We do not support piracy of any kind. We encourage you to maintain an active retail subscription and support the game.
- We operate under GNU General Public License v3.0. We do not accept contributions that use other more restrictive licenses (such as AGPL3).
- It is always better to come into Discord and ask a question instead of investing a lot of time in work that we're going to ask your to rewrite or split up.
- Cite your sources. This can be comments in your code, or your commit messages. Pull Request descriptions and comments will get lost over time.
- If you're commiting work on someone else's behalf, use git's
--authorargument so they get the credit they deserve. - Make your commit messages meaningful, or amend/rebase once you're ready to push.
- If an issue involves incorrect NPCs or text, please include your client and server versions (type
/verand!verin game) - Unimplemented feature requests must be retail behavior, and adequetly cover everything about that feature which is missing.
- Fill out the templated checkboxes that are preloaded in the issue body. These allow us to diagnose your issue as efficiently as possible, and confirm that you've searched for duplicate issues or recent fixes.
All contributions must be done through pull requests to the Topaz repository. We don't take fixes from Discord to apply ourselves. If you need help with making a pull request, there is a GitHub guide on how to do so. If you still need help after consulting the guide, you can ask for help in Discord and we will be happy to help you.
We prefer submitting early and often, over monolithic and once. If you're implementing a complex feature, please try to submit PRs as you get each smaller functional aspect working (use your best judgment on what counts as a useful PR). This way we can help make sure you're on the right track before you sink a lot of time into implementations we might want done in a different way.
Please try to leave your PR alone after submission, unless it's to fix bugs you've noticed, or if we've requested changes. If you're still pushing commits after opening the PR, it makes it hard for reviewers to know when you're "finished" and if it's "safe" to begin their reviews. If you do want to push early for reviews of your in-progress work, you can open your PR as a "draft".
After a pull request is made, if a staff member leaves feedback for you to change, you must either fix or address it for your pull request to be merged.
If you do not fill the checkboxes confirming that you've read the supporting documentation, and that you've tested your code - your PR will not be reviewed.
Much of this can be automated.
We highly recommend editorconfig, which most code editors have either a plugin or native support for.
- Visual Studio Plugin
- Notepad++
- As the plugin manager is usually installed by default*, the easy way is to use that:
Launch Notepad++, click on the
Pluginsmenu, thenPlugin Manager->Show Plugin Manager. In theAvailabletab, findEditorConfigin the list, check the checkbox and click on theInstallbutton.- *64bit may require manual installation.
- As the plugin manager is usually installed by default*, the easy way is to use that:
Launch Notepad++, click on the
- Sublime: Install EditorConfig with Package Control and restart Sublime.
- Vim
Clang-Format is also an option for C++
- Your code should strive to be obvious and readable by the casual observer. You aren't going to be the only person who reads/debugs your code.
- Unix (LF) line ends at the end of every file. GitHub will tell you if you don't have one by putting a ⛔ symbol at the end of your file.
- Try not to exceed 120 chars width. Exceptions will occur, but try.
- 4 space indent, do not use tabs for alignment.
- Trim trailing whitespace.
- Space after starting comments (
-- Commentand// Comment) - If you agree with staff and/or your reviewers that some work in your pull request can be left as "to-do", make new issues on GitHub for your new
TODOitems and put the ID alongside the comment. The comment should also be sufficiently descriptive of what the missing work is, and why it was left out. eg.// TODO: A Boy's Dream - PLD AF Quest 2 - Cannot be completed until fishing is implemented (GitHub Issue #12345)
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
includeandusingstatements 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())
{
}cppcoreguidelines-pro-type-cstyle-cast
// Correct ✔️
uint32 param = static_cast<uint32>(input);
// Wrong ❌
uint32 param = (uint32)input;cppcoreguidelines-pro-type-static-cast-downcast
// 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, here is an example lambda. If you're using them (lambdas, or tools), do your best!
// Correct ✔️
auto isEntityAlive = [&](CBigEntity* entity) -> bool
{
return entity->isAlive;
};
// Correct ✔️
static_cast<CCharEntity>(PPlayer)->ForParty([&](CBattleEntity* entity)
{
entity->doStuff();
});
// Wrong ❌
auto isEntityAlive =
[&](CBigEntity* entity)
{
return entity->isAlive;
};
// Formatting Emergencies ❓
// clang-format off
auto isEntityAlive = [&](CBigEntity* entity) -> bool
{
return entity->isAlive;
};
// clang-format on- The STL is your friend, don't be afraid to use it.
- Be careful with
auto, it can mask important type details. - Be as
constas you reasonably can. UpperCamelCasefor namespaced functions and classes.UPPER_SNAKE_CASEfor enums (exception for enum classes: style as classes).lowerCamelCasefor everything else.
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:
-- Correct ✔️
local var = 0
-- Wrong ❌
var = 0-- Correct ✔️
local table =
{
content = 1,
}
-- Wrong ❌
local table = {
content = 1,
}- 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()
endnotbefore,and/orafter
-- 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()
endTHIS IS THE ONE EXCEPTION TO THE GLOBAL NEWLINE-BRACE RULES
-- Correct ✔️
xi.func({
entry = 1,
entry = 2,
})
-- Wrong ❌
xi.func(
{
entry = 1,
entry = 2,
}
)- Our lua functions and members are typically
lowerCamelCased, with few exceptions. - Make sure you check out
scripts/globals/npc_util.luafor 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-
Don't put single quotes around non string fields:
42,0not:
'42','0' -
No line breaks in the middle of a statement:
INSERT INTO table_name (a,b,c,x,y,z);not:
INSERT INTO table_name (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_namenot:
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.
Example
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) seashellConversely, 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.
- Going forward schema changes should be accompanied by a migration script.
Python is primarily used for support scripts.
- Python uses
lower_snake_case