Code Standards

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!

Code Maintaining

Any contributors who we feel like are strictly making feature or balance changes without any PRs that help improve codebase maintainability may be asked to create some fixes first, failing this a maintainer may freely close any feature PRs if the author doesn't compensate the codebase with fixes.

In addition to this, balance PRs from authors that have no prior PRs that improve the maintainability of the codebase (fixes/refactors) may be freely closed by a maintainer.

Object Oriented Code

As BYOND's Dream Maker (henceforth "DM") is an object-oriented language, code must be object-oriented when possible in order to be more flexible when adding content to it. If you don't know what "object-oriented" means, we highly recommend you do some light research to grasp the basics.

All BYOND paths must contain the full path

(i.e. absolute pathing)

DM will allow you nest almost any type keyword into a block, such as:

datum
	datum1
		var
			varname1 = 1
			varname2
			static
				varname3
				varname4
		proc
			proc1()
				code
			proc2()
				code

		datum2
			varname1 = 0
			proc
				proc3()
					code
			proc2()
				..()
				code

The use of this is not allowed in this project as it makes finding definitions via full text searching next to impossible. The only exception is the variables of an object may be nested to the object, but must not nest further.

The previous code made compliant:

/datum/datum1
	var/varname1
	var/varname2
	var/static/varname3
	var/static/varname4

/datum/datum1/proc/proc1()
	code
/datum/datum1/proc/proc2()
	code
/datum/datum1/datum2
	varname1 = 0
/datum/datum1/datum2/proc/proc3()
	code
/datum/datum1/datum2/proc2()
	..()
	code

No overriding type safety checks

The use of the : operator to override type safety checks is not allowed. You must cast the variable to the proper type.

Type paths must begin with a /

eg: /datum/thing, not datum/thing

Type paths must be lowercase

eg: /datum/thing/blue, not datum/thing/BLUE or datum/thing/Blue

Datum type paths must began with "datum"

In DM, this is optional, but omitting it makes finding definitions harder.

Do not use text/string based type paths

It is rarely allowed to put type paths in a text format, as there are no compile errors if the type path no longer exists. Here is an example:

//Good
var/path_type = /obj/item/baseball_bat

//Bad
var/path_type = "/obj/item/baseball_bat"

Use var/name format when declaring variables

While DM allows other ways of declaring variables, this one should be used for consistency.

Tabs, not spaces

You must use tabs to indent your code, NOT SPACES.

(You may use spaces to align something, but you should tab to the block level first, then add the remaining spaces)

No hacky code

Hacky code, such as adding specific checks, is highly discouraged and only allowed when there is no other option. (Protip: 'I couldn't immediately think of a proper way so thus there must be no other option' is not gonna cut it here! If you can't think of anything else, say that outright and admit that you need help with it. Maintainers exist for exactly that reason.)

You can avoid hacky code by using object-oriented methodologies, such as overriding a function (called "procs" in DM) or sectioning code into functions and then overriding them as required.

No duplicated code

Copying code from one place to another may be suitable for small, short-time projects, but BeeStation is a long-term project and highly discourages this.

Instead you can use object orientation, or simply placing repeated code in a function, to obey this specification easily.

Reduce module scope

Adding new variables/procs to all types should be avoided if it is only used on a small number of types or in a single place. Instead, use a method that limits the scope of the changes to the item you are adding (With a lookup table, for example).

Bad:

/mob
  //This variable is only ever used by a single item, creating a variable that doesn't need to exist on all mobs
  var/can_pet = FALSE

/mob/dog
  can_pet = TRUE

/mob/cat
  can_pet = TRUE

/obj/item/auto_petter

/obj/item/auto_petter/proc/check_mob(mob/target)
  return target.can_pet

Good:

/obj/item/auto_petter
  //Lookup table only exists on this item, if we want to remove this item then its easy,
  //Just delete this file!
  //Static to save on init times and memory usage (Very slightly).
  var/static/list/affected_types = list(
    /mob/cat = TRUE,
    /mob/dog = TRUE
  )

