Skip to content

Commit

Permalink
Merge dev/patch into master (#6750)
Browse files Browse the repository at this point in the history
  • Loading branch information
APickledWalrus authored Jun 1, 2024
2 parents b0052d2 + 159fd3c commit 0355df1
Show file tree
Hide file tree
Showing 44 changed files with 1,398 additions and 701 deletions.
19 changes: 4 additions & 15 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import java.time.LocalTime

plugins {
id 'com.github.johnrengelman.shadow' version '8.1.1'
id 'com.github.hierynomus.license' version '0.16.1'
id 'maven-publish'
id 'java'
}
Expand Down Expand Up @@ -55,7 +54,7 @@ task checkAliases {
}

task testJar(type: ShadowJar) {
dependsOn(compileTestJava, licenseTest)
dependsOn(compileTestJava)
archiveFileName = 'Skript-JUnit.jar'
from sourceSets.test.output, sourceSets.main.output, project.configurations.testShadow
}
Expand Down Expand Up @@ -141,15 +140,6 @@ publishing {
}
}

license {
header file('licenseheader.txt')
exclude('**/Metrics.java') // Not under GPLv3
exclude('**/BurgerHelper.java') // Not exclusively GPLv3
exclude('**/*.sk') // Sample scripts and maybe aliases
exclude('**/*.lang') // Language files do not have headers (still under GPLv3)
exclude('**/*.json') // JSON files do not have headers
}

