Merge branch 'retrooper:2.0' into 2.0

.github/workflows/gradle-publish.yml

@@ -8,54 +8,57 @@ jobs:
     runs-on: ubuntu-latest
     permissions:
       contents: read
-      packages: write
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
+
+      - name: Validate gradle wrapper
+        uses: gradle/actions/wrapper-validation@v3
 
       - name: Set up JDK 21
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           java-version: '21'
           distribution: 'adopt'
+          cache: 'gradle'
           server-id: github # Value of the distributionManagement/repository/id field of the pom.xml
           settings-path: ${{ github.workspace }} # location for the settings.xml file
 
       - name: Build with Gradle
         run: chmod +x gradlew && ./gradlew build
 
       - name: Upload build artifacts api
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: api-build
           path: api/build/libs
 
       - name: Upload build artifacts spigot
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: spigot-build
           path: spigot/build/libs
 
       - name: Upload build artifacts netty-common
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: netty-common-build
           path: netty-common/build/libs
 
       - name: Upload build artifacts bungeecord
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: bungeecord-build
           path: bungeecord/build/libs
 
       - name: Upload build artifacts velocity
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: velocity-build
           path: velocity/build/libs
 
       - name: Upload build artifacts fabric
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: fabric-build
-          path: fabric/build/libs
+          path: fabric/build/libs

README.md