/obj/item/auto_petter/proc/check_mob(mob/target)
  return affected_types[target.type]

Note that this approach is not always better, you will have to determine what the best approach to take when creating features like this.

Readable Line Lengths

Comments should generally appear above the line of code they are commenting on if the line length is extremely long. Any lists and strings that are extremely long should be made multiline.

// An example of a multi-line list
var/list/a = list(
  "hello",
  "hi!",
)

No huge files

Avoid huge files with a ton of different behaviours in them. Large files are difficult to read and make modification/refactoring harder. Where possible, split larger files down into multiple smaller files that manage a single datum. As a decent guideline, files exceeding 300 lines are considered to be pretty big and should be reduced if possible.

For example, a magical item that has different effects will be more readable and easier to modify if its effects are split across many different files, rather than placed into 1 file containing 2000 lines.

Startup/Runtime tradeoffs with lists and the "hidden" init proc

First, read the comments in this BYOND thread, starting where the link takes you.

There are two key points here:

Defining a list in the variable's definition calls a hidden proc - init. If you have to define a list at startup, do so in New() (or preferably Initialize()) and avoid the overhead of a second call (Init() and then New())
It also consumes more memory to the point where the list is actually required, even if the object in question may never use it!

Remember: although this tradeoff makes sense in many cases, it doesn't cover them all. Think carefully about your addition before deciding if you need to use it.

Prefer `Initialize()` over `New()` for atoms

Our game controller is pretty good at handling long operations and lag, but it can't control what happens when the map is loaded, which calls New for all atoms on the map. If you're creating a new atom, use the Initialize proc to do what you would normally do in New. This cuts down on the number of proc calls needed when the world is loaded. See here for details on Initialize: https://github.com/tgstation/tgstation/blob/master/code/game/atoms.dm#L49 While we normally encourage (and in some cases, even require) bringing out of date code up to date when you make unrelated changes near the out of date code, that is not the case for New -> Initialize conversions. These systems are generally more dependant on parent and children procs so unrelated random conversions of existing things can cause bugs that take months to figure out.

No magic numbers or strings

This means stuff like having a "mode" variable for an object set to "1" or "2" with no clear indicator of what that means. Make these #defines with a name that more clearly states what it's for. For instance:

/datum/proc/do_the_thing(thing_to_do)
	switch(thing_to_do)
		if(1)
			(...)
		if(2)
			(...)

There's no indication of what "1" and "2" mean! Instead, you'd do something like this:

#define DO_THE_THING_REALLY_HARD 1
#define DO_THE_THING_EFFICIENTLY 2
/datum/proc/do_the_thing(thing_to_do)
	switch(thing_to_do)
		if(DO_THE_THING_REALLY_HARD)
			(...)
		if(DO_THE_THING_EFFICIENTLY)
			(...)

This is clearer and enhances readability of your code! Get used to doing it!

Control statements

(if, while, for, etc)

All control statements must not contain code on the same line as the statement (if (blah) return)
All control statements comparing a variable to a number should use the formula of thing operator number, not the reverse (eg: if (count <= 10) not if (10 >= count))

Use early return

Do not enclose a proc in an if-block when returning on a condition is more feasible This is bad:

/datum/datum1/proc/proc1()
	if (thing1)
		if (!thing2)
			if (thing3 == 30)
				do stuff

This is good:

/datum/datum1/proc/proc1()
	if (!thing1)
		return
	if (thing2)
		return
	if (thing3 != 30)
		return
	do stuff

This prevents nesting levels from getting deeper then they need to be.

Develop Secure Code

Player input must always be escaped safely, we recommend you use stripped_input in all cases where you would use input. Essentially, just always treat input from players as inherently malicious and design with that use case in mind
Calls to the database must be escaped properly - use sanitizeSQL to escape text based database entries from players or admins, and isnum() for number based database entries from players or admins.
All calls to topics must be checked for correctness. Topic href calls can be easily faked by clients, so you should ensure that the call is valid for the state the item is in. Do not rely on the UI code to provide only valid topic calls, because it won't.
Information that players could use to metagame (that is, to identify round information and/or antagonist type via information that would not be available to them in character) should be kept as administrator only.
It is recommended as well you do not expose information about the players - even something as simple as the number of people who have readied up at the start of the round can and has been used to try to identify the round type.
Where you have code that can cause large-scale modification and FUN, make sure you start it out locked behind one of the default admin roles - use common sense to determine which role fits the level of damage a function could do.