task releaseJavadoc(type: Javadoc) {
title = project.name + ' ' + project.property('version')
source = sourceSets.main.allJava
Expand Down Expand Up @@ -204,7 +194,6 @@ void createTestTask(String name, String desc, String environments, int javaVersi
}
tasks.register(name, JavaExec) {
description = desc
dependsOn licenseTest
if (junit) {
dependsOn testJar
} else if (releaseDocs) {
Expand Down Expand Up @@ -307,7 +296,7 @@ task githubResources(type: ProcessResources) {
include '**'
version = project.property('version')
def channel = 'stable'
if (version.contains('pre'))
if (version.contains('-'))
channel = 'prerelease'
filter ReplaceTokens, tokens: [
'version' : version,
Expand Down Expand Up @@ -340,7 +329,7 @@ task spigotResources(type: ProcessResources) {
include '**'
version = project.property('version')
def channel = 'stable'
if (version.contains('pre'))
if (version.contains('-'))
channel = 'prerelease'
filter ReplaceTokens, tokens: [
'version' : version,
Expand Down Expand Up @@ -388,7 +377,7 @@ task nightlyResources(type: ProcessResources) {

task nightlyRelease(type: ShadowJar) {
from sourceSets.main.output
dependsOn nightlyResources, licenseMain
dependsOn nightlyResources
archiveFileName = 'Skript-nightly.jar'
manifest {
attributes(
Expand Down
127 changes: 102 additions & 25 deletions code-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ With the exception of contacting our own resources (e.g. to check for updates) c
Code contributed must be licensed under GPLv3, by **you**.
We expect that any code you contribute is either owned by you or you have explicit permission to provide and license it to us.

Licenses do not need to be printed in individual files (or packages) unless the licence applying to the code in
that file (or package) deviates from the licence scope of its containing package.

Third party code (under a compatible licence) _may_ be accepted in the following cases:
- It is part of a public, freely-available library or resource.
- It is somehow necessary to your contribution, and you have been given permission to include it.
Expand All @@ -75,34 +78,61 @@ If we receive complaints regarding the licensing of a contribution we will forwa

If you have questions or complaints regarding the licensing or reproduction of a contribution you may contact us (the organisation) or the contributor of that code directly.

If, in the future, we need to relicense contributed code, we will contact all contributors involved.
If, in the future, we need to re-license contributed code, we will contact all contributors involved.
If we need to remove or alter contributed code due to a licensing issue we will attempt to notify its contributor.


## Code Style

### Formatting
* Imports should be grouped together by type (e.g. all `java.lang...` imports together)
* Following the style of existing imports in a class is encouraged, but not required
* Wildcard `*` imports are permitted (as long as they do not interfere with existing imports), e.g. `java.lang.*`.
* Tabs, no spaces (unless in code imported from other projects)
** No tabs/spaces in empty lines
- No tabs/spaces in empty lines
* No trailing whitespace
* At most 120 characters per line
- In Javadoc/multiline comments, at most 80 characters per line
* When statements consume multiple lines, all lines but first have two tabs of additional indentation
- In Javadoc/multiline comments, at most 80 characters per line
* When statements consume multiple lines, all lines but the first have two tabs of additional indentation
- The exception to this is breaking up conditional statements (e.g. `if (x || y)`) where the
condition starts may be aligned
* Each class begins with an empty line
* No squeezing of multiple lines of code on a single line
* Separate method declarations with empty lines
- Empty line after last method in a class is *not* required
- Otherwise, empty line before and after method is a good rule of thumb
- Empty line after last method in a class is *not* required
- Otherwise, empty line before and after method is a good rule of thumb
* If fields have Javadoc, separate them with empty lines
* Use empty lines liberally inside methods to improve readability
* Use curly brackets to start and end most blocks
- Only when a conditional block (if or else) contains a single statement, they may be omitted
- When omitting brackets, still indent as if the code had brackets
- Avoid omitting brackets if it produces hard-to-read code
- When a block contains a single statement, they may be omitted
- Brackets may not be omitted in a chain of other blocks that require brackets, e.g `if ... else {}`
- When omitting brackets, still indent as if the code had brackets
- Avoid omitting brackets if it produces hard-to-read or ambiguous code
* Ternaries should be avoided where it makes the code complex or difficult to read
* Annotations for methods and classes are placed in lines before their declarations, one per line
* When there are multiple annotations, place them in order:
- @Override -> @Nullable -> @SuppressWarnings
- For other annotations, doesn't matter; let your IDE decide
- Annotations for a structure go on the line before that structure
```java
@Override
@SuppressWarnings("xyz")
public void myMethod() {
// Override goes above method because method is overriding
}
```

- Annotations for the _value_ of a thing go before that value's type declaration
```java
@Override
public @Nullable Object myMethod() {
// Nullable goes before Object because Object is Nullable
}
```
* When there are multiple annotations, it looks nicer to place them in length order (longest last)
but this is not strictly required:
```java
@Override
@Deprecated
@SuppressWarnings("xyz")
```
* When splitting Strings into multiple lines the last part of the string must be (space character included) " " +
```java
String string = "example string " +
Expand All @@ -111,6 +141,7 @@ If we need to remove or alter contributed code due to a licensing issue we will
* When extending one of following classes: SimpleExpression, SimplePropertyExpression, Effect, Condition...
- Put overridden methods in order
- Put static registration before all methods
- SimpleExpression: init -> get/getAll -> acceptChange -> change -> setTime -> getTime -> isSingle -> getReturnType -> toString
- SimplePropertyExpression: -> init -> convert -> acceptChange -> change -> setTime -> getTime -> getReturnType -> getPropertyName
- Effect: init -> execute -> toString
Expand All @@ -130,8 +161,8 @@ If we need to remove or alter contributed code due to a licensing issue we will
* Use prefixes only where their use has been already established (such as `ExprSomeRandomThing`)
- Otherwise, use postfixes where necessary
- Common occurrences include: Struct (Structure), Sec (Section), EffSec (EffectSection), Eff (Effect), Cond (Condition), Expr (Expression)
* Ensure variable/field names are descriptive. Avoid using shorthand names like `e`, or `c`.
- e.g. Event should be `event`, not `e`. `e` is ambiguous and could mean a number of things.
* Ensure variable/field names are descriptive. Avoid using shorthand names like `e`, or `c`
- e.g. Event should be `event`, not `e`. `e` is ambiguous and could mean a number of things
### Comments
* Prefer to comment *why* you're doing things instead of how you're doing them
Expand Down Expand Up @@ -163,33 +194,79 @@ Your comments should look something like these:
## Language Features

### Compatibility
* Contributions should maintain Java 8 source/binary compatibility, even though compiling Skript requires Java 21
- Users must not need JRE newer than version 8
[//]: # (To be updated after feature/2.9 for Java 17)
* Contributions should maintain Java 11 source/binary compatibility, even though compiling Skript requires Java 21
- Users must not need JRE newer than version 11
* Versions up to and including Java 21 should work too
- Please avoid using unsafe reflection
* It is recommended to make fields final, if they are effectively final
* Local variables and method parameters should not be declared final
* Local variables and method parameters should not be declared final unless used in anonymous classes, lambdas
or try-with-resources sections where their immutability is necessary
* Methods should be declared final only where necessary
* Use `@Override` whenever applicable

### Nullness
- They may be omitted to prevent compilation errors when something overrides only
on a version-dependent basis (e.g. in Library XYZ version 2 we override `getX()` but in version 3 it's
gone, and we call it ourselves)

### null-ness
* We use **JetBrains** Annotations for specifying null-ness and method contracts.
* If editing a file using a different annotation set (e.g. Javax, Eclipse Sisu, Bukkit)
these should be replaced with their JetBrains equivalent.
* The semantics for JetBrains Annotations are strict _and should be observed!_
* Many IDEs have built-in compiler-level support for these, and can even be set to produce strict
errors when an annotation is misused; do not misuse them.
* **`@NotNull`**
* > An element annotated with NotNull claims null value is forbidden to return (for methods),
pass to (parameters) and hold (local variables and fields).
* Something is `@NotNull` iff it is never null from its inception (new X) to its garbage collection,
i.e. there is no point in time at which the value could ever be null.
* **`@Nullable`**
* > An element annotated with Nullable claims null value is perfectly valid to return (for methods),
> pass to (parameters) or hold in (local variables and fields).
>
> By convention, this annotation applied only when the value should always be checked
> against null because the developer could do nothing to prevent null from happening.
* Something is `@Nullable` iff there is _absolutely no way of determining_ (other than checking its
value `!= null`) whether it is null.
* In other words, if there is another way of knowing (e.g. you set it yourself, an `isPresent` method, etc.)
then it should not be marked nullable.
* **`@Contract`**
* The contract annotation should be used to express other behaviour (e.g. null depending on parameters).
* All fields, method parameters and their return values are non-null by default
- Exceptions: Github API JSON mappings, Metrics
* When something is nullable, mark it as so
* Only ignore nullness errors when a variable is effectively non-null - if in doubt: check
- Most common example is syntax elements, which are not initialised using a constructor
- Exceptions: GitHub API JSON mappings, Metrics
* When ignoring warnings, use the no-inspection comment rather than a blanket suppression annotation
* Use assertions liberally: if you're sure something is not null, assert so to the compiler
- Makes finding bugs easier for developers
* Assertions must **not** have side-effects - they may be skipped in real environments
* Assertions must **not** have side-effects in non-test packages - they may be skipped in real environments
* Avoid checking non-null values for nullability
- Unless working around buggy addons, it is rarely necessary
- This is why ignoring nullness errors is particularly dangerous
- This is why ignoring null-ness errors is particularly dangerous
* Annotations on array types **must** be placed properly:
* Annotations on the array itself go before the array brackets
```java
@Nullable Object @NotNull []
// a not-null array of nullable objects
```
* Annotations on values inside the array go before the value declaration
```java
@NotNull Object @Nullable []
// a nullable array of not-null objects
```
* If this is not adhered to, an IDE may provide incorrect feedback.

### Assertions

Skript must run with assertations enabled; use them in your development environment. \
The JVM flag <code>-ea</code> is used to enable them.

## Code Complexity

Dense, highly-complex code should be avoided to preserve readability and to help with future maintenance,
especially within a single method body.

There are many available metrics for measuring code complexity (for different purposes); [we have our own](https://stable.skriptlang.org/Radical_Complexity.pdf).
There are no strict limits for code complexity, but you may be encouraged (or required) to reformat or break up methods
into smaller, more manageable chunks. If in doubt, keep things simple.

## Minecraft Features

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ org.gradle.parallel=true

groupid=ch.njol
name=skript
version=2.8.5
version=2.8.6
jarName=Skript.jar
testEnv=java21/paper-1.20.6
testEnvJavaVersion=21
16 changes: 0 additions & 16 deletions licenseheader.txt

This file was deleted.

8 changes: 4 additions & 4 deletions src/main/java/ch/njol/skript/SkriptCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,19 @@ public class SkriptCommand implements CommandExecutor {
// TODO /skript scripts show/list - lists all enabled and/or disabled scripts in the scripts folder and/or subfolders (maybe add a pattern [using * and **])
// TODO document this command on the website
private static final CommandHelp SKRIPT_COMMAND_HELP = new CommandHelp("<gray>/<gold>skript", SkriptColor.LIGHT_CYAN, CONFIG_NODE + ".help")
.add(new CommandHelp("reload", SkriptColor.DARK_RED)
.add(new CommandHelp("reload", SkriptColor.DARK_CYAN)
.add("all")
.add("config")
.add("aliases")
.add("scripts")
.add("<script>")
).add(new CommandHelp("enable", SkriptColor.DARK_RED)
).add(new CommandHelp("enable", SkriptColor.DARK_CYAN)
.add("all")
.add("<script>")
).add(new CommandHelp("disable", SkriptColor.DARK_RED)
).add(new CommandHelp("disable", SkriptColor.DARK_CYAN)
.add("all")
.add("<script>")
).add(new CommandHelp("update", SkriptColor.DARK_RED)
).add(new CommandHelp("update", SkriptColor.DARK_CYAN)
.add("check")
.add("changes")
.add("download")
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ch/njol/skript/SkriptConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public class SkriptConfig {
break;
case "stable":
// TODO a better option would be to check that it is not a pre-release through GH API
channel = new ReleaseChannel((name) -> !name.contains("pre"), t);
channel = new ReleaseChannel((name) -> !(name.contains("-")), t);
break;
case "none":
channel = new ReleaseChannel((name) -> false, t);
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/ch/njol/skript/bukkitutil/ItemUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
*/
public class ItemUtils {

public static final boolean HAS_MAX_DAMAGE = Skript.methodExists(Damageable.class, "getMaxDamage");

/**
* Gets damage/durability of an item, or 0 if it does not have damage.
* @param itemStack Item.
Expand All @@ -46,6 +48,18 @@ public static int getDamage(ItemStack itemStack) {
return 0; // Non damageable item
}

/** Gets the max damage/durability of an item
* <p>NOTE: Will account for custom damageable items in MC 1.20.5+</p>
* @param itemStack Item to grab durability from
* @return Max amount of damage this item can take
*/
public static int getMaxDamage(ItemStack itemStack) {
ItemMeta meta = itemStack.getItemMeta();
if (HAS_MAX_DAMAGE && meta instanceof Damageable && ((Damageable) meta).hasMaxDamage())
return ((Damageable) meta).getMaxDamage();
return itemStack.getType().getMaxDurability();
}

/**
* Sets damage/durability of an item if possible.
* @param itemStack Item to modify.
Expand All @@ -72,6 +86,18 @@ public static int getDamage(ItemType itemType) {
return 0; // Non damageable item
}

/** Gets the max damage/durability of an item
* <p>NOTE: Will account for custom damageable items in MC 1.20.5+</p>
* @param itemType Item to grab durability from
* @return Max amount of damage this item can take
*/
public static int getMaxDamage(ItemType itemType) {
ItemMeta meta = itemType.getItemMeta();
if (HAS_MAX_DAMAGE && meta instanceof Damageable && ((Damageable) meta).hasMaxDamage())
return ((Damageable) meta).getMaxDamage();
return itemType.getMaterial().getMaxDurability();
}

/**
* Sets damage/durability of an item if possible.
* @param itemType Item to modify.
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/ch/njol/skript/bukkitutil/PaperEntityUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ public static void lookAt(LookAnchor entityAnchor, Object target, @Nullable Floa
Player player = (Player) entity;
if (target instanceof Vector) {
Vector vector = (Vector) target;
player.lookAt(vector.getX(), vector.getY(), vector.getZ(), LookAnchor.EYES);
player.lookAt(vector.getX(), vector.getY(), vector.getZ(), LookAnchor.FEET);
player.lookAt(player.getEyeLocation().add(vector), LookAnchor.EYES);
player.lookAt(player.getEyeLocation().add(vector), LookAnchor.FEET);
} else if (target instanceof Location) {
player.lookAt((Location) target, LookAnchor.EYES);
player.lookAt((Location) target, LookAnchor.FEET);
Expand Down Expand Up @@ -159,7 +159,7 @@ public void tick() {
switch (type) {
case VECTOR:
Vector vector = ((Vector)target);
mob.lookAt(vector.getX(), vector.getY(), vector.getZ(), speed, maxPitch);
mob.lookAt(mob.getEyeLocation().add(vector), speed, maxPitch);
break;
case LOCATION:
mob.lookAt((Location) target, speed, maxPitch);
Expand Down
Loading

0 comments on commit 0355df1

Please sign in to comment.