Skip to content

Commit

Permalink
Updated contributor information and added official style guide
Browse files Browse the repository at this point in the history
  • Loading branch information
BomBardyGamer committed May 19, 2022
1 parent 3417436 commit dcb6e3f
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 89 deletions.
23 changes: 3 additions & 20 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,13 @@ insert_final_newline = true

ij_continuation_indent_size = 8
ij_smart_tabs = true
ij_any_block_brace_style = end_of_line
ij_any_class_brace_style = end_of_line
ij_any_lambda_brace_style = end_of_line
ij_any_method_brace_style = end_of_line
ij_any_blank_lines_after_class_header = 1
ij_any_blank_lines_after_anonymous_class_header = 1
ij_any_blank_lines_after_imports = 1
ij_any_blank_lines_after_package = 1
ij_any_blank_lines_before_package = 0
ij_any_blank_lines_before_imports = 0
ij_any_blank_lines_around_class = 0
ij_any_blank_lines_around_field = 0
ij_any_blank_lines_around_field_in_interface = 2
ij_any_blank_lines_around_method = 2
ij_any_blank_lines_around_method_in_interface = 2
ij_any_blank_lines_before_method_body = 0
ij_any_blank_lines_before_class_end = 0

[{*.json, *.conf}]
[*.{json,conf}]
indent_size = 2
insert_final_newline = false

[{*.yaml, *.yml}]
[*.{yaml,yml}]
indent_size = 2

[{*.xml, Dockerfile}]
[{*.xml,Dockerfile}]
insert_final_newline = false
97 changes: 28 additions & 69 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
# Krypton contribution guide

Hey there! Good to see you've decided to open this document! I hope you enjoy your stay, and I hope this proves useful to
you.
Hey there! Good to see you've decided to open this document! I hope you enjoy your stay, and I hope this proves useful to you.

First of all, thank you for considering contributing to Krypton! I really appreciate all who do! This project may not
survive without contributions.
First of all, thank you for considering contributing to Krypton! I really appreciate all who do! This project may not survive without contributions.