Files

Because runtime errors do not give the full path, try to avoid having files with the same name across folders.
File names should not be mixed case, or contain spaces or any character that would require escaping in a uri.
Files and path accessed and referenced by code above simply being #included should be strictly lowercase to avoid issues on filesystems where case matters.

SQL

Do not use the shorthand sql insert format (where no column names are specified) because it unnecessarily breaks all queries on minor column changes and prevents using these tables for tracking outside related info such as in a connected site/forum.
All changes to the database's layout(schema) must be specified in the database changelog in SQL, as well as reflected in the schema files
Any time the schema is changed the schema_revision table and DB_MAJOR_VERSION or DB_MINOR_VERSION defines must be incremented.
Queries must never specify the database, be it in code, or in text files in the repo.
Primary keys are inherently immutable and you must never do anything to change the primary key of a row or entity. This includes preserving auto increment numbers of rows when copying data to a table in a conversion script. No amount of bitching about gaps in ids or out of order ids will save you from this policy.

Always create UpdatePaths

Whenever a typepath is changed from one path to another, if the new type is to be replaced with another in maps, you must create an UpdatePaths script. This is important so that WIP maps or other PRs can be easily updated by mappers.

The same applies if a mass-varedit is applied in such a way that it can be done with UpdatePaths.

The UpdatePaths Script should be placed in tools/UpdatePaths/Scripts and formatted with the filename: [PR Number]_[Paths Updated or PR Title].txt

Scripts should preserve varedits wherever possible, by using {@OLD} and using @ANY/@UNSET matching if a var needs to be renamed or removed.

/obj/item/old_path : /obj/item/new_path{@OLD}
/obj/item/old_path_with_varedit{icon_state = @ANY} : /obj/item/new_path_with_varedit{@OLD; icon_state_new = @OLD:icon_state}

To repath subtypes (they are not repathed by default):

/obj/item/old_path/@SUBTYPES : /obj/item/new_path/@SUBTYPES{@OLD}

Mapping Standards

TGM Format & Map Merge
- All new maps submitted to the repo through a pull request must be in TGM format (unless there is a valid reason present to have it in the default BYOND format.) This is done using the Map Merge utility included in the repo to convert the file to TGM format.
- Likewise, you MUST run Map Merge prior to opening your PR when updating existing maps to minimize the change differences (even when using third party mapping programs such as FastDMM.)
  - Failure to run Map Merge on a map after using third party mapping programs (such as FastDMM) greatly increases the risk of the map's key dictionary becoming corrupted by future edits after running map merge. Resolving the corruption issue involves rebuilding the map's key dictionary; id est rewriting all the keys contained within the map by reconverting it from BYOND to TGM format - which creates very large differences that ultimately delay the PR process and is extremely likely to cause merge conflicts with other pull requests.
Variable Editing (Var-edits)
- While var-editing an item within the editor is perfectly fine, it is preferred that when you are changing the base behavior of an item (how it functions) that you make a new subtype of that item within the code, especially if you plan to use the item in multiple locations on the same map, or across multiple maps. This makes it easier to make corrections as needed to all instances of the item at one time as opposed to having to find each instance of it and change them all individually.
  - Subtypes only intended to be used on away mission or ruin maps should be contained within an .dm file with a name corresponding to that map within code\modules\awaymissions or code\modules\ruins respectively. This is so in the event that the map is removed, that subtype will be removed at the same time as well to minimize leftover/unused data within the repo.
