diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 98686aee07d..ba253b3073e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -21,7 +21,7 @@ reporting bugs in the code. Refer to ISSUE_TEMPLATE for the exact format that yo should be in. #### Guidelines: - - Issue reports should be as detailed as possible, and if applicable, should include + * Issue reports should be as detailed as possible, and if applicable, should include instructions on how to reproduce the bug. ## Pull requests @@ -31,119 +31,491 @@ strongly recommended you get approval/traction for it from our forums before sta actual development. #### Guidelines: - - Pull requests should be atomic; Make one commit for each distinct change, so if a part + * Pull requests should be atomic; Make one commit for each distinct change, so if a part of a pull request needs to be removed/changed, you may simply modify that single commit. Due to limitations of the engine, this may not always be possible; but do try your best. - - Document and explain your pull requests thoroughly. Detail what each commit changes, + * Document and explain your pull requests thoroughly. Detail what each commit changes, and why it changes it. We do not want to have to read all of you commit names to figure out what your pull request is about. - - Any pull request that is not solely composed of fixes or non gameplay-affecting + * Any pull request that is not solely composed of fixes or non gameplay-affecting refactors must have a changelog. Inline changelogs are supported through the format described [here](https://github.com/ParadiseSS13/Paradise/pull/3291#issuecomment-172950466) and should be used rather than manually edited .yml file changelogs. - - Pull requests should not have any merge commits except in the case of fixing merge + * Pull requests should not have any merge commits except in the case of fixing merge conflicts for an existing pull request. New pull requests should not have any merge commits. Use `git rebase` or `git reset` to update your branches, not `git pull`. -#### BYOND Specific Guidelines: - - Any `type` or `proc` paths **must** use absolute pathing unless the file you are - working in primarily utilizes relative pathing. - - Paths must begin with `/`. It should be `/obj/machinery/fancy_robot`, - not `obj/machinery/fancy_robot`. - - New bases of datum must begin with `/datum/`. `/datum/arbitrary_datum`, - not `/arbitrary_datum`. - - Don't use strings in combination with `text2path()` unless the paths are being - dynamically created. Variables can contain normal paths just fine. - - Don't duplicate code. If you have identical code in two places, it should probably - be a new proc that they both can use. - - No magic numbers/strings. If you have a number or text that is important and used in - your code, make a `#DEFINE` statement with a name that clearly indicates it's use. - - `if(condition)` must be used over `if (condition)` or any other variation. - - The same applies for `while` and `for` loops, they must have no space between the - keyword and condition brackets. `while(condition)`, `for(condition)` - - If you want to output a message to a player's chat - (this includes text sent to `world`), use `to_chat(mob/client/world, "message")`. - Do not use `mob/client/world << "message"`. - - Do not use one-line control statements (if, else, for, while, etc). The space saved - is not worth the decreased readability. - - Control statements comparing a variable to a constant should be formatted `variable`, - `operator`, `constant`. This means `if(count <= 10)` is preferred over - `if(10 >= count)`. - - **Never** use a colon `:` operator to bypass type safety checks, unless you are doing - something where the tiny performance increase is incredibly noticeable (eg, a loop for - a huge list). You should properly typecast everything and use the period `.` - operator. - - Use early returns, and avoid far-indented if blocks. This means that you should not - do this: +#### Using Changelog + * Tags used in changelog include add/rscadd, del/rscdel, fix/fixes, typo/spellcheck. + * Without specifying a name it will default to using your GitHub name. + Some examples +``` +:cl: +add: The ability to change the color of wires +del: Deleted depreciated wire merging now handled in parent +fix: Moving wires now follows the user input instead of moving the stack +/:cl: +``` +``` +:cl: N3X15 +typo: Fixes some misspelled words under Using Changelog +/:cl: +``` + + +## Specifications + +As mentioned before, you are expected to follow these specifications 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! + +### 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: + +```DM +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 *unless the majority of the file is already relatively pathed* 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: + +```DM +/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` + +### Datum type paths must began with "datum" +In DM, this is optional, but omitting it makes finding definitions harder. To be specific, you can declare the path `/arbitrary`, but it +will still be, in actuality, `/datum/arbitrary`. Write your code to reflect this. + +### 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: + +```DM +//Good +var/path_type = /obj/item/baseball_bat + +//Bad +var/path_type = "/obj/item/baseball_bat" +``` + +### Do not use `\The`. +The `\The` macro doesn't actually do anything when used in the format `\The [atom reference]`. Directly referencing an atom in an embedded string +will automatically prefix `The` or `the` to it as appropriate. As an extension, when referencing an atom, don't use `[atom.name]`, use `[atom]`. +The only exception to this rule is when dealing with items "belonging" to a mob, in which case you should use `[mob]'s [atom.name]` to avoid `The` +ever forming. + +```DM +//Good +var/atom/A +"[A]" + +//Bad +"\The [A]" +``` + +### Use the pronoun library instead of `\his` macros. +We have a system in code/\_\_HELPERS/pronouns.dm for addressing all forms of pronouns. This is useful in a number of ways; + * BYOND's \his macro can be unpredictable on what object it references. + Take this example: `"[user] waves \his [user.weapon] around, hitting \his opponents!"`. + This will end up referencing the user's gender in the first occurence, but what about the second? + It'll actually print the gender set on the weapon he's carrying, which is unintended - and there's no way around this. + * It always prints the real `gender` variable of the atom it's referencing. This can lead to exposing a mob's gender even when their face is covered, + which would normally prevent it's gender from being printed. + +The way to avoid these problems is to use the pronoun system. Instead of `"[user] waves \his arms."`, you can do `"[user] waves [user.p_their()] arms."` + +``` +//Good +"[H] waves [H.p_their()] hands!" +"[user] waves [H.p_their()] [user.weapon] around, hitting [H.p_their()] opponents!"` + +//Bad +"[H] waves \his hands!" +"[user] waves \his [user.weapon] around, hitting \his opponents!" +``` + +### Use `[A.UID()]` over `\ref[A]` +BYOND has a system to pass "soft references" to datums, using the format `"\ref[datum]"` inside a string. This allows you to find the object just based +off of a text string, which is especially useful when dealing with the bridge between BYOND code and HTML/JS in UIs. It's resolved back into an object +reference by using `locate("\ref[datum]")` when the code comes back to BYOND. The issue with this is that locate() can return a unexpected datum +if the original datum has been deleted - BYOND recycles the references. + +UID's are actually unique; they work off of a global counter and are not recycled. Each datum has one assigned to it when it's created, which can be +accessed by `[datum.UID()]`. You can use this as a snap-in replacement for `\ref` by changing any `locate(ref)` calls in your code to `locateUID(ref)`. +Usage of this system is mandatory for any /Topic( calls, and will produce errors in Dream Daemon if it's not used. ``, not `