@@ -8,7 +8,7 @@ our [Discord Server](https://discord.me/packetevents) for help.
 ### Links
 * [Find release builds on Modrinth](https://modrinth.com/plugin/packetevents)
 * [Find development builds on Jenkins](https://ci.codemc.io/job/retrooper/job/packetevents)
-* [View documentation on our JavaDocs Page](https://packetevents.github.io/javadocs)
+* [View the JavaDocs Page](https://packetevents.github.io)
 
 ### Setup
 [How to Setup PacketEvents](https://github.com/retrooper/packetevents/wiki/Depending-on-pre%E2%80%90built-PacketEvents)

api/build.gradle.kts

@@ -21,6 +21,7 @@ java {
 
 dependencies {
     compileOnlyApi(libs.bundles.adventure)
+    compileOnlyApi(libs.bundles.adventure.serializers)
     implementation(libs.adventure.api)
     api(project(":patch:adventure-text-serializer-gson", "shadow")) {
         excludeAdventure()

api/src/main/java/com/github/retrooper/packetevents/PacketEventsAPI.java

@@ -34,22 +34,22 @@
 import java.util.logging.Logger;
 
 public abstract class PacketEventsAPI<T> {
-    private static final EventManager EVENT_MANAGER = new EventManager();
-    private static final PacketEventsSettings SETTINGS = new PacketEventsSettings();
-    private static final UpdateChecker UPDATE_CHECKER = new UpdateChecker();
-    private static final LogManager LOG_MANAGER = new LogManager();
+    private final EventManager eventManager = new EventManager();
+    private final PacketEventsSettings settings = new PacketEventsSettings();
+    private final UpdateChecker updateChecker = new UpdateChecker();
+    private final LogManager logManager = new LogManager();
     private static final Logger LOGGER = Logger.getLogger(PacketEventsAPI.class.getName());
 
     public EventManager getEventManager() {
-        return EVENT_MANAGER;
+        return eventManager;
     }
 
     public PacketEventsSettings getSettings() {
-        return SETTINGS;
+        return settings;
     }
 
     public UpdateChecker getUpdateChecker() {
-        return UPDATE_CHECKER;
+        return updateChecker;
     }
 
     public PEVersion getVersion() {
@@ -62,7 +62,7 @@ public Logger getLogger() {
     }
 
     public LogManager getLogManager() {
-        return LOG_MANAGER;
+        return logManager;
     }
 
     public abstract void load();

api/src/main/java/com/github/retrooper/packetevents/event/EventManager.java

@@ -24,11 +24,35 @@
 
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.logging.Level;
 
+/**
+ * Class for event managing. Implements both, internal and API methods.
+ *
+ * @apiNote It is highly recommended to register the event listeners at server start-up,
+ * without frequent modifications during runtime, since this class needs to recalculate all of
+ * its event listeners during any modifications. In other words, it assumes that reads (events fired)
+ * greatly outnumber writes (listeners modifications). If your case requires frequent listener modifications,
+ * open a pull request on GitHub and describe your case.
+ *
+ * @author retrooper
+ * @author ShadowOfHeaven (optimization)
+ *
+ */
+
 public class EventManager {
-    private final Map<PacketListenerPriority, Set<PacketListenerCommon>> listenersMap =
-            Collections.synchronizedMap(new EnumMap<>(PacketListenerPriority.class));
+
+    //Using a ConcurrentHashMap is faster and more secure here, compared to Collections.synchronizedMap(new EnumMap<>(PacketListenerPriority.class))
+    //This is mainly due to:
+    //1. On each modification Collections.synchronizedMap synchronizes the whole Map object, while ConcurrentHashMap only it's internal, currently modified Node
+    //2. ConcurrentHashMap won't fail in a multi-thread environment, while Collections.synchronizedMap is said to have a lot of potential problems,
+    //being a generalized method for synchronization
+    private final Map<PacketListenerPriority, Set<PacketListenerCommon>> listenersMap = new ConcurrentHashMap<>();
+    //Since reads greatly outnumber writes, create an array for the best possible iteration time
+    //Updated as a whole on writes, no index modifications are allowed
+    private volatile PacketListenerCommon[] listeners = new PacketListenerCommon[0];
+
 
     /**
      * Call the PacketEvent.
@@ -43,32 +67,43 @@ public void callEvent(PacketEvent event) {
         callEvent(event, null);
     }
 
+
+    /**
+     * Call the PacketEvent.
+     * This method processes the event on all the registered dynamic packet event listeners.
+     * The {@link PacketListenerPriority#LOWEST} prioritized listeners will be processing first,
+     * the {@link PacketListenerPriority#MONITOR} will be processing last and can
+     * be the final decider whether the event has been cancelled or not.
+     *
+     * @param event                  {@link PacketEvent}
+     * @param postCallListenerAction The action to be ran after all the listeners have finished processing
+     */
     public void callEvent(PacketEvent event, @Nullable Runnable postCallListenerAction) {
-        for (PacketListenerPriority priority : PacketListenerPriority.values()) {
-            Set<PacketListenerCommon> listeners = listenersMap.get(priority);
-            if (listeners != null) {
-                for (PacketListenerCommon listener : listeners) {
-                    try {
-                        event.call(listener);
-                    } catch (Exception t) {
-                        // ignore handshake exceptions
-                        if (t.getClass() != InvalidHandshakeException.class) {
-                            PacketEvents.getAPI().getLogger().log(Level.WARNING, "PacketEvents caught an unhandled exception while calling your listener.", t);
-                        }
-                    }
-                    if (postCallListenerAction != null) {
-                        postCallListenerAction.run();
-                    }
+        for (PacketListenerCommon listener : listeners) {
+            try {
+                event.call(listener);
+            } catch (Exception t) {
+                // ignore handshake exceptions
+                if (t.getClass() != InvalidHandshakeException.class) {
+                    PacketEvents.getAPI().getLogger().log(Level.WARNING, "PacketEvents caught an unhandled exception while calling your listener.", t);
                 }
             }
+            if (postCallListenerAction != null) {
+                postCallListenerAction.run();
+            }
         }
         // For performance reasons, we don't want to re-encode the packet if it's not needed.
         if (event instanceof ProtocolPacketEvent && !((ProtocolPacketEvent<?>) event).needsReEncode()) {
             ((ProtocolPacketEvent<?>) event).setLastUsedWrapper(null);
         }
-
     }
 
+    /**
+     * Register the dynamic packet event listener.
+     *
+     * @param listener {@link PacketListenerCommon}
+     * @param priority {@link PacketListenerPriority}
+     */
     public PacketListenerCommon registerListener(PacketListener listener, PacketListenerPriority priority) {
         PacketListenerCommon packetListenerAbstract = listener.asAbstract(priority);
         return registerListener(packetListenerAbstract);
@@ -80,12 +115,8 @@ public PacketListenerCommon registerListener(PacketListener listener, PacketList
      * @param listener {@link PacketListenerCommon}
      */
     public PacketListenerCommon registerListener(PacketListenerCommon listener) {
-        Set<PacketListenerCommon> listenerSet = listenersMap.get(listener.getPriority());
-        if (listenerSet == null) {
-            listenerSet = ConcurrentHashMap.newKeySet();
-        }
-        listenerSet.add(listener);
-        listenersMap.put(listener.getPriority(), listenerSet);
+        this.registerListenerNoRecalculation(listener);
+        this.recalculateListeners();
         return listener;
     }
 
@@ -96,28 +127,59 @@ public PacketListenerCommon registerListener(PacketListenerCommon listener) {
      */
     public PacketListenerCommon[] registerListeners(PacketListenerCommon... listeners) {
         for (PacketListenerCommon listener : listeners) {
-            registerListener(listener);
+            this.registerListenerNoRecalculation(listener);
         }
+        this.recalculateListeners();
         return listeners;
     }
 
     public void unregisterListener(PacketListenerCommon listener) {
-        Set<PacketListenerCommon> listenerSet = listenersMap.get(listener.getPriority());
-        if (listenerSet == null) return;
-        listenerSet.remove(listener);
+        if (this.unregisterListenerNoRecalculation(listener)) this.recalculateListeners();
     }
 
     public void unregisterListeners(PacketListenerCommon... listeners) {
+        boolean modified = false;
         for (PacketListenerCommon listener : listeners) {
-            unregisterListener(listener);
+            modified |= this.unregisterListenerNoRecalculation(listener);//OR - at least one of these needs to be true to return true, meaning the boolean will permanently stay true if once set so
         }
+        if (modified) this.recalculateListeners();
     }
 
 
     /**
      * Unregister all dynamic packet event listeners.
      */
     public void unregisterAllListeners() {
-        listenersMap.clear();
+        this.listenersMap.clear();
+        synchronized (this) {//like booky10 said, the synchronization is necessary here
+            this.listeners = new PacketListenerCommon[0];
+        }
+    }
+
+    //Needs to be synchronized in order to avoid race conditions (for example where the 'listeners' variable
+    //is overridden by its non-up-to-date value, simply because it finished a bit later than the most recent update)
+    private void recalculateListeners() {
+        synchronized (this) {
+            List<PacketListenerCommon> list = new ArrayList<>();
+            //adds from LOWEST to MONITOR, so in the correct order
+            for (PacketListenerPriority priority : PacketListenerPriority.values()) {
+                Set<PacketListenerCommon> set = this.listenersMap.get(priority);
+                if (set != null) list.addAll(set);
+            }
+            this.listeners = list.toArray(new PacketListenerCommon[0]);
+        }
+    }
+
+    //Internal registration methods, specifically separated for lesser overhead when registering an array of Listeners
+
+    private void registerListenerNoRecalculation(PacketListenerCommon listener) {
+        Set<PacketListenerCommon> listenerSet = this.listenersMap.computeIfAbsent(listener.getPriority(), p -> new CopyOnWriteArraySet<>());
+        listenerSet.add(listener);
+    }
+
+    //Returns true if the listener was removed, so a modification occurred
+    private boolean unregisterListenerNoRecalculation(PacketListenerCommon listener) {
+        Set<PacketListenerCommon> listenerSet = this.listenersMap.get(listener.getPriority());
+        return listenerSet != null && listenerSet.remove(listener);
     }
 }

api/src/main/java/com/github/retrooper/packetevents/event/PacketListenerPriority.java

@@ -30,8 +30,7 @@
  */
 public enum PacketListenerPriority {
     /**
-     * Listener is of very low importance.
-     * This listener will be ran first.
+     * This listener will be run first and has little say in the outcome of events.
      */
     LOWEST,
 
@@ -41,8 +40,11 @@ public enum PacketListenerPriority {
     LOW,
 
     /**
-     * Default listener priority.
-     * Listener is neither important nor unimportant and may run normally.
+     * The normal listener priority.
+     * If possible, always pick this.
+     * It allows other projects to easily overturn your decisions.
+     * Moreover, it is pretty bold to assume that your project should
+     * always have the final say.
      */
     NORMAL,
 
@@ -52,13 +54,13 @@ public enum PacketListenerPriority {
     HIGH,
 
     /**
-     * Listener is of critical importance and wants to decide the cancellation of an event.
+     * Listener is of critical importance. Use this to decide the final state of packets.
      */
     HIGHEST,
 
     /**
-     * Listener is purely trying to decide the cancellation of an event.
-     * This listener should be ran last.
+     * Only use this priority if you want to perform logic based on the outcome of an event.
+     * Please do not modify packets in this stage.
      */
     MONITOR;