- Please attempt to clean out any dirty variables that may be contained within items you alter through var-editing. For example, due to how DM functions, changing the pixel_x variable from 23 to 0 will leave a dirty record in the map's code of pixel_x = 0. Likewise this can happen when changing an item's icon to something else and then back. This can lead to some issues where an item's icon has changed within the code, but becomes broken on the map due to it still attempting to use the old entry.
- Areas should not be var-edited on a map to change it's name or attributes. All areas of a single type and it's altered instances are considered the same area within the code, and editing their variables on a map can lead to issues with powernets and event subsystems which are difficult to debug.

Other Notes

Code should be modular where possible; if you are working on a new addition, then strongly consider putting it in its own file unless it makes sense to put it with similar ones (i.e. a new tool would go in the "tools.dm" file)
Bloated code may be necessary to add a certain feature, which means there has to be a judgement over whether the feature is worth having or not. You can help make this decision easier by making sure your code is modular.
You are expected to help maintain the code that you add, meaning that if there is a problem then you are likely to be approached in order to fix any issues, runtimes, or bugs.
Do not divide when you can easily convert it to multiplication. (ie 4/2 should be done as 4*0.5)
If you used regex to replace code during development of your code, post the regex in your PR for the benefit of future developers and downstream users.
Changes to the /config tree must be made in a way that allows for updating server deployments while preserving previous behaviour. This is due to the fact that the config tree is to be considered owned by the user and not necessarily updated alongside the remainder of the code. The code to preserve previous behaviour may be removed at some point in the future given the OK by maintainers.
The dlls section of tgs3.json is not designed for dlls that are purely call()()ed since those handles are closed between world reboots. Only put in dlls that may have to exist between world reboots.

Enforced not enforced

The following coding styles are not only not enforced at all, but are generally frowned upon to change for little to no reason:

English/British spelling on var/proc names
- Color/Colour - both are fine, but keep in mind that BYOND uses color as a base variable
Spaces after control statements
- if() and if () - nobody cares!

Operators

Spacing

