Merge dev/patch into master (#6750)

build.gradle

@@ -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'
 }
@@ -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
 }
@@ -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
@@ -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) {
@@ -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,
@@ -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,
@@ -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(

code-conventions.md

@@ -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.
@@ -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 " +
@@ -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
@@ -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
@@ -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
 

gradle.properties

@@ -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

src/main/java/ch/njol/skript/SkriptCommand.java

@@ -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")

src/main/java/ch/njol/skript/SkriptConfig.java

@@ -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);

src/main/java/ch/njol/skript/bukkitutil/ItemUtils.java

@@ -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.
@@ -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.
@@ -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.

src/main/java/ch/njol/skript/bukkitutil/PaperEntityUtils.java

@@ -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);
@@ -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);