Merge remote-tracking branch 'github/dev/feature' into dev/feature

.github/workflows/checkstyle.yml

@@ -0,0 +1,37 @@
+name: checkstyle
+
+on:
+    push:
+        branches:
+            - master
+            - 'dev/**'
+    pull_request:
+
+jobs:
+    build:
+        if: "! contains(toJSON(github.event.commits.*.message), '[ci skip]')"
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/checkout@v4
+              with:
+                  submodules: recursive
+            - name: validate gradle wrapper
+              uses: gradle/wrapper-validation-action@v2
+            - name: Set up JDK 21
+              uses: actions/setup-java@v4
+              with:
+                  java-version: '21'
+                  distribution: 'adopt'
+                  cache: gradle
+            - name: Grant execute permission for gradlew
+              run: chmod +x gradlew
+            - name: Run checkstyle
+              run: ./gradlew clean checkstyleMain
+            - name: Upload checkstyle report
+              uses: actions/upload-artifact@v4
+              if: success()
+              with:
+                  name: checkstyle-report
+                  path: |
+                    build/reports/checkstyle/*.xml
+                    build/reports/checkstyle/*.html

build.gradle

@@ -7,6 +7,7 @@ plugins {
 	id 'com.github.johnrengelman.shadow' version '8.1.1'
 	id 'maven-publish'
 	id 'java'
+	id 'checkstyle'
 }
 
 configurations {
@@ -42,6 +43,11 @@ dependencies {
 	testShadow group: 'org.easymock', name: 'easymock', version: '5.4.0'
 }
 
+checkstyle {
+	configFile = new File("checkstyle.xml")
+	sourceSets = [] // disables checkstyle after build task
+}
+
 task checkAliases {
 	description 'Checks for the existence of the aliases.'
 	doLast {

checkstyle.xml

@@ -0,0 +1,100 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+	"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+	"https://checkstyle.org/dtds/configuration_1_3.dtd">
+
+<module name="Checker">
+
+	<!--Basic Settings-->
+	<!--Warning severity so the builds do not fail because of checkstyle, this is mainly for the GitHub workflow-->
+	<property name="severity" value="warning"/>
+	<property name="fileExtensions" value="java"/>
+	<module name="BeforeExecutionExclusionFileFilter">
+		<property name="fileNamePattern" value="module\-info\.java$"/>
+	</module>
+
+	<!--At most 120 characters per line-->
+	<module name="LineLength">
+		<property name="max" value="120"/>
+	</module>
+
+	<!--New line at the end of the file-->
+	<module name="NewlineAtEndOfFile"/>
+
+	<module name="TreeWalker">
+
+		<!--Tabs, no spaces-->
+		<module name="RegexpSinglelineJava">
+			<property name="format" value="^\t* "/>
+			<property name="message" value="Indent must use tab characters"/>
+			<property name="ignoreComments" value="true"/>
+		</module>
+
+		<!--No trailing whitespace-->
+		<module name="NoWhitespaceAfter" />
+
+		<!--When statements consume multiple lines, all lines but the first have two tabs of additional indentation-->
+		<module name="Indentation">
+			<property name="arrayInitIndent" value="8" />
+			<property name="basicOffset" value="8" />
+			<property name="caseIndent" value="8" />
+			<property name="lineWrappingIndentation" value="8" />
+			<property name="throwsIndent" value="8" />
+		</module>
+
+		<!--Each class begins with an empty line-->
+		<module name="EmptyLineSeparator">
+			<property name="allowNoEmptyLineBetweenFields" value="true" />
+			<property name="tokens"
+					  value="IMPORT, STATIC_IMPORT, CLASS_DEF, INTERFACE_DEF,
+					  		ENUM_DEF, STATIC_INIT, INSTANCE_INIT, METHOD_DEF,
+					  		CTOR_DEF, VARIABLE_DEF, RECORD_DEF, COMPACT_CTOR_DEF" />
+		</module>
+
+		<module name="OneStatementPerLine"/>
+
+		<!--Annotations for a structure go on the line before that structure-->
+		<module name="AnnotationLocation"/>
+
+		<!--When splitting Strings into multiple lines the last part of the string must be (space character included) " " +-->
+		<module name="OperatorWrap">
+			<property name="option" value="eol" />
+			<property name="tokens" value="PLUS" />
+		</module>
+
+		<!--Class names are written in UpperCamelCase-->
+		<module name="TypeName"/>
+
+		<!--Methods named in camelCase-->
+		<module name="MethodName"/>
+
+		<!--Static constant fields should be named in UPPER_SNAKE_CASE-->
+		<module name="ConstantName"/>
+
+		<!--We use JetBrains Annotations for specifying null-ness-->
+		<module name="IllegalImport">
+			<property name="illegalClasses"
+					  value="javax.annotation.Nonnull,
+							javax.annotation.Nullable,
+							org.eclipse.jdt.annotation.NonNull,
+							org.eclipse.jdt.annotation.Nullable,
+							org.eclipse.sisu.Nullable,
+							org.checkerframework.checker.nullness.qual.NonNull,
+							org.checkerframework.checker.nullness.qual.Nullable" />
+			<property name="illegalPkgs" value="" />
+		</module>
+
+		<!--Modules for code improvements-->
+		<module name="MissingOverride"/>
+		<module name="EmptyBlock"/>
+		<module name="HideUtilityClassConstructor"/>
+		<module name="EmptyStatement"/>
+		<module name="EqualsHashCode"/>
+		<module name="SimplifyBooleanExpression"/>
+		<module name="SimplifyBooleanReturn"/>
+		<module name="StringLiteralEquality"/>
+		<module name="UnusedLocalVariable"/>
+
+	</module>
+
+</module>

code-conventions.md

@@ -97,6 +97,7 @@ If we need to remove or alter contributed code due to a licensing issue we will
 	- 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
+* Each Java file ends 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

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.parallel=true
 
 groupid=ch.njol
 name=skript
-version=2.9.2
+version=2.9.3
 jarName=Skript.jar
 testEnv=java21/paper-1.21.0
 testEnvJavaVersion=21

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

@@ -215,7 +215,11 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
 								if (scriptInfo.files == 0) {
 									info(sender, "reload.empty folder", fileName);
 								} else {
-									reloaded(sender, logHandler, timingLogHandler, "x scripts in folder", fileName, scriptInfo.files);
+									if (logHandler.numErrors() == 0) {
+										reloaded(sender, logHandler, timingLogHandler, "x scripts in folder success", fileName, scriptInfo.files);
+									} else {
+										reloaded(sender, logHandler, timingLogHandler, "x scripts in folder error", fileName, scriptInfo.files);
+									}
 								}
 							});
 					}

src/main/java/ch/njol/skript/aliases/Aliases.java

@@ -395,17 +395,17 @@ public static void load() {
 	}
 
 	/**
-	 * Temporarily create an alias for a material which may not have an alias yet.
+	 * Temporarily create an alias for materials which do not have aliases yet.
 	 */
 	private static void loadMissingAliases() {
 		if (!Skript.methodExists(Material.class, "getKey"))
 			return;
 		for (Material material : Material.values()) {
-			if (!provider.hasAliasForMaterial(material)) {
+			if (!material.isLegacy() && !provider.hasAliasForMaterial(material)) {
 				NamespacedKey key = material.getKey();
 				String name = key.getKey().replace("_", " ");
 				parser.loadAlias(name + "¦s", key.toString());
-				Skript.debug(ChatColor.YELLOW + "Creating temporary alias for: " + key.toString());
+				Skript.debug(ChatColor.YELLOW + "Creating temporary alias for: " + key);
 			}
 		}
 	}

src/main/java/ch/njol/skript/aliases/ItemType.java

@@ -1,21 +1,3 @@
-/**
- *   This file is part of Skript.
- *
- *  Skript is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  Skript is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with Skript.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright Peter Güttinger, SkriptLang team and contributors
- */
 package ch.njol.skript.aliases;
 
 import ch.njol.skript.aliases.ItemData.OldItemData;
@@ -41,6 +23,7 @@
 import org.bukkit.Material;
 import org.bukkit.OfflinePlayer;
 import org.bukkit.Tag;
+import org.bukkit.Bukkit;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockState;
 import org.bukkit.block.Skull;
@@ -391,22 +374,35 @@ public boolean hasType() {
 	 */
 	public boolean setBlock(Block block, boolean applyPhysics) {
 		for (int i = random.nextInt(types.size()); i < types.size(); i++) {
-			ItemData d = types.get(i);
-			Material blockType = ItemUtils.asBlock(d.type);
+			ItemData data = types.get(i);
+			Material blockType = ItemUtils.asBlock(data.type);
+
 			if (blockType == null) // Ignore items which cannot be placed
 				continue;
-			if (BlockUtils.set(block, blockType, d.getBlockValues(), applyPhysics)) {
-				ItemMeta itemMeta = getItemMeta();
-				if (itemMeta instanceof SkullMeta) {
-					OfflinePlayer offlinePlayer = ((SkullMeta) itemMeta).getOwningPlayer();
-					if (offlinePlayer == null)
-						continue;
-					Skull skull = (Skull) block.getState();
+
+			if (!BlockUtils.set(block, blockType, data.getBlockValues(), applyPhysics))
+				continue;
+
+			ItemMeta itemMeta = getItemMeta();
+
+			if (itemMeta instanceof SkullMeta) {
+				OfflinePlayer offlinePlayer = ((SkullMeta) itemMeta).getOwningPlayer();
+				if (offlinePlayer == null)
+					continue;
+				Skull skull = (Skull) block.getState();
+				if (offlinePlayer.getName() != null) {
 					skull.setOwningPlayer(offlinePlayer);
-					skull.update(false, applyPhysics);
+				} else if (ItemUtils.CAN_CREATE_PLAYER_PROFILE) {
+					//noinspection deprecation
+					skull.setOwnerProfile(Bukkit.createPlayerProfile(offlinePlayer.getUniqueId(), ""));
+				} else {
+					//noinspection deprecation
+					skull.setOwner("");
 				}
-				return true;
+				skull.update(false, applyPhysics);
 			}
+
+			return true;
 		}
 		return false;
 	}

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

@@ -70,11 +70,7 @@ public class BukkitUnsafe {
 
 	@Nullable
 	public static Material getMaterialFromMinecraftId(String id) {
-		// On 1.13, Vanilla and Spigot names are same
-		if (id.length() > 9)
-			return Material.matchMaterial(id.substring(10)); // Strip 'minecraft:' out
-		else // Malformed material name
-			return null;
+		return Material.matchMaterial(id);
 	}
 
 	public static void modifyItemStack(ItemStack stack, String arguments) {

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

@@ -21,7 +21,9 @@
 import ch.njol.skript.Skript;
 import ch.njol.skript.aliases.ItemType;
 import ch.njol.skript.util.slot.Slot;
+import org.bukkit.Bukkit;
 import org.bukkit.Material;
+import org.bukkit.OfflinePlayer;
 import org.bukkit.Tag;
 import org.bukkit.TreeType;
 import org.bukkit.block.Block;
@@ -32,9 +34,11 @@
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.Damageable;
 import org.bukkit.inventory.meta.ItemMeta;
+import org.bukkit.inventory.meta.SkullMeta;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.HashMap;
+import java.util.UUID;
 
 /**
  * Miscellaneous static utility methods related to items.
@@ -44,6 +48,7 @@ public class ItemUtils {
 	public static final boolean HAS_MAX_DAMAGE = Skript.methodExists(Damageable.class, "getMaxDamage");
 	// Introduced in Paper 1.21
 	public static final boolean HAS_RESET = Skript.methodExists(Damageable.class, "resetDamage");
+	public static final boolean CAN_CREATE_PLAYER_PROFILE = Skript.methodExists(Bukkit.class, "createPlayerProfile", UUID.class, String.class);
 
 	/**
 	 * Gets damage/durability of an item, or 0 if it does not have damage.
@@ -148,6 +153,30 @@ public static void setDamage(ItemType itemType, int damage) {
 		}
 	}
 
+	/**
+	 * Sets the owner of a player head.
+	 * @param skull player head item to modify
+	 * @param player owner of the head
+	 */
+	public static void setHeadOwner(ItemType skull, OfflinePlayer player) {
+		ItemMeta meta = skull.getItemMeta();
+		if (!(meta instanceof SkullMeta))
+			return;
+
+		SkullMeta skullMeta = (SkullMeta) meta;
+
+		if (player.getName() != null) {
+			skullMeta.setOwningPlayer(player);
+		} else if (CAN_CREATE_PLAYER_PROFILE) {
+			//noinspection deprecation
+			skullMeta.setOwnerProfile(Bukkit.createPlayerProfile(player.getUniqueId(), ""));
+		} else {
+			skullMeta.setOwningPlayer(null);
+		}
+
+		skull.setItemMeta(skullMeta);
+	}
+
 	/**
 	 * Gets a block material corresponding to given item material, which might
 	 * be the given material. If no block material is found, null is returned.

src/main/java/ch/njol/skript/classes/data/DefaultOperations.java

@@ -47,11 +47,7 @@ public class DefaultOperations {
 			return left.doubleValue() * right.doubleValue();
 		});
 		Arithmetics.registerOperation(Operator.DIVISION, Number.class, (left, right) -> left.doubleValue() / right.doubleValue());
-		Arithmetics.registerOperation(Operator.EXPONENTIATION, Number.class, (left, right) -> {
-			if (Utils.isInteger(left, right) && right.longValue() >= 0)
-				return (long) Math.pow(left.longValue(), right.longValue());
-			return Math.pow(left.doubleValue(), right.doubleValue());
-		});
+		Arithmetics.registerOperation(Operator.EXPONENTIATION, Number.class, (left, right) -> Math.pow(left.doubleValue(), right.doubleValue()));
 		Arithmetics.registerDifference(Number.class, (left, right) -> {
 			if (Utils.isInteger(left, right))
 				return Math.abs(left.longValue() - right.longValue());

src/main/java/ch/njol/skript/conditions/CondIsWithin.java

@@ -103,15 +103,15 @@ public boolean check(Event event) {
 			Location one = loc1.getSingle(event);
 			Location two = loc2.getSingle(event);
 			if (one == null || two == null || one.getWorld() != two.getWorld())
-				return false;
+				return isNegated();
 			AABB box = new AABB(one, two);
 			return locsToCheck.check(event, box::contains, isNegated());
 		}
 
 		// else, within an entity/block/chunk/world
 		Object area = this.area.getSingle(event);
 		if (area == null)
-			return false;
+			return isNegated();
 
 		// Entities
 		if (area instanceof Entity) {