(this is not strictly enforced, but more a guideline for readability's sake)

Operators that should be separated by spaces
- Boolean and logic operators like &&, || <, >, ==, etc (but not !)
- Bitwise AND &
- Argument separator operators like , (and ; when used in a forloop)
- Assignment operators like = or += or the like
Operators that should not be separated by spaces
- Bitwise OR |
- Access operators like . and :
- Parentheses ()
- logical not !

Math operators like +, -, /, *, etc are up in the air, just choose which version looks more readable.

Use

Bitwise AND - '&'
- Should be written as bitfield & bitflag NEVER bitflag & bitfield, both are valid, but the latter is confusing and nonstandard.
Associated lists declarations must have their key value quoted if it's a string
- WRONG: list(a = "b")
- RIGHT: list("a" = "b")

Dream Maker Quirks/Tricks

Like all languages, Dream Maker has its quirks, some of them are beneficial to us, like these

In-To for-loops

for(var/i = 1, i <= some_value, i++) is a fairly standard way to write an incremental for loop in most languages (especially those in the C family), but DM's for(var/i in 1 to some_value) syntax is oddly faster than its implementation of the former syntax; where possible, it's advised to use DM's syntax. (Note, the to keyword is inclusive, so it automatically defaults to replacing <=; if you want < then you should write it as 1 to some_value-1).

HOWEVER, if either some_value or i changes within the body of the for (underneath the for(...) header) or if you are looping over a list AND changing the length of the list then you can NOT use this type of for-loop!

for(var/A in list) VS for(var/i in 1 to list.len)

The former is faster than the latter, as shown by the following profile results: https://file.house/zy7H.png Code used for the test in a readable format: https://pastebin.com/w50uERkG

Istypeless for loops

A name for a differing syntax for writing for-each style loops in DM. It's NOT DM's standard syntax, hence why this is considered a quirk. Take a look at this:

var/list/bag_of_items = list(sword, apple, coinpouch, sword, sword)
var/obj/item/sword/best_sword
for(var/obj/item/sword/S in bag_of_items)
	if(!best_sword || S.damage > best_sword.damage)
		best_sword = S

The above is a simple proc for checking all swords in a container and returning the one with the highest damage, and it uses DM's standard syntax for a for-loop by specifying a type in the variable of the for's header that DM interprets as a type to filter by. It performs this filter using istype() (or some internal-magic similar to istype() - this is BYOND, after all). This is fine in its current state for bag_of_items, but if bag_of_items contained ONLY swords, or only SUBTYPES of swords, then the above is inefficient. For example:

var/list/bag_of_swords = list(sword, sword, sword, sword)
var/obj/item/sword/best_sword
for(var/obj/item/sword/S in bag_of_swords)
	if(!best_sword || S.damage > best_sword.damage)
		best_sword = S

specifies a type for DM to filter by.

With the previous example that's perfectly fine, we only want swords, but here the bag only contains swords? Is DM still going to try to filter because we gave it a type to filter by? YES, and here comes the inefficiency. Wherever a list (or other container, such as an atom (in which case you're technically accessing their special contents list, but that's irrelevant)) contains datums of the same datatype or subtypes of the datatype you require for your loop's body, you can circumvent DM's filtering and automatic istype() checks by writing the loop as such:

var/list/bag_of_swords = list(sword, sword, sword, sword)
var/obj/item/sword/best_sword
for(var/s in bag_of_swords)
	var/obj/item/sword/S = s
	if(!best_sword || S.damage > best_sword.damage)
		best_sword = S

Of course, if the list contains data of a mixed type then the above optimisation is DANGEROUS, as it will blindly typecast all data in the list as the specified type, even if it isn't really that type, causing runtime errors.

Dot variable

Like other languages in the C family, DM has a . or "Dot" operator, used for accessing variables/members/functions of an object instance. eg:

var/mob/living/carbon/human/H = YOU_THE_READER
H.gib()

However, DM also has a dot variable, accessed just as . on its own, defaulting to a value of null. Now, what's special about the dot operator is that it is automatically returned (as in the return statement) at the end of a proc, provided the proc does not already manually return (return count for example.) Why is this special?

With . being everpresent in every proc, can we use it as a temporary variable? Of course we can! However, the . operator cannot replace a typecasted variable - it can hold data any other var in DM can, it just can't be accessed as one, although the . operator is compatible with a few operators that look weird but work perfectly fine, such as: .++ for incrementing .'s value, or .[1] for accessing the first element of ., provided that it's a list.

Globals versus static

DM has a var keyword, called global. This var keyword is for vars inside of types. For instance:

mob
	var
		global
			thing = TRUE

This does NOT mean that you can access it everywhere like a global var. Instead, it means that that var will only exist once for all instances of its type, in this case that var will only exist once for all mobs - it's shared across everything in its type. (Much more like the keyword static in other languages like PHP/C++/C#/Java)

Isn't that confusing?

There is also an undocumented keyword called static that has the same behaviour as global but more correctly describes BYOND's behaviour. Therefore, we always use static instead of global where we need it, as it reduces suprise when reading BYOND code.

Delta time

All process procs that contain probability or damage calculation should take into account delta time. This allows for consistent results per real world second rather than per tick, especially if the hosting server is under heavy load. Not only does this make the average player experience more consistent but it's a lot easier to manage from a design point of view, otherwise we would need to re-tune every single thing that uses the tick rate if we ever wanted to change it.

Example:

/obj/machinery/cell_charger/process(delta_time)
	var/charge_rate = 20
	charge(charge_rate * delta_time)

This will either increase or decrease the charge rate depending on the time between now and the last process tick, ensuring consistent results.

Probability is a little bit more complicated, it's worth taking a look in maths.dm for an in depth explanation. Otherwise you're better off just using the define for it (also found in maths.dm ).

Text Formatting

Correct Spans

When sending messages to the user, they should always be wrapped in a span that makes sense. All spans must be closed with 

: For informational messages such as installing batteries, turning on stun batons etc.

: For indicating that an action cannot take place, a machine malfunctioning or anything else bad but not posing direct danger to the user.

: For when the user is performing a dangerous action or coming to harm.

: For when someone else is performing a dangerous action or coming to harm.

You may see the stylesheet for more niche spans (Ctrl-shift-f and search for .userdanger in VS code).

Developer Wiki
Game Wiki