Code Standards - BeeStation/BeeStation-Hornet GitHub Wiki
As mentioned before, you are expected to follow these standards in order to make everyone's lives easier. It'll save both your time and ours, by making sure you don't have to make any changes and we don't have to ask you to. Thank you for reading this section!
An absolute path is one defined in a single line:
/datum/proc/test()
A relative path is defined using indents:
/datum
proc
test()
A datum is Byond's object type. Datums include subtypes of /datum
, as well as any subtype of /atom
.
A typepath is the name given to the bit of code that defines a type, for example: /datum/foo/bar
defines the bar type as a subtype of /datum/foo
. /datum/foo/bar
is the full typepath.
Modular code is any code which can be reasonably self-contained. It does not need to be directly integrated with the entire codebase and can be fully defined inside of a single folder which can be removed or added by simply removing or adding that folder. Modular code is more maintainable as it can be easilly removed or ported to another codebase.
- Avoid using
istype
where there are better alternatives: - If a subtype needs to run different code from its parent type, override the proc.
- If a subtype needs to use a different variable value, use an overriden variable.
- If either of these need to be done using modular code, use traits.
// Prefer:
/mob/living/proc/action()
// Base action
/mob/living/carbon/human/action()
// Human mob action
// Over:
/mob/living/proc/action()
if (istype(src, /mob/living/carbon/human))
// Human mob action
return
// Base action
- Code that is copied from one place to another is not allowed.
- Seperate the code that needs to be copied into its own proc, or use any other object oriented approach.
- Avoid defining variables that will only be used in a single location to identify certain types.
- Use traits when behaviours should be shared without a clear and obvious base type.
- Declare a variable and override it when it is obvious how that variable relates to the base type.
- Note that this contradicts the object-oriented over type-checking rule, you will need to use judgement as to when one approach is better than the other. A modular approach is better when the following conditions are met:
- The object-oriented approach would require creating specific variables and procs on a highly generic type (such as
/mob
). - The variable/proc that would be created is not going to be widely used across the codebase by multiple different modules.
- There are only a small number types that need checking (1-3).
- The code is unlikely to be expanded upon in the future.
- The object-oriented approach would require creating specific variables and procs on a highly generic type (such as
// Example of an item that needs to check for types
/obj/item/pet_detector
/obj/item/pet_detector/proc/check_mob(mob/target)
return istype(target, /mob/cat) || istype(target, /mob/dog)
// Example of using traits instead of a widespread atom/var/marked
/obj/item/marker/proc/mark(atom/target)
if (HAS_TRAIT(target, TRAIT_MARKED))
return FALSE
ADD_TRAIT(target, TRAIT_MARKED, SOURCE_MARKER)
return TRUE
- Do not use
/atom/New()
, use/atom/Initialize(mapload)
instead. - Initialize is called once all atoms are created during mapload, which prevents issues.
- The parameters passed into
new object()
are passed into initialize from the 2nd parameter onwards. - The first parameter inside of
Initialize
indicates whether or not the atom was loaded as part of a map, or throughnew()
at runtime.
- Code may be considered bloat if it will have a high maintenance cost.
- Code with a high maintenance cost should be made modular, so that it is easier to maintain.
- Do not convert strings into typepaths. This will lead to compiler errors when code is updated, and security issues if used with player input.
- User input is any data recieved from the user, including from using tgui_input, hrefs, and params/action inside of ui_act.
- Always check that a user has permission to execute a href call.
- Always check that a user has the correct permissions before performing an administrative action.
- If user input is not expected to contain HTML tags, it must use
sanitize
to clear any HTML tags. - Sanitization should be performed before the user input is stored inside a variable or passed as an argument to a function.
- Numbers should be validated using
text2num
and checking their range. - If a user input value is expected to be a value of a list, check to see if that user input belongs to the list.
- Never use
locate()
to find something in the world. Always search for it in the same list you sent to the client.
- User input inside an SQL statement must be espaced using SQL variables.
- SQL inserts must specify the columns being inserted.
- All changes to the database's schema must be specified in the database changelog using SQL, on top of changes to the schema files.
- The database update SQL should perform no action if the modification is already present.
- Any time the schema is changed the
schema_revision
table andDB_MAJOR_VERSION
orDB_MINOR_VERSION
defines must be incremented. - Queries must never specify the database, be it in code, or in text files in the repo.
- Do not change the primary key of a row or entity, including when converting or moving data.
- File names should consist only of lowercase alphabetic character, periods, and underscores.
- Folder names should only consist of lowercase alphabetic character and underscores.
- Any code that should not be touched by administrators should be protected by checking for
IsAdvancedAdminProcCall()
- Any config settings should be appropriately protected.
- And secure variables should be protected by overriding
can_vv_get
.
- Asynchronous code is any code which pauses a proc's execution until it completes. An example of this is tgui_input, which displays an input box and pauses until the user submits a value.
- Any pre-conditions must be re-validated after every single call to a proc that sleeps. For example, if you have an item that requires the player is adjacent to another player in order to send a message to them, once the player types in their message, the code must re-check that the player is still adjacent to their target.
- Procs using
set waitfor = FALSE
must not return a value, unless returning using the.
operator prior to sleeping.
- Read https://wiki.beestation13.com/view/Guide_to_mapping
- Any time a typepath is completely removed, an update path must be provided.
/obj/item : @DELETE
- Any time a typepath is renamed, an update path must be provided.
/obj/item/old_path: /obj/item/new_path{@OLD}
- Any changes that convert one type to another must provide an update path file.
/obj/item/@SUBTYPES: /obj/item2/@SUBTYPES{@OLD}
- If a variable is likely to be changed by a mapper, enforce it using a maplint.
- Check mapping pre-conditions using maplints (such as no duplicate items on the same tile).
- Update paths should be named
[PR number]_[Title].txt
. Use the number 99999 as a placeholder when you have no PR. - Maps must use the TGM format.
- Map merge must be run to resolve mapping conflicts.
- Mapping should be done using StrongDMM.
- Avoid var-edits, create a new typepath in code (excluding pixel_x and pixel_y).
- Use directional mapping helpers rather than setting dir, pixel_x and pixel_y.
These rules enforce code style. They have no effect on the compiled code, but are needed so that other developers can easilly process what you are writing.
- Use tab indentation and not space indentation.
- Avoid vertically aligning multiple lines using spaces, as this is hard to maintain.
// Prefer:
#define something 5
#define something_else 6
#define test 7
// Over:
#define something 5
#define something_else 6
#define test 7
- Typepaths must be defined using absolute pathing.
- The base implementation of a datum is the primary definition for that datum.
- Variable declarations must be defined using relative pathing at the base implementation of a datum.
- Variable declarations must be defined using absolute pathing when extending a datum's base implementation.
- Proc declarations must be defined using absolute pathing.
// Foo.dm
// Define the base implementation for the foo type.
/datum/foo
var/a
var/list/b
var/datum/c
var/d
/datum/foo/proc/do_something()
return
// Foo_extension.dm
// Add the E variable to the foo type modularly.
/datum/foo/var/e
- Type path declarations must begin with a
/
:/datum/test
- Type paths must be lowercase.
- Type declarations that extend from
/datum
must begin with/datum
:/datum/test
instead of/test
. - When declaring a variable for the first time, it must begin with
var/
. - Overriden variables, such as on subtypes, should not begin with
var/
. - Always use appropriate proc and variable names. These should be descriptive but not too long.
-
for(var/i in 1 to 10)
is accepted as standard programming convention forfor
loops that iterate between 2 numbers. j and k can be used for embedded for loops. - Single capital letter variable names are not permitted, even if they make sense for the path:
/mob/living/carbon/human/H
is not allowed.
-
- Proc parameters should not start with
var/
/datum
var/a = 1
var/b = 4
/datum/test
b = 3
var/c = 7
/datum/proc/example(first, datum/second, list/third)
return
- Use of the colon operator for variable access is not allowed:
a.b
instead ofa:b
.
- Inside of a file, all types must be grouped together.
- All lines must be a readable length.
- List declarations should be broken up into multiple lines.
- Procs with large amounts of parameters should be split across multiple lines.
- Avoid extremely long files. If a file contains more than 300 lines, then it should be split up.
// Prefer:
/datum/proc/example_1()
/datum/proc/example_2()
/datum/bar/proc/example()
// Over:
/datum/proc/example_1()
/datum/bar/proc/example()
/datum/proc/example_2()
// Prefer
var/list/example = list(
"first",
"second"
)
// Over
var/list/example = list("first", "second")
- A magic number/string is an arbritrary comparison where it would break if one of the numbers were to be changed.
- When comparing against arbritrary values that have no meaning, such as to identify stages, use a define to give those stages a name.
- Do not define a name for typepaths, as these are identifiers in of themselves.
// Prefer:
#define STAGE_1 1
#define STAGE_4 4
/proc/check_needs_screwdriver(construction_step)
return construction_step== STAGE_1 || construction_step== STAGE_4
// Over:
/proc/check_needs_screwdriver(value)
return value == 1 || value == 4
- Any comparison against a constant value should use the form of
thing operator number
:count <= 10
rather than10 > count
. - Control statements (while, if, for, etc.) must contain their code block on a new line.
// Prefer:
if (test)
return
// Over:
if (test) return
- Avoid writing code that travels several indentation levels deep.
- If you need to check multiple conditions, use an early return.
// Prefer:
/datum/datum1/proc/proc1()
if (!thing1)
return
if (thing2)
return
if (thing3 != 30)
return
do_stuff()
// Over:
/datum/datum1/proc/proc1()
if (thing1)
if (!thing2)
if (thing3 == 30)
do_stuff()
Maintainers are not allowed to strictly enforce this section, and you should avoid changing these things where not necessary as they may result in merge conflicts:
- Preference towards
if ()
overif()
- No preference towards british vs american spelling of words.
- Wrap bitwise operators in brackets to explicitly show how you intend for it to be executed:
((bitflag | 15) & 5) == 5
. - Bitwise comparisons should always have the bitfield on the left hand side of the operator:
bitflag & 5
.
- Assoc lists must use strings for their keys:
list("a" = 5)
- Always use
length(list)
overlist.len
- For loops that iterate over a range, must use
for (var/i in 1 to 5)
syntax, over C style loops.
- Always use
static
instead ofglobal
.
- Use the
span_
procs when you want text to be wrapped in a span:span_warning("You feel a warning!")
. - Complex spans in a text do not need to use
span_
procs:"<span class='deadmessage'><span class='deadtitle'>You have died!</span><br/>You will respawn in 5 minutes!</span>"
.
- When using process(), you must respond to delta_time by multiplying any variables by the delta_time value.
- Developers are responsible for fixing their bugs.
- Unfixed bugs will result in a reversion.
- Developers who do not frequently create fixes will not be allowed to make PRs that cause a high review workload, such as balance PRs.
- If you update code on mass using a regex, post the regex in your PR so downstreams and porters can use it.
- You must provide full credit and the license of anything you did not make yourself.