**Table of contents:**
* [Requirements](#requirements)
Expand All @@ -17,73 +15,41 @@ survive without contributions.

## Requirements

First and foremost, I assume that you already have your chosen IDE. If not, I can personally recommend [IntelliJ](https://www.jetbrains.com/idea/).
First of all, this guide assumes that you have some knowledge on how to program in Kotlin, and you have chosen your IDE to use.
If you don't know how to program in Kotlin, I recommend the [official Kotlin documentation](https://kotlinlang.org/docs/home.html).

Secondly, you'll need a few tools to start making contributions. Most of these should come with your IDE, but some may not:
* Gradle
* Git
* Some form of Kotlin support / Kotlin plugin

If your IDE doesn't have support for Gradle, you can use the Gradle wrapper (`gradlew` or `gradlew.bat`), and for Git, you can
either use Git Bash (Windows/Mac), or just install the `git` command-line tool (Linux). Or, if you prefer GUIs, I can personally
recommend [GitKraken](https://www.gitkraken.com/).

Third and final, I recommend familiarising yourself with the code style of the project (you can see a few style hints in `.editorconfig`,
or you can just browse some files and have a look for yourself).
We closely follow the official [Kotlin conventions](https://kotlinlang.org/docs/coding-conventions.html), with the following constraints:
* All spacing must use the space character. No tab characters must be present in source files.
* Empty lines should contain no spaces at all
* No trailing whitespace
* All API additions, and (preferably) all public server additions should be properly documented (for maintainability).
* All code should be free of magic values (generally hard coded numbers or strings that have no meaning and no purpose other
than just making things work) if possible. If you have an addition that should be configurable, add it to the config. If your
addition should be hard coded, use a variable, or better yet, a constant
* For example:
```kotlin
const val SOME_VALUE = 10

// Usage
if (something == SOME_VALUE) doSomething()
```
is better than:
```kotlin
if (something == 10) doSomething()
```
If your IDE doesn't have support for Gradle, you can use the Gradle wrapper (`gradlew` or `gradlew.bat`), which requires no installation and no setup.
For Git, you can download a standalone version [here](https://git-scm.com/downloads), though if you are using a Linux distribution, you probably already have Git.

Third and final, I recommend familiarising yourself with the code style of the project.
I recommend you browse some files in the project and learn how the project is styled.
You should also read the [style guide](STYLE_GUIDE.md) for more in-depth details.

## Specific Information
### API

The API is designed around that of it's predecessors. It is primarily designed to be concise enough to not be complex for
beginners to pick up, but also be advanced enough that more experienced developers can do a lot with it. We mainly take
inspiration from Bukkit, Sponge, Velocity, and Minestom when choosing how to design new APIs.
Plugins should be heavily reliant on dependency injection, and using static accessors is not recommended, with the exception
of catalogue classes and registries. We use [Guice](https://github.com/google/guice) for dependency injection, which is
quite easy to pick up, if you can make sense of dependency inversion.
The API is designed around that of it's predecessors. It is primarily designed to be concise enough to not be complex for beginners to pick up, but also be advanced enough that more experienced developers can do a lot with it. We mainly take inspiration from Bukkit, Sponge, Velocity, and Minestom when choosing how to design new APIs.

Use of classes should be avoided, in favour of interfaces with factories, in order to abstract away as much implementation
detail as possible, help with maintaining and updating the API for future versions of Minecraft, and also in some cases
permit custom implementations of parts of the API.
Plugins should be heavily reliant on dependency injection, and using static accessors is not recommended, with the exception of catalogue classes and registries. We use [Guice](https://github.com/google/guice) for dependency injection, which is quite easy to pick up, if you can make sense of dependency inversion.

Also, Krypton's API should be designed in a way that allows it to evolve nicely with new Minecraft versions. That isn't to
say that new API changes are not welcome, but please consider the impact that your changes may have in the future.
You should also look in to potential use cases as well as potential maintenance costs. If the negatives outweigh the positives,
it's probably not worth it.
Also, Krypton's API should be designed in a way that allows it to evolve nicely with new Minecraft versions.
That isn't to say that new API changes are not welcome, but please consider the impact that your changes may have in the future.
You should also look in to potential use cases as well as potential maintenance costs. If the negatives outweigh the positives, it's probably not worth it.

In Krypton's current state, maintaining backwards compatibility is not a priority, and things like deprecation and removal processes
aren't something that we are doing yet. These are, however, on the list of things that will likely be implemented with a stable
release of the API.
In Krypton's current state, maintaining backwards compatibility is not a priority, and things like deprecation and removal processes aren't something that we are doing yet.
These are, however, on the list of things that will likely be implemented with a stable release of the API.

### Server

The server is a bit more complex than the API. It does not have many of the strict requirements of the API, like Java
compatibility or abstraction layers for plugins, but it still does have some guidelines that you should at least attempt
to follow.
The server is a bit more complex than the API. It does not have many of the strict requirements of the API, like Java compatibility or abstraction layers for plugins, but it still does have some guidelines that you should at least attempt to follow.

The server is not designed in a way that will maintain backwards compatibility for dependents of it, as this would have too high
of a maintenance cost, and severely limit our ability to evolve Krypton through Minecraft versions.
Generally, there will be certain things within the server that are provided purely for the purpose of being used as an API, and
these will usually be considered somewhat public API, though they can be changed if it becomes necessary to do so.
The server is not designed in a way that will maintain backwards compatibility for dependents of it, as this would have too high of a maintenance cost, and severely limit our ability to evolve Krypton through Minecraft versions.
Generally, there will be certain things within the server that are provided purely for the purpose of being used as an API, and these will usually be considered somewhat public API, though they can be changed if it becomes necessary to do so.

Also, when designing implementations, you should be careful to ensure that you are:
* Properly following the specification for the API you are implementing
Expand All @@ -94,31 +60,24 @@ Also, when designing implementations, you should be careful to ensure that you a

The rules on documentation vary depending on what you're working on.

For the API, everything has to be documented, regardless of what it is. This is to avoid the possibility of something not getting
documented, and so being ambiguous for people trying to understand how it works.
In addition, documentation should be clear and concise. You don't need to ramble on about every single detail about how something
works, but don't be over ambiguous. Also, word choice is key. For functions like `contains`, the word "checks" is often used. For
getters, use "Gets". For functions that should create a new object, the words "Creates a new {type}" should be used, where {type}
should be replaced with the name of the thing you are creating, such as "Creates a new game profile".
For the API, everything has to be documented, regardless of what it is. This is to avoid the possibility of something not getting documented, and so being ambiguous for people trying to understand how it works.
In addition, documentation should be clear and concise. You don't need to ramble on about every single detail about how something works, but don't be over ambiguous.
Also, word choice is key. For functions like `contains`, the word "checks" is often used. For
getters, consider "Gets". For functions that should create a new object, consider using the words "Creates a new {type}", where {type} is be replaced with the name of the thing you are creating, such as "Creates a new game profile".

For the server, not everything has to be documented, though everything that you may deem ambiguous should be documented, to reduce
the time that people have to spend trying to decipher your code.
For the server, not everything has to be documented, though everything that you may deem ambiguous should be documented, to reduce the time that people have to spend trying to decipher your code.

## Formatting

Your commit messages for Git should preferably be short and concise, and describe the changes that you have made in it.
It is strongly recommended that you only make one change per commit, and this will help with keeping your description
short.

Krypton uses a Kotlin static analysis tool called Detekt. This is already pre-configured for the API, and it is mandatory that
this be adhered to, else building the project will fail. The rules are rather lenient, and so you should have no issues if you
are following proper procedures, though you can add exclusions in Detekt's `baseline.xml`. More information about how to setup
the baseline can be found [here](https://detekt.github.io/detekt/baseline.html).
Krypton uses a Kotlin static analysis tool called Detekt. This is already pre-configured for the API, and it is mandatory that this be adhered to, else building the project will fail. The rules are rather lenient, and so you should have no issues if you are following proper procedures, though you can add exclusions in Detekt's `baseline.xml`.
More information about how to setup the baseline can be found [here](https://detekt.github.io/detekt/baseline.html).

## Other

If you have any suggestions that you think would make this contribution guide better, perhaps either making this more
informative, removing informality, fixing grammar or typos, or anything else, feel free to either open an issue or make
a pull request.
If you have any suggestions that you think would make this contribution guide better, perhaps either making this more informative, removing informality, fixing grammar or typos, or anything else, feel free to either open an issue or make a pull request.

And finally, thank you for spending the time to read this contribution guide! Happy contributing!
93 changes: 93 additions & 0 deletions STYLE_GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# Krypton style guide
This guide serves as a guide for code style when contributing to the Krypton project.
Most of this is banning bad practices and recommendations to avoid additional maintenance burden.

For the most part, we follow the [official Kotlin conventions](https://kotlinlang.org/docs/coding-conventions.html), with the following constraints:
* The top-level package is not omitted, and it is a full directory.
* Avoid files with multiple top-level classes, as it can be difficult to name files, and it can also be difficult to find these classes if you don't have an IDE (such as viewing through GitHub).
* Do not use UpperCamelCase for any variable or function declarations, no matter what they are.
* Avoid private backing fields with public overrides, especially fields beginning with an underscore (`_`).

### Banned language features
1. Extension properties. These can be hard to understand, they hide what's actually going on (they're backed by methods), and they often require other techniques like memoization to make them work effectively like fields. Just use a function.
2. Extension functions as members. This is when you place an extension function inside a class or interface, like this:
```kotlin
class MyClass {

fun String.slice(delimeter: CharSequence): Array<String> = split(delimiter).toTypedArray()
}
```
This has two receivers, and the syntax for calling this is really strange.
As a reference, [here is an example of how bad it can get](https://github.com/mworzala/canvas/blob/f74d0e3568a28214056155eef7ec1532e13d21d4/src/main/kotlin/com/mattworzala/canvas/RenderContext.kt#L54). Don't do this. Ever.

### Language syntax recommendations
* Avoid public top-level functions, as the imports for them can be hard to trace back to their source outside of an IDE, and they can lack context. The exception to this is DSL functions.
* Always explicitly declare return types of functions and types of public variables.
* Avoid public top-level constants. The imports can be hard to track outside of an IDE, and they will also lack context.
* Avoid excessive labels. The nesting context can be hard to track.
* Avoid statement expressions or overly complex expressions in string templates.
These can be hard to read and hard to track, often require confusing wrapping of the strings, and are usually just there to avoid writing one extra line with a variable.
For example, you can make an if expression in a template a separate variable:
```kotlin
// This should be avoided
val hello = "Hello ${if (hasSurname) "$firstName:$surname" else firstName}!"
// This would be better
val name = if (hasSurname) "$firstName:$surname" else firstName
val hello = "Hello $name!"
```
* Avoid expressions with bodies as expression function return types, as it can sometimes be unclear what something belongs to, especially as the bodies are usually aligned where the function's body would be.
For example, this should be avoided:
```kotlin
fun getFile(name: String): File? = try {
File(name)
} catch (exception: Exception) {
null
}
```
* Wrap long chained arguments on the `.`, for example:
```kotlin
val myValue = myMap.filter { it.key == "Hello" }
.map { it.value }
.toList()
.firstOrNull { it == "Goodbye" }
```
* Avoid nullable primitive types, as they will always result in boxed types, and boxing/unboxing primitives has bad performance.
All Kotlin types that are wrappers for Java primitives: `Byte`, `Short`, `Int`, `Long`, `Char`, `Float`, `Double`, and `Boolean`.
For example, these two functions:
```kotlin
fun nonNullNumber(number: Int): Int = number
fun nullableNumber(number: Int?): Int? = number
```
Map to these methods in Java:
```java
public int nonNullNumber(int number) {
return number;
}
public Integer nullableNumber(Integer number) {
return number;
}
```
* Always try to use `@Jvm` annotations, such as `@JvmStatic`, `@JvmOverride`, `@JvmField`, etc., especially when writing API code, as the API must be Java friendly as well as Kotlin friendly.
### API-specific rules and recommendations
* All classes, interfaces, functions, and variables must have explicit visibility and types. This is enforced by the compiler.
* All public elements must be documented. No exceptions.
* Try to follow the factory pattern, in which API elements are interfaces with factories for initializers. This allows us the greatest freedom when implementing the chosen API design, and allows the greatest freedom to adapt to a changing Minecraft.
* Try to find the right balance between too much and not enough when designing APIs. Avoid providing many different alternatives to reach the same result, as this will be confusing. Remember that every single thing you add will need to be maintained.
* Try to avoid revealing implementation details in the API, as these constrain the implementation, stopping us from changing how it works without breaking the API.
### Server-specific rules and recommentations
* Avoid stealing code from elsewhere unless the licensing permits it. Always ensure the licensing of the target code permits use in our code.
Places you can take code from:
* Bukkit/Spigot/Paper
* Sponge
* Minestom
* BungeeCord
* Velocity
* Stack Overflow
Places you cannot take code from:
* Vanilla Minecraft
* Try to write automated tests for your implementations, to avoid having to manually test everything every time.
* If you are writing an API implementation, try to explain areas that may seem confusing or weird. Try to document things that cannot easily be understood by reading the code. Try to explain why you chose the implementation you did in your pull request.

0 comments on commit dcb6e3f

Please sign in to comment.