From 93cbd52313340aab295acf052fc32795ef8a3e59 Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 24 Aug 2021 15:12:39 +0200 Subject: [PATCH 01/56] Use arrays instead of lists for loops --- .../minestom/server/event/EventNodeImpl.java | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index e6179a0a7..bf759a991 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -44,8 +44,8 @@ class EventNodeImpl implements EventNode { final Handle castedHandle = (Handle) handle; Check.argCondition(castedHandle.node != this, "Invalid handle owner"); if (!castedHandle.updated) castedHandle.update(); - final List> listeners = castedHandle.listeners; - if (listeners.isEmpty()) return; + final Consumer[] listeners = castedHandle.listeners; + if (listeners.length == 0) return; for (Consumer listener : listeners) { listener.accept(event); } @@ -62,7 +62,7 @@ class EventNodeImpl implements EventNode { public boolean hasListener(@NotNull ListenerHandle handle) { final Handle castedHandle = (Handle) handle; if (!castedHandle.updated) castedHandle.update(); - return !castedHandle.listeners.isEmpty(); + return castedHandle.listeners.length > 0; } @Override @@ -277,7 +277,8 @@ class EventNodeImpl implements EventNode { private static final class Handle implements ListenerHandle { private final EventNodeImpl node; private final Class eventType; - private final List> listeners = new CopyOnWriteArrayList<>(); + private Consumer[] listeners = new Consumer[0]; + private final List> listenersCache = new ArrayList<>(); private volatile boolean updated; Handle(EventNodeImpl node, Class eventType) { @@ -287,8 +288,9 @@ class EventNodeImpl implements EventNode { void update() { synchronized (GLOBAL_CHILD_LOCK) { - this.listeners.clear(); + this.listenersCache.clear(); recursiveUpdate(node); + this.listeners = listenersCache.toArray(Consumer[]::new); this.updated = true; } } @@ -345,16 +347,16 @@ class EventNodeImpl implements EventNode { if (size == 1) { final var firstFilter = filterList.get(0); // Common case where there is only one filter - this.listeners.add(event -> mapper.accept(firstFilter, event)); + this.listenersCache.add(event -> mapper.accept(firstFilter, event)); } else if (size == 2) { final var firstFilter = filterList.get(0); final var secondFilter = filterList.get(1); - this.listeners.add(event -> { + this.listenersCache.add(event -> { mapper.accept(firstFilter, event); mapper.accept(secondFilter, event); }); } else { - this.listeners.add(event -> { + this.listenersCache.add(event -> { for (var filter : filterList) { mapper.accept(filter, event); } @@ -374,20 +376,21 @@ class EventNodeImpl implements EventNode { final var predicate = targetNode.predicate; final boolean hasPredicate = predicate != null; - final List> listenersCopy = List.copyOf(entry.listeners); - final List> bindingsCopy = List.copyOf(entry.bindingConsumers); - if (!hasPredicate && listenersCopy.isEmpty() && bindingsCopy.isEmpty()) + final EventListener[] listenersCopy = entry.listeners.toArray(EventListener[]::new); + final Consumer[] bindingsCopy = entry.bindingConsumers.toArray(Consumer[]::new); + final boolean listenersEmpty = listenersCopy.length == 0; + final boolean bindingsEmpty = bindingsCopy.length == 0; + if (!hasPredicate && listenersEmpty && bindingsEmpty) return; // Nothing to run - if (!hasPredicate && bindingsCopy.isEmpty() && listenersCopy.size() == 1) { + if (!hasPredicate && bindingsEmpty && listenersCopy.length == 1) { // Only one normal listener - final EventListener listener = listenersCopy.get(0); - this.listeners.add(e -> callListener(targetNode, listener, e)); + final EventListener listener = listenersCopy[0]; + this.listenersCache.add(e -> callListener(targetNode, listener, e)); return; } - // Worse case scenario, try to run everything - this.listeners.add(e -> { + this.listenersCache.add(e -> { if (hasPredicate) { final Object value = filter.getHandler(e); try { @@ -397,12 +400,12 @@ class EventNodeImpl implements EventNode { return; } } - if (!listenersCopy.isEmpty()) { + if (!listenersEmpty) { for (EventListener listener : listenersCopy) { callListener(targetNode, listener, e); } } - if (!bindingsCopy.isEmpty()) { + if (!bindingsEmpty) { for (Consumer eConsumer : bindingsCopy) { try { eConsumer.accept(e); From aed441123ed225f70f698a6658312ab0567f03aa Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 24 Aug 2021 15:25:11 +0200 Subject: [PATCH 02/56] Marks some methods as experimental --- src/main/java/net/minestom/server/event/EventNode.java | 3 +++ src/main/java/net/minestom/server/event/ListenerHandle.java | 1 + 2 files changed, 4 insertions(+) diff --git a/src/main/java/net/minestom/server/event/EventNode.java b/src/main/java/net/minestom/server/event/EventNode.java index a89d64713..8bb6b6127 100644 --- a/src/main/java/net/minestom/server/event/EventNode.java +++ b/src/main/java/net/minestom/server/event/EventNode.java @@ -202,6 +202,7 @@ public interface EventNode { * @param the event type * @throws IllegalArgumentException if {@code handle}'s owner is not {@code this} */ + @ApiStatus.Experimental void call(@NotNull E event, @NotNull ListenerHandle handle); /** @@ -211,6 +212,7 @@ public interface EventNode { * @param the event type * @return the handle linked to {@code handleType} */ + @ApiStatus.Experimental @NotNull ListenerHandle getHandle(@NotNull Class handleType); /** @@ -224,6 +226,7 @@ public interface EventNode { * @param handle the listener handle * @return true if the event has 1 or more listeners */ + @ApiStatus.Experimental boolean hasListener(@NotNull ListenerHandle handle); /** diff --git a/src/main/java/net/minestom/server/event/ListenerHandle.java b/src/main/java/net/minestom/server/event/ListenerHandle.java index 8ddf7d3bf..9e610d19a 100644 --- a/src/main/java/net/minestom/server/event/ListenerHandle.java +++ b/src/main/java/net/minestom/server/event/ListenerHandle.java @@ -8,6 +8,7 @@ import org.jetbrains.annotations.ApiStatus; * * @param the event type */ +@ApiStatus.Experimental @ApiStatus.NonExtendable public interface ListenerHandle { } From 5ddd97cee24d6fa8d6e5f820f46bcb307e9b1485 Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 24 Aug 2021 15:35:09 +0200 Subject: [PATCH 03/56] Move optimized event calling inside ListenerHandle --- .../net/minestom/server/entity/Entity.java | 2 +- .../net/minestom/server/entity/Player.java | 2 +- .../server/event/EventDispatcher.java | 4 -- .../net/minestom/server/event/EventNode.java | 30 +-------------- .../minestom/server/event/EventNodeImpl.java | 37 +++++++++---------- .../minestom/server/event/ListenerHandle.java | 14 +++++++ .../net/minestom/server/instance/Chunk.java | 2 +- .../minestom/server/instance/Instance.java | 2 +- .../server/instance/InstanceContainer.java | 2 +- .../listener/PlayerPositionListener.java | 3 +- .../manager/PacketListenerManager.java | 3 +- 11 files changed, 39 insertions(+), 62 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Entity.java b/src/main/java/net/minestom/server/entity/Entity.java index 07f2218f4..41082744d 100644 --- a/src/main/java/net/minestom/server/entity/Entity.java +++ b/src/main/java/net/minestom/server/entity/Entity.java @@ -449,7 +449,7 @@ public class Entity implements Viewable, Tickable, TagHandler, PermissionHandler update(time); ticks++; - EventDispatcher.call(new EntityTickEvent(this), GlobalHandles.ENTITY_TICK); + GlobalHandles.ENTITY_TICK.call(new EntityTickEvent(this)); // remove expired effects effectTick(time); diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index a3070ceed..aba439774 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -351,7 +351,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, } // Tick event - EventDispatcher.call(new PlayerTickEvent(this), GlobalHandles.PLAYER_TICK); + GlobalHandles.PLAYER_TICK.call(new PlayerTickEvent(this)); } @Override diff --git a/src/main/java/net/minestom/server/event/EventDispatcher.java b/src/main/java/net/minestom/server/event/EventDispatcher.java index 742736e4e..6d0738ef6 100644 --- a/src/main/java/net/minestom/server/event/EventDispatcher.java +++ b/src/main/java/net/minestom/server/event/EventDispatcher.java @@ -10,10 +10,6 @@ public final class EventDispatcher { MinecraftServer.getGlobalEventHandler().call(event); } - public static void call(@NotNull E event, @NotNull ListenerHandle handle) { - MinecraftServer.getGlobalEventHandler().call(event, handle); - } - public static ListenerHandle getHandle(@NotNull Class handleType) { return MinecraftServer.getGlobalEventHandler().getHandle(handleType); } diff --git a/src/main/java/net/minestom/server/event/EventNode.java b/src/main/java/net/minestom/server/event/EventNode.java index 8bb6b6127..621031355 100644 --- a/src/main/java/net/minestom/server/event/EventNode.java +++ b/src/main/java/net/minestom/server/event/EventNode.java @@ -188,23 +188,9 @@ public interface EventNode { */ default void call(@NotNull T event) { //noinspection unchecked - call(event, getHandle((Class) event.getClass())); + getHandle((Class) event.getClass()).call(event); } - /** - * Calls an event starting from this node. - *

- * The event handle can be retrieved using {@link #getHandle(Class)} - * and is useful to avoid map lookups for high-frequency events. - * - * @param event the event to call - * @param handle the event handle linked to this node - * @param the event type - * @throws IllegalArgumentException if {@code handle}'s owner is not {@code this} - */ - @ApiStatus.Experimental - void call(@NotNull E event, @NotNull ListenerHandle handle); - /** * Gets the handle of an event type. * @@ -215,20 +201,6 @@ public interface EventNode { @ApiStatus.Experimental @NotNull ListenerHandle getHandle(@NotNull Class handleType); - /** - * Gets if any listener has been registered for the given handle. - * May trigger an update if the cached data is not correct. - *

- * Useful if you are able to avoid expensive computation in the case where - * the event is unused. Be aware that {@link #call(Event, ListenerHandle)} - * has similar optimization built-in. - * - * @param handle the listener handle - * @return true if the event has 1 or more listeners - */ - @ApiStatus.Experimental - boolean hasListener(@NotNull ListenerHandle handle); - /** * Execute a cancellable event with a callback to execute if the event is successful. * Event conditions and propagation is the same as {@link #call(Event)}. diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index bf759a991..b646be2b4 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -39,18 +39,6 @@ class EventNodeImpl implements EventNode { this.eventType = filter.eventType(); } - @Override - public void call(@NotNull E event, @NotNull ListenerHandle handle) { - final Handle castedHandle = (Handle) handle; - Check.argCondition(castedHandle.node != this, "Invalid handle owner"); - if (!castedHandle.updated) castedHandle.update(); - final Consumer[] listeners = castedHandle.listeners; - if (listeners.length == 0) return; - for (Consumer listener : listeners) { - listener.accept(event); - } - } - @Override public @NotNull ListenerHandle getHandle(@NotNull Class handleType) { //noinspection unchecked @@ -58,13 +46,6 @@ class EventNodeImpl implements EventNode { aClass -> new Handle<>(this, (Class) aClass)); } - @Override - public boolean hasListener(@NotNull ListenerHandle handle) { - final Handle castedHandle = (Handle) handle; - if (!castedHandle.updated) castedHandle.update(); - return castedHandle.listeners.length > 0; - } - @Override public @NotNull List> findChildren(@NotNull String name, Class eventType) { synchronized (GLOBAL_CHILD_LOCK) { @@ -286,6 +267,22 @@ class EventNodeImpl implements EventNode { this.eventType = eventType; } + @Override + public void call(@NotNull E event) { + if (!updated) update(); + final Consumer[] listeners = this.listeners; + if (listeners.length == 0) return; + for (Consumer listener : listeners) { + listener.accept(event); + } + } + + @Override + public boolean hasListener() { + if (!updated) update(); + return listeners.length > 0; + } + void update() { synchronized (GLOBAL_CHILD_LOCK) { this.listenersCache.clear(); @@ -325,7 +322,7 @@ class EventNodeImpl implements EventNode { for (var mappedEntry : mappedNodeCache.entrySet()) { final EventNodeImpl mappedNode = mappedEntry.getValue(); final Handle handle = (Handle) mappedNode.getHandle(eventType); - if (!mappedNode.hasListener(handle)) continue; // Implicit update + if (!handle.hasListener()) continue; // Implicit update filters.add(mappedNode.filter); handlers.put(mappedEntry.getKey(), handle); } diff --git a/src/main/java/net/minestom/server/event/ListenerHandle.java b/src/main/java/net/minestom/server/event/ListenerHandle.java index 9e610d19a..eb3f82362 100644 --- a/src/main/java/net/minestom/server/event/ListenerHandle.java +++ b/src/main/java/net/minestom/server/event/ListenerHandle.java @@ -1,6 +1,7 @@ package net.minestom.server.event; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; /** * Represents a key to access an {@link EventNode} listeners. @@ -11,4 +12,17 @@ import org.jetbrains.annotations.ApiStatus; @ApiStatus.Experimental @ApiStatus.NonExtendable public interface ListenerHandle { + void call(@NotNull E event); + + /** + * Gets if any listener has been registered for the given handle. + * May trigger an update if the cached data is not correct. + *

+ * Useful if you are able to avoid expensive computation in the case where + * the event is unused. Be aware that {@link #call(Event)} + * has similar optimization built-in. + * + * @return true if the event has 1 or more listeners + */ + boolean hasListener(); } diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index 7c486c0a7..0e4f0aa68 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/src/main/java/net/minestom/server/instance/Chunk.java @@ -280,7 +280,7 @@ public abstract class Chunk implements BlockGetter, BlockSetter, Viewable, Ticka if (result) { PlayerChunkLoadEvent playerChunkLoadEvent = new PlayerChunkLoadEvent(player, chunkX, chunkZ); - EventDispatcher.call(playerChunkLoadEvent, GlobalHandles.PLAYER_CHUNK_LOAD); + GlobalHandles.PLAYER_CHUNK_LOAD.call(playerChunkLoadEvent); } return result; diff --git a/src/main/java/net/minestom/server/instance/Instance.java b/src/main/java/net/minestom/server/instance/Instance.java index 222cee95e..46991b268 100644 --- a/src/main/java/net/minestom/server/instance/Instance.java +++ b/src/main/java/net/minestom/server/instance/Instance.java @@ -732,7 +732,7 @@ public abstract class Instance implements BlockGetter, BlockSetter, Tickable, Ta // Tick event { // Process tick events - EventDispatcher.call(new InstanceTickEvent(this, time, lastTickAge), GlobalHandles.INSTANCE_TICK); + GlobalHandles.INSTANCE_TICK.call(new InstanceTickEvent(this, time, lastTickAge)); // Set last tick age this.lastTickAge = time; } diff --git a/src/main/java/net/minestom/server/instance/InstanceContainer.java b/src/main/java/net/minestom/server/instance/InstanceContainer.java index ed177b272..52f3fd3ae 100644 --- a/src/main/java/net/minestom/server/instance/InstanceContainer.java +++ b/src/main/java/net/minestom/server/instance/InstanceContainer.java @@ -275,7 +275,7 @@ public class InstanceContainer extends Instance { .whenComplete((chunk, throwable) -> { // TODO run in the instance thread? cacheChunk(chunk); - EventDispatcher.call(new InstanceChunkLoadEvent(this, chunkX, chunkZ), GlobalHandles.INSTANCE_CHUNK_LOAD); + GlobalHandles.INSTANCE_CHUNK_LOAD.call(new InstanceChunkLoadEvent(this, chunkX, chunkZ)); synchronized (loadingChunks) { this.loadingChunks.remove(ChunkUtils.getChunkIndex(chunk)); } diff --git a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java index 9ca1e1f3d..52132c004 100644 --- a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java +++ b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java @@ -2,7 +2,6 @@ package net.minestom.server.listener; import net.minestom.server.coordinate.Pos; import net.minestom.server.entity.Player; -import net.minestom.server.event.EventDispatcher; import net.minestom.server.event.GlobalHandles; import net.minestom.server.event.player.PlayerMoveEvent; import net.minestom.server.instance.Instance; @@ -54,7 +53,7 @@ public class PlayerPositionListener { } PlayerMoveEvent playerMoveEvent = new PlayerMoveEvent(player, newPosition); - EventDispatcher.call(playerMoveEvent, GlobalHandles.PLAYER_MOVE); + GlobalHandles.PLAYER_MOVE.call(playerMoveEvent); // True if the event call changed the player position (possibly a teleport) if (!playerMoveEvent.isCancelled() && currentPosition.equals(player.getPosition())) { // Move the player diff --git a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java index e75d83e5a..b0ec7d267 100644 --- a/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java +++ b/src/main/java/net/minestom/server/listener/manager/PacketListenerManager.java @@ -2,7 +2,6 @@ package net.minestom.server.listener.manager; import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; -import net.minestom.server.event.EventDispatcher; import net.minestom.server.event.GlobalHandles; import net.minestom.server.event.player.PlayerPacketEvent; import net.minestom.server.listener.*; @@ -90,7 +89,7 @@ public final class PacketListenerManager { // Event PlayerPacketEvent playerPacketEvent = new PlayerPacketEvent(player, packet); - EventDispatcher.call(playerPacketEvent, GlobalHandles.PLAYER_PACKET); + GlobalHandles.PLAYER_PACKET.call(playerPacketEvent); if (playerPacketEvent.isCancelled()) { return; } From 748b24b8ef7efe56e9991e5fec9b75f818f18ef8 Mon Sep 17 00:00:00 2001 From: TheMode Date: Wed, 25 Aug 2021 18:26:45 +0200 Subject: [PATCH 04/56] Expand block properties error --- src/main/java/net/minestom/server/instance/block/BlockImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/instance/block/BlockImpl.java b/src/main/java/net/minestom/server/instance/block/BlockImpl.java index df311d2f4..a39048b62 100644 --- a/src/main/java/net/minestom/server/instance/block/BlockImpl.java +++ b/src/main/java/net/minestom/server/instance/block/BlockImpl.java @@ -173,7 +173,7 @@ final class BlockImpl implements Block { private Block compute(Map properties) { Block block = propertyEntry.get(properties); if (block == null) - throw new IllegalArgumentException("Invalid properties: " + properties); + throw new IllegalArgumentException("Invalid properties: " + properties + " for block " + this); return nbt == null && handler == null ? block : new BlockImpl(block.registry(), propertyEntry, block.properties(), nbt, handler); } From 811e3c542b80b23434f93d2426e30d94feb8f382 Mon Sep 17 00:00:00 2001 From: LeoDog896 Date: Wed, 25 Aug 2021 18:35:51 -0400 Subject: [PATCH 05/56] Fix up tab completion for EntityType --- .../registry/ArgumentEntityType.java | 2 +- .../java/demo/commands/SummonCommand.java | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/minestom/server/command/builder/arguments/minecraft/registry/ArgumentEntityType.java b/src/main/java/net/minestom/server/command/builder/arguments/minecraft/registry/ArgumentEntityType.java index def764fc1..6e2a2749d 100644 --- a/src/main/java/net/minestom/server/command/builder/arguments/minecraft/registry/ArgumentEntityType.java +++ b/src/main/java/net/minestom/server/command/builder/arguments/minecraft/registry/ArgumentEntityType.java @@ -23,7 +23,7 @@ public class ArgumentEntityType extends ArgumentRegistry { @Override public void processNodes(@NotNull NodeMaker nodeMaker, boolean executable) { DeclareCommandsPacket.Node argumentNode = simpleArgumentNode(this, executable, false, true); - argumentNode.parser = "minecraft:entity_summon"; + argumentNode.parser = "minecraft:resource_location"; argumentNode.suggestionsType = SuggestionType.SUMMONABLE_ENTITIES.getIdentifier(); nodeMaker.addNodes(new DeclareCommandsPacket.Node[]{argumentNode}); diff --git a/src/test/java/demo/commands/SummonCommand.java b/src/test/java/demo/commands/SummonCommand.java index 3440476bb..2ef48afa7 100644 --- a/src/test/java/demo/commands/SummonCommand.java +++ b/src/test/java/demo/commands/SummonCommand.java @@ -3,32 +3,39 @@ package demo.commands; import net.minestom.server.command.CommandSender; import net.minestom.server.command.builder.Command; import net.minestom.server.command.builder.CommandContext; +import net.minestom.server.command.builder.arguments.Argument; import net.minestom.server.command.builder.arguments.ArgumentEnum; import net.minestom.server.command.builder.arguments.ArgumentType; import net.minestom.server.command.builder.arguments.minecraft.registry.ArgumentEntityType; import net.minestom.server.command.builder.arguments.relative.ArgumentRelativeVec3; import net.minestom.server.command.builder.condition.Conditions; +import net.minestom.server.coordinate.Vec; import net.minestom.server.entity.Entity; import net.minestom.server.entity.EntityCreature; import net.minestom.server.entity.EntityType; import net.minestom.server.entity.LivingEntity; +import net.minestom.server.utils.location.RelativeVec; import org.jetbrains.annotations.NotNull; public class SummonCommand extends Command { private final ArgumentEntityType entity; - private final ArgumentRelativeVec3 pos; - private final ArgumentEnum entityClass; + private final Argument pos; + private final Argument entityClass; public SummonCommand() { super("summon"); setCondition(Conditions::playerOnly); entity = ArgumentType.EntityType("entity type"); - pos = ArgumentType.RelativeVec3("pos"); - entityClass = ArgumentType.Enum("class", EntityClass.class); - entityClass.setFormat(ArgumentEnum.Format.LOWER_CASED); - entityClass.setDefaultValue(EntityClass.CREATURE); + pos = ArgumentType.RelativeVec3("pos").setDefaultValue(() -> new RelativeVec( + new Vec(0, 0, 0), + RelativeVec.CoordinateType.RELATIVE, + true, true, true + )); + entityClass = ArgumentType.Enum("class", EntityClass.class) + .setFormat(ArgumentEnum.Format.LOWER_CASED) + .setDefaultValue(EntityClass.CREATURE);; addSyntax(this::execute, entity, pos, entityClass); setDefaultExecutor((sender, context) -> sender.sendMessage("Usage: /summon ")); } From d56479d316c382ed7db17c3539a1c375bfb6df6b Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 26 Aug 2021 13:24:16 +0200 Subject: [PATCH 06/56] Ensure that child also invalidate events Signed-off-by: TheMode --- src/main/java/net/minestom/server/event/EventNodeImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index b646be2b4..3b116c573 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -220,6 +220,10 @@ class EventNodeImpl implements EventNode { for (Class eventType : listenerMap.keySet()) { node.invalidateEvent(eventType); } + // TODO bindings? + for (EventNodeImpl child : children) { + child.invalidateEventsFor(node); + } } private void invalidateEvent(Class eventClass) { From 66681c5caff0a6e4e85523d4b496904990554f8e Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 26 Aug 2021 14:25:13 +0200 Subject: [PATCH 07/56] Use an array for filtering --- .../net/minestom/server/event/EventNodeImpl.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index 3b116c573..f9e77546b 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -333,8 +333,7 @@ class EventNodeImpl implements EventNode { // If at least one mapped node listen to this handle type, // loop through them and forward to mapped node if there is a match if (!filters.isEmpty()) { - final var filterList = List.copyOf(filters); - final int size = filterList.size(); + final EventFilter[] filterList = filters.toArray(EventFilter[]::new); final BiConsumer, E> mapper = (filter, event) -> { final Object handler = filter.castHandler(event); final Handle handle = handlers.get(handler); @@ -345,13 +344,13 @@ class EventNodeImpl implements EventNode { } } }; - if (size == 1) { - final var firstFilter = filterList.get(0); + if (filterList.length == 1) { + final var firstFilter = filterList[0]; // Common case where there is only one filter this.listenersCache.add(event -> mapper.accept(firstFilter, event)); - } else if (size == 2) { - final var firstFilter = filterList.get(0); - final var secondFilter = filterList.get(1); + } else if (filterList.length == 2) { + final var firstFilter = filterList[0]; + final var secondFilter = filterList[1]; this.listenersCache.add(event -> { mapper.accept(firstFilter, event); mapper.accept(secondFilter, event); From 6eac7282afccfd1a524e2fabadbef62f69cb98d8 Mon Sep 17 00:00:00 2001 From: kiipy <25848425+kiipy@users.noreply.github.com> Date: Mon, 2 Aug 2021 13:02:29 +0200 Subject: [PATCH 08/56] Made Player#setPermissionLevel persistent after death. --- src/main/java/net/minestom/server/entity/Player.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index aba439774..df55efa90 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -150,7 +150,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, private BelowNameTag belowNameTag; - private int permissionLevel; + private byte permissionLevel; private boolean reducedDebugScreenInformation; @@ -418,6 +418,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, getPlayerConnection().sendPacket(respawnPacket); PlayerRespawnEvent respawnEvent = new PlayerRespawnEvent(this); EventDispatcher.call(respawnEvent); + triggerStatus((byte) (24 + permissionLevel)); // Set permission level refreshIsDead(false); // Runnable called when teleportation is successful (after loading and sending necessary chunk) @@ -1590,7 +1591,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, public void setPermissionLevel(int permissionLevel) { Check.argCondition(!MathUtils.isBetween(permissionLevel, 0, 4), "permissionLevel has to be between 0 and 4"); - this.permissionLevel = permissionLevel; + this.permissionLevel = (byte) permissionLevel; // Magic values: https://wiki.vg/Entity_statuses#Player // TODO remove magic values From 3ce51ff4705005beae2920e268a994550588098c Mon Sep 17 00:00:00 2001 From: Matt Worzala Date: Thu, 26 Aug 2021 11:10:48 -0400 Subject: [PATCH 09/56] switch permission level back to an int internally --- src/main/java/net/minestom/server/entity/Player.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index df55efa90..038acc530 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -150,7 +150,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, private BelowNameTag belowNameTag; - private byte permissionLevel; + private int permissionLevel; private boolean reducedDebugScreenInformation; @@ -1591,7 +1591,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, public void setPermissionLevel(int permissionLevel) { Check.argCondition(!MathUtils.isBetween(permissionLevel, 0, 4), "permissionLevel has to be between 0 and 4"); - this.permissionLevel = (byte) permissionLevel; + this.permissionLevel = permissionLevel; // Magic values: https://wiki.vg/Entity_statuses#Player // TODO remove magic values From 2674ca933ef7caabe1fa216785efc4185c58bae4 Mon Sep 17 00:00:00 2001 From: Matthew Date: Sun, 15 Aug 2021 13:13:48 +0200 Subject: [PATCH 10/56] Move functionality from `TaskBuilder#schedule` to `Task#schedule` and `Task#scheduleForShutdown` --- .../java/net/minestom/server/timer/Task.java | 14 +++++++++++++ .../minestom/server/timer/TaskBuilder.java | 21 ++----------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index a97d1ca1c..69d7d5ad9 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -100,6 +100,10 @@ public class Task implements Runnable { * Sets up the task for correct execution. */ public void schedule() { + Int2ObjectMap tasks = this.schedulerManager.tasks; + synchronized (tasks) { + tasks.put(getId(), this); + } if(owningExtension != null) { this.schedulerManager.onScheduleFromExtension(owningExtension, this); } @@ -108,6 +112,16 @@ public class Task implements Runnable { this.schedulerManager.getTimerExecutionService().scheduleAtFixedRate(this, this.delay, this.repeat, TimeUnit.MILLISECONDS); } + public void scheduleForShutdown() { + Int2ObjectMap shutdownTasks = this.schedulerManager.shutdownTasks; + synchronized (shutdownTasks) { + shutdownTasks.put(getId(), this); + } + if (owningExtension != null) { + this.schedulerManager.onScheduleShutdownFromExtension(owningExtension, this); + } + } + /** * Gets the current status of the task. * diff --git a/src/main/java/net/minestom/server/timer/TaskBuilder.java b/src/main/java/net/minestom/server/timer/TaskBuilder.java index d8cf7e6d5..cf527bf1e 100644 --- a/src/main/java/net/minestom/server/timer/TaskBuilder.java +++ b/src/main/java/net/minestom/server/timer/TaskBuilder.java @@ -144,27 +144,10 @@ public class TaskBuilder { */ @NotNull public Task schedule() { - Task task = new Task( - this.schedulerManager, - this.runnable, - this.shutdown, - this.delay, - this.repeat, - this.isTransient, - this.owningExtension); + Task task = build(); if (this.shutdown) { - Int2ObjectMap shutdownTasks = this.schedulerManager.shutdownTasks; - synchronized (shutdownTasks) { - shutdownTasks.put(task.getId(), task); - } - if (owningExtension != null) { - this.schedulerManager.onScheduleShutdownFromExtension(owningExtension, task); - } + task.scheduleForShutdown(); } else { - Int2ObjectMap tasks = this.schedulerManager.tasks; - synchronized (tasks) { - tasks.put(task.getId(), task); - } task.schedule(); } return task; From 1c92414cad7e2b4b9cecffc1b9930483379470f4 Mon Sep 17 00:00:00 2001 From: Matthew Date: Sun, 15 Aug 2021 13:14:38 +0200 Subject: [PATCH 11/56] Add `TaskBuilder#build` --- .../minestom/server/timer/TaskBuilder.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/timer/TaskBuilder.java b/src/main/java/net/minestom/server/timer/TaskBuilder.java index cf527bf1e..178d70549 100644 --- a/src/main/java/net/minestom/server/timer/TaskBuilder.java +++ b/src/main/java/net/minestom/server/timer/TaskBuilder.java @@ -138,11 +138,28 @@ public class TaskBuilder { } /** - * Schedules this {@link Task} for execution. + * Builds a {@link Task}. * * @return the built {@link Task} */ @NotNull + public Task build() { + return new Task( + this.schedulerManager, + this.runnable, + this.shutdown, + this.delay, + this.repeat, + this.isTransient, + this.owningExtension); + } + + /** + * Schedules this {@link Task} for execution. + * + * @return the scheduled {@link Task} + */ + @NotNull public Task schedule() { Task task = build(); if (this.shutdown) { From 75964f87b4c047d911b5cd65b13b1987b459dbee Mon Sep 17 00:00:00 2001 From: Matthew Date: Mon, 23 Aug 2021 19:37:10 +0200 Subject: [PATCH 12/56] Merge `Task#scheduleForShutdown` into `Task#schedule` --- .../java/net/minestom/server/timer/Task.java | 38 +++++++++---------- .../minestom/server/timer/TaskBuilder.java | 6 +-- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index 69d7d5ad9..8b7e8218a 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -100,25 +100,25 @@ public class Task implements Runnable { * Sets up the task for correct execution. */ public void schedule() { - Int2ObjectMap tasks = this.schedulerManager.tasks; - synchronized (tasks) { - tasks.put(getId(), this); - } - if(owningExtension != null) { - this.schedulerManager.onScheduleFromExtension(owningExtension, this); - } - this.future = this.repeat == 0L ? - this.schedulerManager.getTimerExecutionService().schedule(this, this.delay, TimeUnit.MILLISECONDS) : - this.schedulerManager.getTimerExecutionService().scheduleAtFixedRate(this, this.delay, this.repeat, TimeUnit.MILLISECONDS); - } - - public void scheduleForShutdown() { - Int2ObjectMap shutdownTasks = this.schedulerManager.shutdownTasks; - synchronized (shutdownTasks) { - shutdownTasks.put(getId(), this); - } - if (owningExtension != null) { - this.schedulerManager.onScheduleShutdownFromExtension(owningExtension, this); + if(this.shutdown) { + Int2ObjectMap shutdownTasks = this.schedulerManager.shutdownTasks; + synchronized (shutdownTasks) { + shutdownTasks.put(getId(), this); + } + if (owningExtension != null) { + this.schedulerManager.onScheduleShutdownFromExtension(owningExtension, this); + } + } else { + Int2ObjectMap tasks = this.schedulerManager.tasks; + synchronized (tasks) { + tasks.put(getId(), this); + } + if (owningExtension != null) { + this.schedulerManager.onScheduleFromExtension(owningExtension, this); + } + this.future = this.repeat == 0L ? + this.schedulerManager.getTimerExecutionService().schedule(this, this.delay, TimeUnit.MILLISECONDS) : + this.schedulerManager.getTimerExecutionService().scheduleAtFixedRate(this, this.delay, this.repeat, TimeUnit.MILLISECONDS); } } diff --git a/src/main/java/net/minestom/server/timer/TaskBuilder.java b/src/main/java/net/minestom/server/timer/TaskBuilder.java index 178d70549..f1387f0c7 100644 --- a/src/main/java/net/minestom/server/timer/TaskBuilder.java +++ b/src/main/java/net/minestom/server/timer/TaskBuilder.java @@ -162,11 +162,7 @@ public class TaskBuilder { @NotNull public Task schedule() { Task task = build(); - if (this.shutdown) { - task.scheduleForShutdown(); - } else { - task.schedule(); - } + task.schedule(); return task; } } From b60b785dcd4170663ec99b27113919243f3ae437 Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 26 Aug 2021 17:59:51 +0200 Subject: [PATCH 13/56] Fix unhandled exception Signed-off-by: TheMode --- src/main/java/net/minestom/server/entity/Player.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 038acc530..246c0618d 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -1196,7 +1196,11 @@ public class Player extends LivingEntity implements CommandSender, Localizable, // Cannot load chunk (auto load is not enabled) return; } - chunk.addViewer(this); + try { + chunk.addViewer(this); + } catch (Exception e) { + MinecraftServer.getExceptionManager().handleException(e); + } }); }); } From f7cd0def20ab9f17dc744320b2d03ad31a920c8d Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 26 Aug 2021 18:18:35 +0200 Subject: [PATCH 14/56] Add dedicated `CachedPacket` for future network optimizations Signed-off-by: TheMode --- .../server/instance/DynamicChunk.java | 41 ++++++++----------- .../server/network/packet/CachedPacket.java | 31 ++++++++++++++ 2 files changed, 49 insertions(+), 23 deletions(-) create mode 100644 src/main/java/net/minestom/server/network/packet/CachedPacket.java diff --git a/src/main/java/net/minestom/server/instance/DynamicChunk.java b/src/main/java/net/minestom/server/instance/DynamicChunk.java index 46fd1adb2..c38eea11d 100644 --- a/src/main/java/net/minestom/server/instance/DynamicChunk.java +++ b/src/main/java/net/minestom/server/instance/DynamicChunk.java @@ -9,13 +9,13 @@ import net.minestom.server.entity.Player; import net.minestom.server.entity.pathfinding.PFBlock; import net.minestom.server.instance.block.Block; import net.minestom.server.instance.block.BlockHandler; +import net.minestom.server.network.packet.CachedPacket; import net.minestom.server.network.packet.FramedPacket; import net.minestom.server.network.packet.server.play.ChunkDataPacket; import net.minestom.server.network.packet.server.play.UpdateLightPacket; import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.network.player.PlayerSocketConnection; import net.minestom.server.utils.ArrayUtils; -import net.minestom.server.utils.PacketUtils; import net.minestom.server.utils.chunk.ChunkUtils; import net.minestom.server.world.biomes.Biome; import org.jetbrains.annotations.NotNull; @@ -40,10 +40,8 @@ public class DynamicChunk extends Chunk { protected final Int2ObjectOpenHashMap tickableMap = new Int2ObjectOpenHashMap<>(); private volatile long lastChangeTime; - - private FramedPacket cachedChunkBuffer; - private FramedPacket cachedLightBuffer; - private long cachedPacketTime; + private final CachedPacket chunkCache = new CachedPacket(this::createChunkPacket); + private final CachedPacket lightCache = new CachedPacket(this::createLightPacket); public DynamicChunk(@NotNull Instance instance, @Nullable Biome[] biomes, int chunkX, int chunkZ) { super(instance, biomes, chunkX, chunkZ, true); @@ -126,34 +124,31 @@ public class DynamicChunk extends Chunk { } @Override - public synchronized void sendChunk(@NotNull Player player) { + public void sendChunk(@NotNull Player player) { if (!isLoaded()) return; final PlayerConnection connection = player.getPlayerConnection(); + final long lastChange = getLastChangeTime(); + final FramedPacket lightPacket = lightCache.retrieveFramedPacket(lastChange); + final FramedPacket chunkPacket = chunkCache.retrieveFramedPacket(lastChange); if (connection instanceof PlayerSocketConnection) { - final long lastChange = getLastChangeTime(); - var chunkPacket = cachedChunkBuffer; - var lightPacket = cachedLightBuffer; - if (lastChange > cachedPacketTime || (chunkPacket == null || lightPacket == null)) { - chunkPacket = PacketUtils.allocateTrimmedPacket(createChunkPacket()); - lightPacket = PacketUtils.allocateTrimmedPacket(createLightPacket()); - this.cachedChunkBuffer = chunkPacket; - this.cachedLightBuffer = lightPacket; - this.cachedPacketTime = lastChange; - } PlayerSocketConnection socketConnection = (PlayerSocketConnection) connection; - socketConnection.write(lightPacket); - socketConnection.write(chunkPacket); + socketConnection.write(lightPacket.body()); + socketConnection.write(chunkPacket.body()); } else { - connection.sendPacket(createLightPacket()); - connection.sendPacket(createChunkPacket()); + connection.sendPacket(lightPacket.packet()); + connection.sendPacket(chunkPacket.packet()); } } @Override - public synchronized void sendChunk() { + public void sendChunk() { if (!isLoaded()) return; - sendPacketToViewers(createLightPacket()); - sendPacketToViewers(createChunkPacket()); + if (getViewers().isEmpty()) return; + final long lastChange = getLastChangeTime(); + final FramedPacket lightPacket = lightCache.retrieveFramedPacket(lastChange); + final FramedPacket chunkPacket = chunkCache.retrieveFramedPacket(lastChange); + sendPacketToViewers(lightPacket.packet()); + sendPacketToViewers(chunkPacket.packet()); } @NotNull diff --git a/src/main/java/net/minestom/server/network/packet/CachedPacket.java b/src/main/java/net/minestom/server/network/packet/CachedPacket.java new file mode 100644 index 000000000..d06654424 --- /dev/null +++ b/src/main/java/net/minestom/server/network/packet/CachedPacket.java @@ -0,0 +1,31 @@ +package net.minestom.server.network.packet; + +import net.minestom.server.network.packet.server.ServerPacket; +import net.minestom.server.utils.PacketUtils; +import org.jetbrains.annotations.ApiStatus; + +import java.lang.ref.SoftReference; +import java.util.function.Supplier; + +@ApiStatus.Internal +public final class CachedPacket { + private final Supplier supplier; + private volatile long packetTimestamp; + private SoftReference packet; + + public CachedPacket(Supplier supplier) { + this.supplier = supplier; + } + + public FramedPacket retrieveFramedPacket(long lastChange) { + final long timestamp = packetTimestamp; + final var ref = packet; + FramedPacket cache = ref != null ? ref.get() : null; + if (cache == null || lastChange > timestamp) { + cache = PacketUtils.allocateTrimmedPacket(supplier.get()); + this.packet = new SoftReference<>(cache); + this.packetTimestamp = lastChange; + } + return cache; + } +} From 42938111d5529b908aafb6430f1a26231d2eaedb Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Thu, 26 Aug 2021 19:50:28 +0200 Subject: [PATCH 15/56] Allow tasks to be bound to the same thread for each run. --- .../map/framebuffers/GLFWCapableBuffer.java | 2 + .../server/timer/SchedulerManager.java | 19 +++- .../java/net/minestom/server/timer/Task.java | 30 ++++-- .../minestom/server/timer/TaskBuilder.java | 18 +++- .../utils/thread/ThreadBindingExecutor.java | 92 +++++++++++++++++++ 5 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java diff --git a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java index 92dc949f0..18dfb866f 100644 --- a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java +++ b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java @@ -68,6 +68,7 @@ public abstract class GLFWCapableBuffer { } public void changeRenderingThreadToCurrent() { + System.out.println("Currently on thread "+Thread.currentThread().getId()); glfwMakeContextCurrent(glfwWindow); GL.createCapabilities(); } @@ -90,6 +91,7 @@ public abstract class GLFWCapableBuffer { render(rendering); } }) + .bindToSingleThread() .repeat(period) .schedule(); } diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index bddfd29b8..698044727 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -7,6 +7,7 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.extensions.Extension; import net.minestom.server.extensions.IExtensionObserver; import net.minestom.server.utils.thread.MinestomThread; +import net.minestom.server.utils.thread.ThreadBindingExecutor; import org.jetbrains.annotations.NotNull; import java.util.Collection; @@ -35,6 +36,8 @@ public final class SchedulerManager implements IExtensionObserver { private final AtomicInteger shutdownCounter; //A threaded execution private final ExecutorService batchesPool; + //Thread execution, which always uses the same Thread for a given Task + private final ExecutorService threadBindingPool; // A single threaded scheduled execution private final ScheduledExecutorService timerExecutionService; // All the registered tasks (task id = task) @@ -66,6 +69,7 @@ public final class SchedulerManager implements IExtensionObserver { this.shutdownCounter = new AtomicInteger(); this.batchesPool = new MinestomThread(MinecraftServer.THREAD_COUNT_SCHEDULER, MinecraftServer.THREAD_NAME_SCHEDULER); + this.threadBindingPool = new ThreadBindingExecutor(MinecraftServer.THREAD_COUNT_SCHEDULER, MinecraftServer.THREAD_NAME_SCHEDULER); this.timerExecutionService = Executors.newSingleThreadScheduledExecutor(); this.tasks = new Int2ObjectOpenHashMap<>(); this.shutdownTasks = new Int2ObjectOpenHashMap<>(); @@ -183,15 +187,26 @@ public final class SchedulerManager implements IExtensionObserver { } /** - * Gets the execution service for all the registered {@link Task}. + * Gets the execution service for all the registered {@link Task}, which are not marked as thread-bound. * - * @return the execution service for all the registered {@link Task} + * @return the execution service for all the registered {@link Task}, which are not marked as thread-bound */ @NotNull public ExecutorService getBatchesPool() { return batchesPool; } + /** + * Gets the execution service for all the registered {@link Task}, which are marked as thread-bound. + * The thread to which tasks are assigned depends on their runnable hashcode. Two (or more) tasks can be bound to the same Thread. + * + * @return the execution service for all the registered {@link Task}, which are marked as thread-bound + */ + @NotNull + public ExecutorService getThreadBindingPool() { + return threadBindingPool; + } + /** * Gets the scheduled execution service for all the registered {@link Task}. * diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index 8b7e8218a..2e3d40e7d 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -6,6 +6,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Objects; +import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -42,6 +43,12 @@ public class Task implements Runnable { private ScheduledFuture future; // The thread of the task private volatile Thread currentThreadTask; + // The executor service used for this task + private final ExecutorService executorService; + // Whether this task will always execute on the same thread + private final boolean boundToSingleThread; + // Action executed on the executor. Stored inside the Task to avoid changing the hashcode (which ThreadBindingExecutor relies on) + private final Runnable action; /** * Creates a task. @@ -51,24 +58,21 @@ public class Task implements Runnable { * @param shutdown Defines whether the task is a shutdown task * @param delay The time to delay * @param repeat The time until the repetition + * @param bindToSingleThread Whether to run the given task always on the same thread. */ - public Task(@NotNull SchedulerManager schedulerManager, @NotNull Runnable runnable, boolean shutdown, long delay, long repeat, boolean isTransient, @Nullable String owningExtension) { + public Task(@NotNull SchedulerManager schedulerManager, @NotNull Runnable runnable, boolean shutdown, long delay, long repeat, boolean isTransient, @Nullable String owningExtension, boolean bindToSingleThread) { this.schedulerManager = schedulerManager; this.runnable = runnable; this.shutdown = shutdown; this.id = shutdown ? this.schedulerManager.getShutdownCounterIdentifier() : this.schedulerManager.getCounterIdentifier(); + this.executorService = bindToSingleThread ? this.schedulerManager.getThreadBindingPool() : this.schedulerManager.getBatchesPool(); this.delay = delay; this.repeat = repeat; this.isTransient = isTransient; + this.boundToSingleThread = bindToSingleThread; this.owningExtension = owningExtension; - } - /** - * Executes the task. - */ - @Override - public void run() { - this.schedulerManager.getBatchesPool().execute(() -> { + this.action = () -> { this.currentThreadTask = Thread.currentThread(); try { this.runnable.run(); @@ -84,7 +88,15 @@ public class Task implements Runnable { if (this.repeat == 0) this.finish(); this.currentThreadTask = null; } - }); + }; + } + + /** + * Executes the task. + */ + @Override + public void run() { + executorService.execute(action); } /** diff --git a/src/main/java/net/minestom/server/timer/TaskBuilder.java b/src/main/java/net/minestom/server/timer/TaskBuilder.java index f1387f0c7..287e3d3b9 100644 --- a/src/main/java/net/minestom/server/timer/TaskBuilder.java +++ b/src/main/java/net/minestom/server/timer/TaskBuilder.java @@ -36,6 +36,12 @@ public class TaskBuilder { */ private boolean isTransient; + /** + * Should this Task be run on the same thread for each run? Can be used for situations where context is bound to a + * single thread. (for instance OpenGL context) + */ + private boolean boundToSingleThread = false; + /** * Creates a task builder. *
@@ -137,6 +143,15 @@ public class TaskBuilder { return this; } + /** + * Makes this Task be run on the same thread for each run. Can be used for situations where context is bound to a + * single thread. (for instance OpenGL context) + */ + public TaskBuilder bindToSingleThread() { + boundToSingleThread = true; + return this; + } + /** * Builds a {@link Task}. * @@ -151,7 +166,8 @@ public class TaskBuilder { this.delay, this.repeat, this.isTransient, - this.owningExtension); + this.owningExtension, + this.boundToSingleThread); } /** diff --git a/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java b/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java new file mode 100644 index 000000000..3b8c119cb --- /dev/null +++ b/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java @@ -0,0 +1,92 @@ +package net.minestom.server.utils.thread; + +import org.jetbrains.annotations.NotNull; + +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.AbstractExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * Executor service which will always give the same thread to a given Runnable. + * Uses {@link Runnable#hashCode()} to determine the thread to assign. + */ +public class ThreadBindingExecutor extends AbstractExecutorService { + + private MinestomThread[] threadExecutors; + + /** + * Creates a non-local thread-binding executor + * + * @param nThreads the number of threads + * @param name the name of the thread pool + */ + public ThreadBindingExecutor(int nThreads, String name) { + this(nThreads, name, false); + } + + /** + * @param nThreads the number of threads + * @param name the name of the thread pool + * @param local set to true if this executor is only used inside a method and should *not* be kept in the internal list of executors + */ + public ThreadBindingExecutor(int nThreads, String name, boolean local) { + threadExecutors = new MinestomThread[nThreads]; + for (int i = 0; i < nThreads; i++) { + threadExecutors[i] = new MinestomThread(1, name, local); + } + } + + @Override + public void shutdown() { + for (MinestomThread t : threadExecutors) { + t.shutdown(); + } + } + + @NotNull + @Override + public List shutdownNow() { + List allTasks = new LinkedList<>(); + for (MinestomThread t : threadExecutors) { + allTasks.addAll(t.shutdownNow()); + } + return allTasks; + } + + @Override + public boolean isShutdown() { + for (MinestomThread t : threadExecutors) { + if(!t.isShutdown()) + return false; + } + return true; + } + + @Override + public boolean isTerminated() { + for (MinestomThread t : threadExecutors) { + if(!t.isShutdown()) + return false; + } + return true; + } + + @Override + public boolean awaitTermination(long timeout, @NotNull TimeUnit unit) throws InterruptedException { + boolean terminated = true; + for (MinestomThread t : threadExecutors) { + terminated &= t.awaitTermination(timeout, unit); + } + return terminated; + } + + @Override + public void execute(@NotNull Runnable command) { + int hash = command.hashCode(); + if(hash < 0) hash = -hash; + int bucket = hash % threadExecutors.length; + + threadExecutors[bucket].execute(command); + } +} From 4588c707c9deda99de94ccf8e10aae18a6d11958 Mon Sep 17 00:00:00 2001 From: MrGazdag <44264503+MrGazdag@users.noreply.github.com> Date: Thu, 26 Aug 2021 23:17:01 +0200 Subject: [PATCH 16/56] test --- .github/workflows/main.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/main.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 000000000..41257c6e8 --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,25 @@ +# This is a basic workflow to help you get started with Actions + +name: Trigger Jitpack Build + +# Controls when the workflow will run +on: + # Triggers the workflow on push or pull request events but only for the master branch + push: + branches: [ master ] + + # Allows you to run this workflow manually from the Actions tab + workflow_dispatch: + +# A workflow run is made up of one or more jobs that can run sequentially or in parallel +jobs: + # This workflow contains a single job called "build" + build: + # The type of runner that the job will run on + runs-on: ubuntu-latest + + # Steps represent a sequence of tasks that will be executed as part of the job + steps: + # Runs a single command using the runners shell + - name: Trigger Jitpack Build + run: curl "https://jitpack.io/com/github/Minestom/Minestom/${${github.sha}:0:10}/build.log" From f74f8753db46665cc59e10feee89f05bec3af44c Mon Sep 17 00:00:00 2001 From: MrGazdag <44264503+MrGazdag@users.noreply.github.com> Date: Thu, 26 Aug 2021 23:18:45 +0200 Subject: [PATCH 17/56] fix thing --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 41257c6e8..3696e606c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,4 +22,4 @@ jobs: steps: # Runs a single command using the runners shell - name: Trigger Jitpack Build - run: curl "https://jitpack.io/com/github/Minestom/Minestom/${${github.sha}:0:10}/build.log" + run: curl "https://jitpack.io/com/github/Minestom/Minestom/${github.sha:0:10}/build.log" From c508f86b45db91299ddccf5f8897b1aacaf032ae Mon Sep 17 00:00:00 2001 From: MrGazdag <44264503+MrGazdag@users.noreply.github.com> Date: Thu, 26 Aug 2021 23:19:35 +0200 Subject: [PATCH 18/56] try fix thing? --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3696e606c..692edc432 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,4 +22,4 @@ jobs: steps: # Runs a single command using the runners shell - name: Trigger Jitpack Build - run: curl "https://jitpack.io/com/github/Minestom/Minestom/${github.sha:0:10}/build.log" + run: curl "https://jitpack.io/com/github/Minestom/Minestom/${GITHUB_SHA:0:10}/build.log" From c3bcdf3796868bd5b09d9a2b3fc45652fdacdd27 Mon Sep 17 00:00:00 2001 From: MrGazdag <44264503+MrGazdag@users.noreply.github.com> Date: Thu, 26 Aug 2021 23:22:34 +0200 Subject: [PATCH 19/56] Rename main.yml to trigger-jitpack-build.yml --- .github/workflows/{main.yml => trigger-jitpack-build.yml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/workflows/{main.yml => trigger-jitpack-build.yml} (100%) diff --git a/.github/workflows/main.yml b/.github/workflows/trigger-jitpack-build.yml similarity index 100% rename from .github/workflows/main.yml rename to .github/workflows/trigger-jitpack-build.yml From e90907ade026746941ba1097d2aa89748dc00316 Mon Sep 17 00:00:00 2001 From: MrGazdag <44264503+MrGazdag@users.noreply.github.com> Date: Thu, 26 Aug 2021 23:23:01 +0200 Subject: [PATCH 20/56] remove comments --- .github/workflows/trigger-jitpack-build.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/.github/workflows/trigger-jitpack-build.yml b/.github/workflows/trigger-jitpack-build.yml index 692edc432..8366c4f6f 100644 --- a/.github/workflows/trigger-jitpack-build.yml +++ b/.github/workflows/trigger-jitpack-build.yml @@ -1,25 +1,12 @@ -# This is a basic workflow to help you get started with Actions - name: Trigger Jitpack Build - -# Controls when the workflow will run on: - # Triggers the workflow on push or pull request events but only for the master branch push: branches: [ master ] - # Allows you to run this workflow manually from the Actions tab workflow_dispatch: - -# A workflow run is made up of one or more jobs that can run sequentially or in parallel jobs: - # This workflow contains a single job called "build" build: - # The type of runner that the job will run on runs-on: ubuntu-latest - - # Steps represent a sequence of tasks that will be executed as part of the job steps: - # Runs a single command using the runners shell - name: Trigger Jitpack Build run: curl "https://jitpack.io/com/github/Minestom/Minestom/${GITHUB_SHA:0:10}/build.log" From 5c9c57439ac121cc3713df455c05ea0b76249332 Mon Sep 17 00:00:00 2001 From: TheMode Date: Fri, 27 Aug 2021 09:51:36 +0200 Subject: [PATCH 21/56] Prevent node update from being called multiple times Signed-off-by: TheMode --- .../java/net/minestom/server/event/EventNodeImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index f9e77546b..ebd882c15 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -273,7 +273,7 @@ class EventNodeImpl implements EventNode { @Override public void call(@NotNull E event) { - if (!updated) update(); + update(); final Consumer[] listeners = this.listeners; if (listeners.length == 0) return; for (Consumer listener : listeners) { @@ -283,12 +283,14 @@ class EventNodeImpl implements EventNode { @Override public boolean hasListener() { - if (!updated) update(); + update(); return listeners.length > 0; } void update() { + if (updated) return; synchronized (GLOBAL_CHILD_LOCK) { + if (updated) return; this.listenersCache.clear(); recursiveUpdate(node); this.listeners = listenersCache.toArray(Consumer[]::new); @@ -338,7 +340,7 @@ class EventNodeImpl implements EventNode { final Object handler = filter.castHandler(event); final Handle handle = handlers.get(handler); if (handle != null) { // Run the listeners of the mapped node - if (!handle.updated) handle.update(); + handle.update(); for (Consumer listener : handle.listeners) { listener.accept(event); } From 4680dbd64ec7a42d68d80881e801cfcfd01369a3 Mon Sep 17 00:00:00 2001 From: TheMode Date: Fri, 27 Aug 2021 09:55:10 +0200 Subject: [PATCH 22/56] Reuse call method for mapped nodes Signed-off-by: TheMode --- src/main/java/net/minestom/server/event/EventNodeImpl.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index ebd882c15..aeece586d 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -339,12 +339,7 @@ class EventNodeImpl implements EventNode { final BiConsumer, E> mapper = (filter, event) -> { final Object handler = filter.castHandler(event); final Handle handle = handlers.get(handler); - if (handle != null) { // Run the listeners of the mapped node - handle.update(); - for (Consumer listener : handle.listeners) { - listener.accept(event); - } - } + if (handle != null) handle.call(event); }; if (filterList.length == 1) { final var firstFilter = filterList[0]; From 34ba838ab538907e23fd6440263065bff2134d96 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sat, 28 Aug 2021 11:28:14 +0200 Subject: [PATCH 23/56] Improve movement cancelling --- .../net/minestom/server/entity/Player.java | 6 ++++- .../listener/PlayerPositionListener.java | 26 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 246c0618d..dd32d7506 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -1553,6 +1553,10 @@ public class Player extends LivingEntity implements CommandSender, Localizable, playerConnection.sendPacket(new UpdateViewPositionPacket(chunkX, chunkZ)); } + public int getNextTeleportId() { + return teleportId.incrementAndGet(); + } + public int getLastSentTeleportId() { return teleportId.get(); } @@ -1572,7 +1576,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable, @ApiStatus.Internal protected void synchronizePosition(boolean includeSelf) { if (includeSelf) { - playerConnection.sendPacket(new PlayerPositionAndLookPacket(position, (byte) 0x00, teleportId.incrementAndGet(), false)); + playerConnection.sendPacket(new PlayerPositionAndLookPacket(position, (byte) 0x00, getNextTeleportId(), false)); } super.synchronizePosition(includeSelf); } diff --git a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java index 52132c004..d8fba54ff 100644 --- a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java +++ b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java @@ -6,6 +6,8 @@ import net.minestom.server.event.GlobalHandles; import net.minestom.server.event.player.PlayerMoveEvent; import net.minestom.server.instance.Instance; import net.minestom.server.network.packet.client.play.*; +import net.minestom.server.network.packet.server.play.PlayerPositionAndLookPacket; +import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.utils.chunk.ChunkUtils; import org.jetbrains.annotations.NotNull; @@ -54,14 +56,28 @@ public class PlayerPositionListener { PlayerMoveEvent playerMoveEvent = new PlayerMoveEvent(player, newPosition); GlobalHandles.PLAYER_MOVE.call(playerMoveEvent); - // True if the event call changed the player position (possibly a teleport) - if (!playerMoveEvent.isCancelled() && currentPosition.equals(player.getPosition())) { - // Move the player + if (!currentPosition.equals(player.getPosition())) { + // Player has been teleported in the event + return; + } + if (playerMoveEvent.isCancelled()) { + // Teleport to previous position + PlayerConnection connection = player.getPlayerConnection(); + connection.sendPacket(new PlayerPositionAndLookPacket(currentPosition, (byte) 0x00, player.getNextTeleportId(), false)); + return; + } + final Pos eventPosition = playerMoveEvent.getNewPosition(); + if (newPosition.equals(eventPosition)) { + // Event didn't change the position player.refreshPosition(playerMoveEvent.getNewPosition()); player.refreshOnGround(onGround); } else { - // Cancelled, teleport to previous position - player.teleport(player.getPosition()); + // Position modified by the event + if (newPosition.samePoint(eventPosition)) { + player.setView(eventPosition.yaw(), eventPosition.pitch()); + } else { + player.teleport(eventPosition); + } } } } From 9739403ff442771cb5c06fde6757c48434b86652 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sat, 28 Aug 2021 11:37:42 +0200 Subject: [PATCH 24/56] Properly update the client position when switching vehicle --- .../java/net/minestom/server/entity/Entity.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/entity/Entity.java b/src/main/java/net/minestom/server/entity/Entity.java index 41082744d..b48d42532 100644 --- a/src/main/java/net/minestom/server/entity/Entity.java +++ b/src/main/java/net/minestom/server/entity/Entity.java @@ -882,16 +882,15 @@ public class Entity implements Viewable, Tickable, TagHandler, PermissionHandler public void addPassenger(@NotNull Entity entity) { Check.stateCondition(instance == null, "You need to set an instance using Entity#setInstance"); Check.stateCondition(entity == getVehicle(), "Cannot add the entity vehicle as a passenger"); - final Entity vehicle = entity.getVehicle(); if (vehicle != null) { vehicle.removePassenger(entity); } - this.passengers.add(entity); entity.vehicle = this; - sendPacketToViewersAndSelf(getPassengersPacket()); + entity.refreshPosition(position); + entity.synchronizePosition(true); } /** @@ -903,11 +902,14 @@ public class Entity implements Viewable, Tickable, TagHandler, PermissionHandler */ public void removePassenger(@NotNull Entity entity) { Check.stateCondition(instance == null, "You need to set an instance using Entity#setInstance"); - - if (!passengers.remove(entity)) - return; + if (!passengers.remove(entity)) return; entity.vehicle = null; sendPacketToViewersAndSelf(getPassengersPacket()); + if (entity instanceof Player) { + Player player = (Player) entity; + player.getPlayerConnection().sendPacket(new PlayerPositionAndLookPacket(player.getPosition(), + (byte) 0x00, player.getNextTeleportId(), true)); + } } /** From bf847fac793acb50e6657d7b85bd36ff80a060ec Mon Sep 17 00:00:00 2001 From: TheMode Date: Sat, 28 Aug 2021 11:51:39 +0200 Subject: [PATCH 25/56] Oops position --- .../server/listener/PlayerPositionListener.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java index d8fba54ff..da11e1a13 100644 --- a/src/main/java/net/minestom/server/listener/PlayerPositionListener.java +++ b/src/main/java/net/minestom/server/listener/PlayerPositionListener.java @@ -33,9 +33,9 @@ public class PlayerPositionListener { player.refreshReceivedTeleportId(packet.teleportId); } - private static void processMovement(@NotNull Player player, @NotNull Pos newPosition, boolean onGround) { + private static void processMovement(@NotNull Player player, @NotNull Pos packetPosition, boolean onGround) { final var currentPosition = player.getPosition(); - if (currentPosition.equals(newPosition)) { + if (currentPosition.equals(packetPosition)) { // For some reason, the position is the same return; } @@ -49,12 +49,12 @@ public class PlayerPositionListener { return; } // Try to move in an unloaded chunk, prevent it - if (!currentPosition.sameChunk(newPosition) && !ChunkUtils.isLoaded(instance, newPosition)) { + if (!currentPosition.sameChunk(packetPosition) && !ChunkUtils.isLoaded(instance, packetPosition)) { player.teleport(currentPosition); return; } - PlayerMoveEvent playerMoveEvent = new PlayerMoveEvent(player, newPosition); + PlayerMoveEvent playerMoveEvent = new PlayerMoveEvent(player, packetPosition); GlobalHandles.PLAYER_MOVE.call(playerMoveEvent); if (!currentPosition.equals(player.getPosition())) { // Player has been teleported in the event @@ -67,13 +67,15 @@ public class PlayerPositionListener { return; } final Pos eventPosition = playerMoveEvent.getNewPosition(); - if (newPosition.equals(eventPosition)) { + if (packetPosition.equals(eventPosition)) { // Event didn't change the position - player.refreshPosition(playerMoveEvent.getNewPosition()); + player.refreshPosition(eventPosition); player.refreshOnGround(onGround); } else { // Position modified by the event - if (newPosition.samePoint(eventPosition)) { + if (packetPosition.samePoint(eventPosition)) { + player.refreshPosition(eventPosition, true); + player.refreshOnGround(onGround); player.setView(eventPosition.yaw(), eventPosition.pitch()); } else { player.teleport(eventPosition); From 4d0924a5229537b8c490813109fdc2c0f176b437 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sat, 28 Aug 2021 13:06:09 +0200 Subject: [PATCH 26/56] Can now specify InstanceContainer chunk loader during initialization --- .../server/instance/InstanceContainer.java | 18 ++++++++---------- .../server/instance/InstanceManager.java | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/minestom/server/instance/InstanceContainer.java b/src/main/java/net/minestom/server/instance/InstanceContainer.java index 52f3fd3ae..b94cf0012 100644 --- a/src/main/java/net/minestom/server/instance/InstanceContainer.java +++ b/src/main/java/net/minestom/server/instance/InstanceContainer.java @@ -27,6 +27,7 @@ import net.minestom.server.utils.chunk.ChunkUtils; import net.minestom.server.utils.validate.Check; import net.minestom.server.world.DimensionType; import net.minestom.server.world.biomes.Biome; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -68,21 +69,18 @@ public class InstanceContainer extends Instance { protected InstanceContainer srcInstance; // only present if this instance has been created using a copy private long lastBlockChangeTime; // Time at which the last block change happened (#setBlock) - /** - * Creates an {@link InstanceContainer}. - * - * @param uniqueId the unique id of the instance - * @param dimensionType the dimension type of the instance - */ - public InstanceContainer(@NotNull UUID uniqueId, @NotNull DimensionType dimensionType) { + @ApiStatus.Experimental + public InstanceContainer(@NotNull UUID uniqueId, @NotNull DimensionType dimensionType, @Nullable IChunkLoader loader) { super(uniqueId, dimensionType); - // Set the default chunk supplier using DynamicChunk setChunkSupplier(DynamicChunk::new); - // Set the default chunk loader which use the Anvil format - setChunkLoader(new AnvilLoader("world")); + setChunkLoader(Objects.requireNonNullElseGet(loader, () -> new AnvilLoader("world"))); this.chunkLoader.loadInstance(this); } + public InstanceContainer(@NotNull UUID uniqueId, @NotNull DimensionType dimensionType) { + this(uniqueId, dimensionType, null); + } + @Override public void setBlock(int x, int y, int z, @NotNull Block block) { final Chunk chunk = getChunkAt(x, z); diff --git a/src/main/java/net/minestom/server/instance/InstanceManager.java b/src/main/java/net/minestom/server/instance/InstanceManager.java index 0dfcd9016..4dbece1a2 100644 --- a/src/main/java/net/minestom/server/instance/InstanceManager.java +++ b/src/main/java/net/minestom/server/instance/InstanceManager.java @@ -4,6 +4,7 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.storage.StorageLocation; import net.minestom.server.utils.validate.Check; import net.minestom.server.world.DimensionType; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -39,21 +40,32 @@ public final class InstanceManager { * with the specified {@link DimensionType} and {@link StorageLocation}. * * @param dimensionType the {@link DimensionType} of the instance + * @param loader the chunk loader * @return the created {@link InstanceContainer} */ - public @NotNull InstanceContainer createInstanceContainer(@NotNull DimensionType dimensionType) { - final InstanceContainer instanceContainer = new InstanceContainer(UUID.randomUUID(), dimensionType); + @ApiStatus.Experimental + public @NotNull InstanceContainer createInstanceContainer(@NotNull DimensionType dimensionType, @Nullable IChunkLoader loader) { + final InstanceContainer instanceContainer = new InstanceContainer(UUID.randomUUID(), dimensionType, loader); registerInstance(instanceContainer); return instanceContainer; } + public @NotNull InstanceContainer createInstanceContainer(@NotNull DimensionType dimensionType) { + return createInstanceContainer(dimensionType, null); + } + + @ApiStatus.Experimental + public @NotNull InstanceContainer createInstanceContainer(@Nullable IChunkLoader loader) { + return createInstanceContainer(DimensionType.OVERWORLD, loader); + } + /** * Creates and register an {@link InstanceContainer}. * * @return the created {@link InstanceContainer} */ public @NotNull InstanceContainer createInstanceContainer() { - return createInstanceContainer(DimensionType.OVERWORLD); + return createInstanceContainer(DimensionType.OVERWORLD, null); } /** From 85ca2a5302b838d11118b4341d77fb0239be2df7 Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 14:20:47 +0200 Subject: [PATCH 27/56] No longer expose Thread-binding to Task API --- .../map/framebuffers/GLFWCapableBuffer.java | 19 ++++++++++--- .../server/timer/SchedulerManager.java | 14 ---------- .../java/net/minestom/server/timer/Task.java | 28 +++++++------------ .../minestom/server/timer/TaskBuilder.java | 18 +----------- 4 files changed, 26 insertions(+), 53 deletions(-) diff --git a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java index 18dfb866f..4b2fbc951 100644 --- a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java +++ b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java @@ -4,6 +4,7 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.map.Framebuffer; import net.minestom.server.map.MapColors; import net.minestom.server.timer.Task; +import net.minestom.server.utils.thread.ThreadBindingExecutor; import org.lwjgl.BufferUtils; import org.lwjgl.PointerBuffer; import org.lwjgl.glfw.GLFWErrorCallback; @@ -27,6 +28,8 @@ public abstract class GLFWCapableBuffer { private final ByteBuffer colorsBuffer; private boolean onlyMapColors; + private static ThreadBindingExecutor threadBindingPool; + protected GLFWCapableBuffer(int width, int height) { this(width, height, GLFW_NATIVE_CONTEXT_API, GLFW_OPENGL_API); } @@ -60,6 +63,12 @@ public abstract class GLFWCapableBuffer { throw new RuntimeException("("+errcode+") Failed to create GLFW Window."); } } + + synchronized(GLFWCapableBuffer.class) { + if(threadBindingPool == null) { + threadBindingPool = new ThreadBindingExecutor(MinecraftServer.THREAD_COUNT_SCHEDULER, MinecraftServer.THREAD_NAME_SCHEDULER); + } + } } public GLFWCapableBuffer unbindContextFromThread() { @@ -81,17 +90,19 @@ public abstract class GLFWCapableBuffer { return MinecraftServer.getSchedulerManager() .buildTask(new Runnable() { private boolean first = true; - - @Override - public void run() { + private final Runnable subAction = () -> { if(first) { changeRenderingThreadToCurrent(); first = false; } render(rendering); + }; + + @Override + public void run() { + threadBindingPool.execute(subAction); } }) - .bindToSingleThread() .repeat(period) .schedule(); } diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index 698044727..a05ef7dc2 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -36,8 +36,6 @@ public final class SchedulerManager implements IExtensionObserver { private final AtomicInteger shutdownCounter; //A threaded execution private final ExecutorService batchesPool; - //Thread execution, which always uses the same Thread for a given Task - private final ExecutorService threadBindingPool; // A single threaded scheduled execution private final ScheduledExecutorService timerExecutionService; // All the registered tasks (task id = task) @@ -69,7 +67,6 @@ public final class SchedulerManager implements IExtensionObserver { this.shutdownCounter = new AtomicInteger(); this.batchesPool = new MinestomThread(MinecraftServer.THREAD_COUNT_SCHEDULER, MinecraftServer.THREAD_NAME_SCHEDULER); - this.threadBindingPool = new ThreadBindingExecutor(MinecraftServer.THREAD_COUNT_SCHEDULER, MinecraftServer.THREAD_NAME_SCHEDULER); this.timerExecutionService = Executors.newSingleThreadScheduledExecutor(); this.tasks = new Int2ObjectOpenHashMap<>(); this.shutdownTasks = new Int2ObjectOpenHashMap<>(); @@ -196,17 +193,6 @@ public final class SchedulerManager implements IExtensionObserver { return batchesPool; } - /** - * Gets the execution service for all the registered {@link Task}, which are marked as thread-bound. - * The thread to which tasks are assigned depends on their runnable hashcode. Two (or more) tasks can be bound to the same Thread. - * - * @return the execution service for all the registered {@link Task}, which are marked as thread-bound - */ - @NotNull - public ExecutorService getThreadBindingPool() { - return threadBindingPool; - } - /** * Gets the scheduled execution service for all the registered {@link Task}. * diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index 2e3d40e7d..397170bd1 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -45,10 +45,6 @@ public class Task implements Runnable { private volatile Thread currentThreadTask; // The executor service used for this task private final ExecutorService executorService; - // Whether this task will always execute on the same thread - private final boolean boundToSingleThread; - // Action executed on the executor. Stored inside the Task to avoid changing the hashcode (which ThreadBindingExecutor relies on) - private final Runnable action; /** * Creates a task. @@ -58,21 +54,25 @@ public class Task implements Runnable { * @param shutdown Defines whether the task is a shutdown task * @param delay The time to delay * @param repeat The time until the repetition - * @param bindToSingleThread Whether to run the given task always on the same thread. */ - public Task(@NotNull SchedulerManager schedulerManager, @NotNull Runnable runnable, boolean shutdown, long delay, long repeat, boolean isTransient, @Nullable String owningExtension, boolean bindToSingleThread) { + public Task(@NotNull SchedulerManager schedulerManager, @NotNull Runnable runnable, boolean shutdown, long delay, long repeat, boolean isTransient, @Nullable String owningExtension) { this.schedulerManager = schedulerManager; this.runnable = runnable; this.shutdown = shutdown; this.id = shutdown ? this.schedulerManager.getShutdownCounterIdentifier() : this.schedulerManager.getCounterIdentifier(); - this.executorService = bindToSingleThread ? this.schedulerManager.getThreadBindingPool() : this.schedulerManager.getBatchesPool(); + this.executorService = this.schedulerManager.getBatchesPool(); this.delay = delay; this.repeat = repeat; this.isTransient = isTransient; - this.boundToSingleThread = bindToSingleThread; this.owningExtension = owningExtension; + } - this.action = () -> { + /** + * Executes the task. + */ + @Override + public void run() { + executorService.execute(() -> { this.currentThreadTask = Thread.currentThread(); try { this.runnable.run(); @@ -88,15 +88,7 @@ public class Task implements Runnable { if (this.repeat == 0) this.finish(); this.currentThreadTask = null; } - }; - } - - /** - * Executes the task. - */ - @Override - public void run() { - executorService.execute(action); + }); } /** diff --git a/src/main/java/net/minestom/server/timer/TaskBuilder.java b/src/main/java/net/minestom/server/timer/TaskBuilder.java index 287e3d3b9..f1387f0c7 100644 --- a/src/main/java/net/minestom/server/timer/TaskBuilder.java +++ b/src/main/java/net/minestom/server/timer/TaskBuilder.java @@ -36,12 +36,6 @@ public class TaskBuilder { */ private boolean isTransient; - /** - * Should this Task be run on the same thread for each run? Can be used for situations where context is bound to a - * single thread. (for instance OpenGL context) - */ - private boolean boundToSingleThread = false; - /** * Creates a task builder. *
@@ -143,15 +137,6 @@ public class TaskBuilder { return this; } - /** - * Makes this Task be run on the same thread for each run. Can be used for situations where context is bound to a - * single thread. (for instance OpenGL context) - */ - public TaskBuilder bindToSingleThread() { - boundToSingleThread = true; - return this; - } - /** * Builds a {@link Task}. * @@ -166,8 +151,7 @@ public class TaskBuilder { this.delay, this.repeat, this.isTransient, - this.owningExtension, - this.boundToSingleThread); + this.owningExtension); } /** From b1b1e70c408b7c47b90bb004c2d0f775890296a3 Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 14:27:52 +0200 Subject: [PATCH 28/56] Cleanup un-needed changes --- .../java/net/minestom/server/timer/SchedulerManager.java | 4 ++-- src/main/java/net/minestom/server/timer/Task.java | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index a05ef7dc2..8c5ce4305 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -184,9 +184,9 @@ public final class SchedulerManager implements IExtensionObserver { } /** - * Gets the execution service for all the registered {@link Task}, which are not marked as thread-bound. + * Gets the execution service for all the registered {@link Task} * - * @return the execution service for all the registered {@link Task}, which are not marked as thread-bound + * @return the execution service for all the registered {@link Task} */ @NotNull public ExecutorService getBatchesPool() { diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index 397170bd1..63308058e 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -43,8 +43,6 @@ public class Task implements Runnable { private ScheduledFuture future; // The thread of the task private volatile Thread currentThreadTask; - // The executor service used for this task - private final ExecutorService executorService; /** * Creates a task. @@ -60,7 +58,6 @@ public class Task implements Runnable { this.runnable = runnable; this.shutdown = shutdown; this.id = shutdown ? this.schedulerManager.getShutdownCounterIdentifier() : this.schedulerManager.getCounterIdentifier(); - this.executorService = this.schedulerManager.getBatchesPool(); this.delay = delay; this.repeat = repeat; this.isTransient = isTransient; @@ -72,7 +69,7 @@ public class Task implements Runnable { */ @Override public void run() { - executorService.execute(() -> { + this.schedulerManager.getBatchesPool().execute(() -> { this.currentThreadTask = Thread.currentThread(); try { this.runnable.run(); From b9f1f75249a05a85f033223e5dd7d0e11d8d7e45 Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 14:28:45 +0200 Subject: [PATCH 29/56] Cleanup un-needed changes 2 --- src/main/java/net/minestom/server/timer/SchedulerManager.java | 2 +- src/main/java/net/minestom/server/timer/Task.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index 8c5ce4305..7ca75f841 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -164,7 +164,7 @@ public final class SchedulerManager implements IExtensionObserver { } /** - * Gets a {@link Collection} with all the registered shutdown {@link Task}. + * Gets a {@link Collection} with all the registered shutdown {@link Task} * * @return a {@link Collection} with all the registered shutdown {@link Task} */ diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index 63308058e..8b7e8218a 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -6,7 +6,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Objects; -import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; From 4184602eb0ab476e9dd94aa18074ca9f8fed0ba3 Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 14:30:49 +0200 Subject: [PATCH 30/56] oops wrong line --- src/main/java/net/minestom/server/timer/SchedulerManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index 7ca75f841..88f82661f 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -164,7 +164,7 @@ public final class SchedulerManager implements IExtensionObserver { } /** - * Gets a {@link Collection} with all the registered shutdown {@link Task} + * Gets a {@link Collection} with all the registered shutdown {@link Task}. * * @return a {@link Collection} with all the registered shutdown {@link Task} */ @@ -184,7 +184,7 @@ public final class SchedulerManager implements IExtensionObserver { } /** - * Gets the execution service for all the registered {@link Task} + * Gets the execution service for all the registered {@link Task}. * * @return the execution service for all the registered {@link Task} */ From 07a05a6ed2625b8365a1cc57085866cf74d734fd Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 16:45:06 +0200 Subject: [PATCH 31/56] Remove unused import --- src/main/java/net/minestom/server/timer/SchedulerManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index 88f82661f..bddfd29b8 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -7,7 +7,6 @@ import net.minestom.server.MinecraftServer; import net.minestom.server.extensions.Extension; import net.minestom.server.extensions.IExtensionObserver; import net.minestom.server.utils.thread.MinestomThread; -import net.minestom.server.utils.thread.ThreadBindingExecutor; import org.jetbrains.annotations.NotNull; import java.util.Collection; From 6fc89c20173ab3440efc7989500c6077d8bf614b Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 16:46:09 +0200 Subject: [PATCH 32/56] Remove debug line --- .../net/minestom/server/map/framebuffers/GLFWCapableBuffer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java index 4b2fbc951..6afe6c960 100644 --- a/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java +++ b/src/lwjgl/java/net/minestom/server/map/framebuffers/GLFWCapableBuffer.java @@ -77,7 +77,6 @@ public abstract class GLFWCapableBuffer { } public void changeRenderingThreadToCurrent() { - System.out.println("Currently on thread "+Thread.currentThread().getId()); glfwMakeContextCurrent(glfwWindow); GL.createCapabilities(); } From 9f97c985aa28bb2078c940802d5b523908f0bae7 Mon Sep 17 00:00:00 2001 From: jglrxavpok Date: Sat, 28 Aug 2021 17:09:07 +0200 Subject: [PATCH 33/56] Make javadoc build again --- .../net/minestom/server/utils/thread/ThreadBindingExecutor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java b/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java index 3b8c119cb..800be9a8d 100644 --- a/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java +++ b/src/main/java/net/minestom/server/utils/thread/ThreadBindingExecutor.java @@ -9,7 +9,7 @@ import java.util.concurrent.TimeUnit; /** * Executor service which will always give the same thread to a given Runnable. - * Uses {@link Runnable#hashCode()} to determine the thread to assign. + * Uses 

Runnable#hashCode()
to determine the thread to assign. */ public class ThreadBindingExecutor extends AbstractExecutorService { From 8b7fb7a7c5331018b8fc4cfdd23e6fc502ebd000 Mon Sep 17 00:00:00 2001 From: EpicPlayerA10 Date: Sat, 28 Aug 2021 21:10:56 +0200 Subject: [PATCH 34/56] Add missing husk --- src/main/java/net/minestom/server/entity/EntityTypeImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/net/minestom/server/entity/EntityTypeImpl.java b/src/main/java/net/minestom/server/entity/EntityTypeImpl.java index 3d89f8f1c..50c6eb833 100644 --- a/src/main/java/net/minestom/server/entity/EntityTypeImpl.java +++ b/src/main/java/net/minestom/server/entity/EntityTypeImpl.java @@ -23,6 +23,7 @@ import net.minestom.server.entity.metadata.monster.skeleton.SkeletonMeta; import net.minestom.server.entity.metadata.monster.skeleton.StrayMeta; import net.minestom.server.entity.metadata.monster.skeleton.WitherSkeletonMeta; import net.minestom.server.entity.metadata.monster.zombie.DrownedMeta; +import net.minestom.server.entity.metadata.monster.zombie.HuskMeta; import net.minestom.server.entity.metadata.monster.zombie.ZombieMeta; import net.minestom.server.entity.metadata.monster.zombie.ZombieVillagerMeta; import net.minestom.server.entity.metadata.monster.zombie.ZombifiedPiglinMeta; @@ -119,6 +120,7 @@ final class EntityTypeImpl implements EntityType { supplier.put("minecraft:guardian", GuardianMeta::new); supplier.put("minecraft:hoglin", HoglinMeta::new); supplier.put("minecraft:horse", HorseMeta::new); + supplier.put("minecraft:husk", HuskMeta::new); supplier.put("minecraft:illusioner", IllusionerMeta::new); supplier.put("minecraft:iron_golem", IronGolemMeta::new); supplier.put("minecraft:item", ItemEntityMeta::new); From a75910fe3fd348a58b40250f510f66bfa038f9c6 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sun, 29 Aug 2021 00:07:58 +0200 Subject: [PATCH 35/56] Fix drag clicks --- .../minestom/server/inventory/Inventory.java | 13 +--- .../server/inventory/PlayerInventory.java | 4 +- .../click/InventoryClickProcessor.java | 67 +++++++++++-------- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/main/java/net/minestom/server/inventory/Inventory.java b/src/main/java/net/minestom/server/inventory/Inventory.java index edc155626..c8a253939 100644 --- a/src/main/java/net/minestom/server/inventory/Inventory.java +++ b/src/main/java/net/minestom/server/inventory/Inventory.java @@ -401,18 +401,7 @@ public class Inventory extends AbstractInventory implements Viewable { final InventoryClickResult clickResult = clickProcessor.dragging(player, slot != -999 ? (isInWindow ? this : playerInventory) : null, clickSlot, button, - clicked, cursor, - - s -> isClickInWindow(s) ? getItemStack(s) : - playerInventory.getItemStack(PlayerInventoryUtils.convertSlot(s, offset)), - - (s, item) -> { - if (isClickInWindow(s)) { - setItemStack(s, item); - } else { - playerInventory.setItemStack(PlayerInventoryUtils.convertSlot(s, offset), item); - } - }); + clicked, cursor); if (clickResult == null || clickResult.isCancel()) { updateAll(player); return false; diff --git a/src/main/java/net/minestom/server/inventory/PlayerInventory.java b/src/main/java/net/minestom/server/inventory/PlayerInventory.java index af3fbe2d2..def1fdca1 100644 --- a/src/main/java/net/minestom/server/inventory/PlayerInventory.java +++ b/src/main/java/net/minestom/server/inventory/PlayerInventory.java @@ -317,9 +317,7 @@ public class PlayerInventory extends AbstractInventory implements EquipmentHandl final ItemStack cursor = getCursorItem(); final ItemStack clicked = slot != -999 ? getItemStackFromPacketSlot(slot) : ItemStack.AIR; final InventoryClickResult clickResult = clickProcessor.dragging(player, this, - slot, button, - clicked, cursor, this::getItemStackFromPacketSlot, - this::setItemStackFromPacketSlot); + convertPlayerInventorySlot(slot, OFFSET), button, clicked, cursor); if (clickResult == null || clickResult.isCancel()) { update(); return false; diff --git a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java index acc8ffa10..b88ba53b7 100644 --- a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java +++ b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java @@ -1,8 +1,5 @@ package net.minestom.server.inventory.click; -import it.unimi.dsi.fastutil.ints.Int2ObjectFunction; -import it.unimi.dsi.fastutil.ints.IntOpenHashSet; -import it.unimi.dsi.fastutil.ints.IntSet; import net.minestom.server.entity.EquipmentSlot; import net.minestom.server.entity.Player; import net.minestom.server.event.EventDispatcher; @@ -18,18 +15,18 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.BiConsumer; import java.util.function.BiFunction; @ApiStatus.Internal public final class InventoryClickProcessor { // Dragging maps - private final Map leftDraggingMap = new ConcurrentHashMap<>(); - private final Map rightDraggingMap = new ConcurrentHashMap<>(); + private final Map> leftDraggingMap = new ConcurrentHashMap<>(); + private final Map> rightDraggingMap = new ConcurrentHashMap<>(); public @NotNull InventoryClickResult leftClick(@NotNull Player player, @NotNull AbstractInventory inventory, int slot, @@ -162,23 +159,21 @@ public final class InventoryClickProcessor { public @Nullable InventoryClickResult dragging(@NotNull Player player, @Nullable AbstractInventory inventory, int slot, int button, - @NotNull ItemStack clicked, @NotNull ItemStack cursor, - @NotNull Int2ObjectFunction itemGetter, - @NotNull BiConsumer itemSetter) { + @NotNull ItemStack clicked, @NotNull ItemStack cursor) { InventoryClickResult clickResult = null; final StackingRule stackingRule = cursor.getStackingRule(); if (slot != -999) { // Add slot if (button == 1) { // Add left - IntSet left = leftDraggingMap.get(player); + List left = leftDraggingMap.get(player); if (left == null) return null; - left.add(slot); + left.add(new DragData(slot, inventory)); } else if (button == 5) { // Add right - IntSet right = rightDraggingMap.get(player); + List right = rightDraggingMap.get(player); if (right == null) return null; - right.add(slot); + right.add(new DragData(slot, inventory)); } else if (button == 9) { // Add middle // TODO @@ -188,18 +183,18 @@ public final class InventoryClickProcessor { if (button == 0) { // Start left clickResult = startCondition(player, inventory, slot, ClickType.START_LEFT_DRAGGING, clicked, cursor); - if (!clickResult.isCancel()) this.leftDraggingMap.put(player, new IntOpenHashSet()); + if (!clickResult.isCancel()) this.leftDraggingMap.put(player, new ArrayList<>()); } else if (button == 2) { // End left - final IntSet slots = leftDraggingMap.remove(player); + final List slots = leftDraggingMap.remove(player); if (slots == null) return null; final int slotCount = slots.size(); final int cursorAmount = stackingRule.getAmount(cursor); if (slotCount > cursorAmount) return null; - for (int s : slots) { + for (DragData s : slots) { // Apply each drag element - final ItemStack slotItem = itemGetter.apply(s); - clickResult = startCondition(player, inventory, s, ClickType.LEFT_DRAGGING, slotItem, cursor); + final ItemStack slotItem = s.inventory.getItemStack(s.slot); + clickResult = startCondition(player, s.inventory, s.slot, ClickType.LEFT_DRAGGING, slotItem, cursor); if (clickResult.isCancel()) { return clickResult; } @@ -210,8 +205,10 @@ public final class InventoryClickProcessor { final int slotSize = (int) ((float) cursorAmount / (float) slotCount); // Place all waiting drag action int finalCursorAmount = cursorAmount; - for (int s : slots) { - ItemStack slotItem = itemGetter.apply(s); + for (DragData dragData : slots) { + final var inv = dragData.inventory; + final int s = dragData.slot; + ItemStack slotItem = inv.getItemStack(s); final StackingRule slotItemRule = slotItem.getStackingRule(); final int amount = slotItemRule.getAmount(slotItem); if (stackingRule.canBeStacked(cursor, slotItem)) { @@ -231,7 +228,7 @@ public final class InventoryClickProcessor { slotItem = stackingRule.apply(cursor, slotSize); finalCursorAmount -= slotSize; } - itemSetter.accept(s, slotItem); + inv.setItemStack(s, slotItem); callClickEvent(player, inventory, s, ClickType.LEFT_DRAGGING, slotItem, cursor); } // Update the cursor @@ -239,18 +236,18 @@ public final class InventoryClickProcessor { } else if (button == 4) { // Start right clickResult = startCondition(player, inventory, slot, ClickType.START_RIGHT_DRAGGING, clicked, cursor); - if (!clickResult.isCancel()) this.rightDraggingMap.put(player, new IntOpenHashSet()); + if (!clickResult.isCancel()) this.rightDraggingMap.put(player, new ArrayList<>()); } else if (button == 6) { // End right - final IntSet slots = rightDraggingMap.remove(player); + final List slots = rightDraggingMap.remove(player); if (slots == null) return null; final int size = slots.size(); int cursorAmount = stackingRule.getAmount(cursor); if (size > cursorAmount) return null; // Verify if each slot can be modified (or cancel the whole drag) - for (int s : slots) { - ItemStack slotItem = itemGetter.apply(s); - clickResult = startCondition(player, inventory, s, ClickType.RIGHT_DRAGGING, slotItem, cursor); + for (DragData s : slots) { + final ItemStack slotItem = s.inventory.getItemStack(s.slot); + clickResult = startCondition(player, s.inventory, s.slot, ClickType.RIGHT_DRAGGING, slotItem, cursor); if (clickResult.isCancel()) { return clickResult; } @@ -259,8 +256,10 @@ public final class InventoryClickProcessor { if (clickResult.isCancel()) return clickResult; // Place all waiting drag action int finalCursorAmount = cursorAmount; - for (int s : slots) { - ItemStack slotItem = itemGetter.apply(s); + for (DragData dragData : slots) { + final var inv = dragData.inventory; + final int s = dragData.slot; + ItemStack slotItem = inv.getItemStack(s); StackingRule slotItemRule = slotItem.getStackingRule(); if (stackingRule.canBeStacked(cursor, slotItem)) { // Compatible item in the slot, increment by 1 @@ -274,7 +273,7 @@ public final class InventoryClickProcessor { slotItem = stackingRule.apply(cursor, 1); finalCursorAmount -= 1; } - itemSetter.accept(s, slotItem); + inv.setItemStack(s, slotItem); callClickEvent(player, inventory, s, ClickType.RIGHT_DRAGGING, slotItem, cursor); } // Update the cursor @@ -462,4 +461,14 @@ public final class InventoryClickProcessor { this.leftDraggingMap.remove(player); this.rightDraggingMap.remove(player); } + + private static final class DragData { + private final int slot; + private final AbstractInventory inventory; + + public DragData(int slot, AbstractInventory inventory) { + this.slot = slot; + this.inventory = inventory; + } + } } From 0a3ad69e58584706ba415aaf3bb4e26b2139bfa1 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sun, 29 Aug 2021 01:03:09 +0200 Subject: [PATCH 36/56] Ensure listeners being up to date --- .../net/minestom/server/event/EventNodeImpl.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index aeece586d..74615b819 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -273,8 +273,7 @@ class EventNodeImpl implements EventNode { @Override public void call(@NotNull E event) { - update(); - final Consumer[] listeners = this.listeners; + final Consumer[] listeners = updatedListeners(); if (listeners.length == 0) return; for (Consumer listener : listeners) { listener.accept(event); @@ -283,18 +282,19 @@ class EventNodeImpl implements EventNode { @Override public boolean hasListener() { - update(); - return listeners.length > 0; + return updatedListeners().length > 0; } - void update() { - if (updated) return; + Consumer[] updatedListeners() { + if (updated) return listeners; synchronized (GLOBAL_CHILD_LOCK) { - if (updated) return; + if (updated) return listeners; this.listenersCache.clear(); recursiveUpdate(node); - this.listeners = listenersCache.toArray(Consumer[]::new); + final Consumer[] listenersArray = listenersCache.toArray(Consumer[]::new); + this.listeners = listenersArray; this.updated = true; + return listenersArray; } } From 8b61ead08e9b891559375020e9ed6aeb8c46c017 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sun, 29 Aug 2021 12:25:57 +0200 Subject: [PATCH 37/56] Fix parent filtering --- .../minestom/server/event/EventNodeImpl.java | 232 +++++++++--------- 1 file changed, 121 insertions(+), 111 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index 74615b819..c9c0ccc12 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -262,8 +262,7 @@ class EventNodeImpl implements EventNode { private static final class Handle implements ListenerHandle { private final EventNodeImpl node; private final Class eventType; - private Consumer[] listeners = new Consumer[0]; - private final List> listenersCache = new ArrayList<>(); + private Consumer listener = null; private volatile boolean updated; Handle(EventNodeImpl node, Class eventType) { @@ -273,55 +272,125 @@ class EventNodeImpl implements EventNode { @Override public void call(@NotNull E event) { - final Consumer[] listeners = updatedListeners(); - if (listeners.length == 0) return; - for (Consumer listener : listeners) { - listener.accept(event); + final Consumer listener = updatedListener(); + if (listener != null) { + try { + listener.accept(event); + } catch (Throwable e) { + MinecraftServer.getExceptionManager().handleException(e); + } } } @Override public boolean hasListener() { - return updatedListeners().length > 0; + return updatedListener() != null; } - Consumer[] updatedListeners() { - if (updated) return listeners; + @Nullable Consumer updatedListener() { + if (updated) return listener; synchronized (GLOBAL_CHILD_LOCK) { - if (updated) return listeners; - this.listenersCache.clear(); - recursiveUpdate(node); - final Consumer[] listenersArray = listenersCache.toArray(Consumer[]::new); - this.listeners = listenersArray; + if (updated) return listener; + final Consumer listener = createConsumer(); + this.listener = listener; this.updated = true; - return listenersArray; + return listener; } } - private void recursiveUpdate(EventNodeImpl targetNode) { + private @Nullable Consumer createConsumer() { // Standalone listeners + List> listeners = new ArrayList<>(); forTargetEvents(eventType, type -> { - final ListenerEntry entry = targetNode.listenerMap.get(type); - if (entry != null) appendEntries(entry, targetNode); + final ListenerEntry entry = node.listenerMap.get(type); + if (entry != null) { + final Consumer result = appendEntries(entry, node); + if (result != null) listeners.add(result); + } }); - // Mapped nodes - handleMappedNode(targetNode); - // Add children - final Set> children = targetNode.children; - if (children.isEmpty()) return; - children.stream() + final Consumer[] listenersArray = listeners.toArray(Consumer[]::new); + // Mapped + final Consumer mappedListener = handleMappedNode(node); + // Children + final Consumer[] childrenListeners = node.children.stream() .filter(child -> child.eventType.isAssignableFrom(eventType)) // Invalid event type .sorted(Comparator.comparing(EventNode::getPriority)) - .forEach(this::recursiveUpdate); + .map(child -> ((Handle) child.getHandle(eventType)).updatedListener()) + .filter(eConsumer -> eConsumer != null) + .toArray(Consumer[]::new); + // Empty check + final BiPredicate predicate = node.predicate; + final EventFilter filter = node.filter; + final boolean hasPredicate = predicate != null; + final boolean hasListeners = listenersArray.length > 0; + final boolean hasMap = mappedListener != null; + final boolean hasChildren = childrenListeners.length > 0; + if (listenersArray.length == 0 && mappedListener == null && childrenListeners.length == 0) { + // No listener + return null; + } + return e -> { + // Filtering + if (hasPredicate) { + final Object value = filter.getHandler(e); + if (!predicate.test(e, value)) return; + } + // Normal listeners + if (hasListeners) { + for (Consumer listener : listenersArray) { + listener.accept(e); + } + } + // Mapped nodes + if (hasMap) mappedListener.accept(e); + // Children + if (hasChildren) { + for (Consumer childHandle : childrenListeners) { + childHandle.accept(e); + } + } + }; } /** - * Add the node's listeners from {@link EventNode#map(EventNode, Object)}. + * Add listeners from {@link EventNode#addListener(EventListener)} and + * {@link EventNode#register(EventBinding)} to the handle list. + *

+ * Most computation should ideally be done outside the consumers as a one-time cost. + */ + private @Nullable Consumer appendEntries(ListenerEntry entry, EventNodeImpl targetNode) { + final EventListener[] listenersCopy = entry.listeners.toArray(EventListener[]::new); + final Consumer[] bindingsCopy = entry.bindingConsumers.toArray(Consumer[]::new); + final boolean listenersEmpty = listenersCopy.length == 0; + final boolean bindingsEmpty = bindingsCopy.length == 0; + if (listenersEmpty && bindingsEmpty) return null; + if (bindingsEmpty && listenersCopy.length == 1) { + // Only one normal listener + final EventListener listener = listenersCopy[0]; + return e -> callListener(targetNode, listener, e); + } + // Worse case scenario, try to run everything + return e -> { + if (!listenersEmpty) { + for (EventListener listener : listenersCopy) { + callListener(targetNode, listener, e); + } + } + if (!bindingsEmpty) { + for (Consumer eConsumer : bindingsCopy) { + eConsumer.accept(e); + } + } + }; + } + + /** + * Create a consumer handling {@link EventNode#map(EventNode, Object)}. * The goal is to limit the amount of map lookup. */ - private void handleMappedNode(EventNodeImpl targetNode) { + private @Nullable Consumer handleMappedNode(EventNodeImpl targetNode) { final var mappedNodeCache = targetNode.mappedNodeCache; - if (mappedNodeCache.isEmpty()) return; + if (mappedNodeCache.isEmpty()) return null; Set> filters = new HashSet<>(mappedNodeCache.size()); Map> handlers = new HashMap<>(mappedNodeCache.size()); // Retrieve all filters used to retrieve potential handlers @@ -334,94 +403,35 @@ class EventNodeImpl implements EventNode { } // If at least one mapped node listen to this handle type, // loop through them and forward to mapped node if there is a match - if (!filters.isEmpty()) { - final EventFilter[] filterList = filters.toArray(EventFilter[]::new); - final BiConsumer, E> mapper = (filter, event) -> { - final Object handler = filter.castHandler(event); - final Handle handle = handlers.get(handler); - if (handle != null) handle.call(event); + if (filters.isEmpty()) return null; + final EventFilter[] filterList = filters.toArray(EventFilter[]::new); + final BiConsumer, E> mapper = (filter, event) -> { + final Object handler = filter.castHandler(event); + final Handle handle = handlers.get(handler); + if (handle != null) handle.call(event); + }; + if (filterList.length == 1) { + final var firstFilter = filterList[0]; + // Common case where there is only one filter + return event -> mapper.accept(firstFilter, event); + } else if (filterList.length == 2) { + final var firstFilter = filterList[0]; + final var secondFilter = filterList[1]; + return event -> { + mapper.accept(firstFilter, event); + mapper.accept(secondFilter, event); + }; + } else { + return event -> { + for (var filter : filterList) { + mapper.accept(filter, event); + } }; - if (filterList.length == 1) { - final var firstFilter = filterList[0]; - // Common case where there is only one filter - this.listenersCache.add(event -> mapper.accept(firstFilter, event)); - } else if (filterList.length == 2) { - final var firstFilter = filterList[0]; - final var secondFilter = filterList[1]; - this.listenersCache.add(event -> { - mapper.accept(firstFilter, event); - mapper.accept(secondFilter, event); - }); - } else { - this.listenersCache.add(event -> { - for (var filter : filterList) { - mapper.accept(filter, event); - } - }); - } } } - /** - * Add listeners from {@link EventNode#addListener(EventListener)} and - * {@link EventNode#register(EventBinding)} to the handle list. - *

- * Most computation should ideally be done outside the consumers as a one-time cost. - */ - private void appendEntries(ListenerEntry entry, EventNodeImpl targetNode) { - final var filter = targetNode.filter; - final var predicate = targetNode.predicate; - - final boolean hasPredicate = predicate != null; - final EventListener[] listenersCopy = entry.listeners.toArray(EventListener[]::new); - final Consumer[] bindingsCopy = entry.bindingConsumers.toArray(Consumer[]::new); - final boolean listenersEmpty = listenersCopy.length == 0; - final boolean bindingsEmpty = bindingsCopy.length == 0; - if (!hasPredicate && listenersEmpty && bindingsEmpty) - return; // Nothing to run - - if (!hasPredicate && bindingsEmpty && listenersCopy.length == 1) { - // Only one normal listener - final EventListener listener = listenersCopy[0]; - this.listenersCache.add(e -> callListener(targetNode, listener, e)); - return; - } - // Worse case scenario, try to run everything - this.listenersCache.add(e -> { - if (hasPredicate) { - final Object value = filter.getHandler(e); - try { - if (!predicate.test(e, value)) return; - } catch (Throwable t) { - MinecraftServer.getExceptionManager().handleException(t); - return; - } - } - if (!listenersEmpty) { - for (EventListener listener : listenersCopy) { - callListener(targetNode, listener, e); - } - } - if (!bindingsEmpty) { - for (Consumer eConsumer : bindingsCopy) { - try { - eConsumer.accept(e); - } catch (Throwable t) { - MinecraftServer.getExceptionManager().handleException(t); - } - } - } - }); - } - static void callListener(EventNodeImpl targetNode, EventListener listener, E event) { - EventListener.Result result; - try { - result = listener.run(event); - } catch (Throwable t) { - result = EventListener.Result.EXCEPTION; - MinecraftServer.getExceptionManager().handleException(t); - } + EventListener.Result result = listener.run(event); if (result == EventListener.Result.EXPIRED) { targetNode.removeListener(listener); } From 61d19243f7d4019879029ae1eadfe94e43989f9c Mon Sep 17 00:00:00 2001 From: TheMode Date: Sun, 29 Aug 2021 12:50:44 +0200 Subject: [PATCH 38/56] Fix event expiration --- src/main/java/net/minestom/server/event/EventNodeImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index c9c0ccc12..c40486c0a 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -430,10 +430,11 @@ class EventNodeImpl implements EventNode { } } - static void callListener(EventNodeImpl targetNode, EventListener listener, E event) { + void callListener(EventNodeImpl targetNode, EventListener listener, E event) { EventListener.Result result = listener.run(event); if (result == EventListener.Result.EXPIRED) { targetNode.removeListener(listener); + this.updated = false; } } } From a92ac203f7966d419501847876cb1f28e040f276 Mon Sep 17 00:00:00 2001 From: TheMode Date: Sun, 29 Aug 2021 17:11:10 +0200 Subject: [PATCH 39/56] Style cleanup --- .../minestom/server/event/EventNodeImpl.java | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/main/java/net/minestom/server/event/EventNodeImpl.java b/src/main/java/net/minestom/server/event/EventNodeImpl.java index c40486c0a..495a6b1a1 100644 --- a/src/main/java/net/minestom/server/event/EventNodeImpl.java +++ b/src/main/java/net/minestom/server/event/EventNodeImpl.java @@ -273,12 +273,11 @@ class EventNodeImpl implements EventNode { @Override public void call(@NotNull E event) { final Consumer listener = updatedListener(); - if (listener != null) { - try { - listener.accept(event); - } catch (Throwable e) { - MinecraftServer.getExceptionManager().handleException(e); - } + if (listener == null) return; + try { + listener.accept(event); + } catch (Throwable e) { + MinecraftServer.getExceptionManager().handleException(e); } } @@ -304,19 +303,19 @@ class EventNodeImpl implements EventNode { forTargetEvents(eventType, type -> { final ListenerEntry entry = node.listenerMap.get(type); if (entry != null) { - final Consumer result = appendEntries(entry, node); + final Consumer result = listenersConsumer(entry); if (result != null) listeners.add(result); } }); final Consumer[] listenersArray = listeners.toArray(Consumer[]::new); // Mapped - final Consumer mappedListener = handleMappedNode(node); + final Consumer mappedListener = mappedConsumer(); // Children final Consumer[] childrenListeners = node.children.stream() .filter(child -> child.eventType.isAssignableFrom(eventType)) // Invalid event type .sorted(Comparator.comparing(EventNode::getPriority)) .map(child -> ((Handle) child.getHandle(eventType)).updatedListener()) - .filter(eConsumer -> eConsumer != null) + .filter(Objects::nonNull) .toArray(Consumer[]::new); // Empty check final BiPredicate predicate = node.predicate; @@ -353,12 +352,12 @@ class EventNodeImpl implements EventNode { } /** - * Add listeners from {@link EventNode#addListener(EventListener)} and - * {@link EventNode#register(EventBinding)} to the handle list. + * Create a consumer calling all listeners from {@link EventNode#addListener(EventListener)} and + * {@link EventNode#register(EventBinding)}. *

* Most computation should ideally be done outside the consumers as a one-time cost. */ - private @Nullable Consumer appendEntries(ListenerEntry entry, EventNodeImpl targetNode) { + private @Nullable Consumer listenersConsumer(@NotNull ListenerEntry entry) { final EventListener[] listenersCopy = entry.listeners.toArray(EventListener[]::new); final Consumer[] bindingsCopy = entry.bindingConsumers.toArray(Consumer[]::new); final boolean listenersEmpty = listenersCopy.length == 0; @@ -367,13 +366,13 @@ class EventNodeImpl implements EventNode { if (bindingsEmpty && listenersCopy.length == 1) { // Only one normal listener final EventListener listener = listenersCopy[0]; - return e -> callListener(targetNode, listener, e); + return e -> callListener(listener, e); } // Worse case scenario, try to run everything return e -> { if (!listenersEmpty) { for (EventListener listener : listenersCopy) { - callListener(targetNode, listener, e); + callListener(listener, e); } } if (!bindingsEmpty) { @@ -388,8 +387,8 @@ class EventNodeImpl implements EventNode { * Create a consumer handling {@link EventNode#map(EventNode, Object)}. * The goal is to limit the amount of map lookup. */ - private @Nullable Consumer handleMappedNode(EventNodeImpl targetNode) { - final var mappedNodeCache = targetNode.mappedNodeCache; + private @Nullable Consumer mappedConsumer() { + final var mappedNodeCache = node.mappedNodeCache; if (mappedNodeCache.isEmpty()) return null; Set> filters = new HashSet<>(mappedNodeCache.size()); Map> handlers = new HashMap<>(mappedNodeCache.size()); @@ -430,10 +429,10 @@ class EventNodeImpl implements EventNode { } } - void callListener(EventNodeImpl targetNode, EventListener listener, E event) { + void callListener(@NotNull EventListener listener, E event) { EventListener.Result result = listener.run(event); if (result == EventListener.Result.EXPIRED) { - targetNode.removeListener(listener); + node.removeListener(listener); this.updated = false; } } From ae04ca5574f7197b407b3f86300e791898336531 Mon Sep 17 00:00:00 2001 From: TheMode Date: Mon, 30 Aug 2021 12:33:09 +0200 Subject: [PATCH 40/56] More comments for ListenerHandle --- .../net/minestom/server/event/ListenerHandle.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/event/ListenerHandle.java b/src/main/java/net/minestom/server/event/ListenerHandle.java index eb3f82362..5f3850695 100644 --- a/src/main/java/net/minestom/server/event/ListenerHandle.java +++ b/src/main/java/net/minestom/server/event/ListenerHandle.java @@ -4,14 +4,24 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; /** - * Represents a key to access an {@link EventNode} listeners. + * Represents a key to a listenable event, retrievable from {@link EventNode#getHandle(Class)}. * Useful to avoid map lookups. + *

+ * It is recommended to store instances of this class in {@code static final} fields. * * @param the event type */ @ApiStatus.Experimental @ApiStatus.NonExtendable public interface ListenerHandle { + /** + * Calls the given event. + * Will try to fast exit the execution when possible if {@link #hasListener()} return {@code false}. + *

+ * Anonymous and subclasses are not supported, events must have the exact type {@code E}. + * + * @param event the event to call + */ void call(@NotNull E event); /** From e99d8c6a6f66dfdd44ee85e65bf1def11392f8fd Mon Sep 17 00:00:00 2001 From: TheMode Date: Mon, 30 Aug 2021 15:52:07 +0200 Subject: [PATCH 41/56] Add BlockHandler.PlayerPlacement#getHand --- src/main/java/net/minestom/server/instance/Instance.java | 3 +-- .../net/minestom/server/instance/InstanceContainer.java | 9 ++++----- .../net/minestom/server/instance/SharedInstance.java | 7 +++---- .../net/minestom/server/instance/block/BlockHandler.java | 8 +++++++- .../minestom/server/listener/BlockPlacementListener.java | 4 ++-- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/minestom/server/instance/Instance.java b/src/main/java/net/minestom/server/instance/Instance.java index 46991b268..4dc1b8623 100644 --- a/src/main/java/net/minestom/server/instance/Instance.java +++ b/src/main/java/net/minestom/server/instance/Instance.java @@ -139,8 +139,7 @@ public abstract class Instance implements BlockGetter, BlockSetter, Tickable, Ta } @ApiStatus.Internal - public abstract boolean placeBlock(@NotNull Player player, @NotNull Block block, @NotNull Point blockPosition, - @NotNull BlockFace blockFace, float cursorX, float cursorY, float cursorZ); + public abstract boolean placeBlock(@NotNull BlockHandler.Placement placement); /** * Does call {@link net.minestom.server.event.player.PlayerBlockBreakEvent} diff --git a/src/main/java/net/minestom/server/instance/InstanceContainer.java b/src/main/java/net/minestom/server/instance/InstanceContainer.java index b94cf0012..3a320cf6d 100644 --- a/src/main/java/net/minestom/server/instance/InstanceContainer.java +++ b/src/main/java/net/minestom/server/instance/InstanceContainer.java @@ -13,7 +13,6 @@ import net.minestom.server.event.instance.InstanceChunkUnloadEvent; import net.minestom.server.event.player.PlayerBlockBreakEvent; import net.minestom.server.instance.batch.ChunkGenerationBatch; import net.minestom.server.instance.block.Block; -import net.minestom.server.instance.block.BlockFace; import net.minestom.server.instance.block.BlockHandler; import net.minestom.server.instance.block.rule.BlockPlacementRule; import net.minestom.server.network.packet.server.play.BlockChangePacket; @@ -154,12 +153,12 @@ public class InstanceContainer extends Instance { } @Override - public boolean placeBlock(@NotNull Player player, @NotNull Block block, @NotNull Point blockPosition, - @NotNull BlockFace blockFace, float cursorX, float cursorY, float cursorZ) { + public boolean placeBlock(@NotNull BlockHandler.Placement placement) { + final Point blockPosition = placement.getBlockPosition(); final Chunk chunk = getChunkAt(blockPosition); if (!ChunkUtils.isLoaded(chunk)) return false; - UNSAFE_setBlock(chunk, blockPosition.blockX(), blockPosition.blockY(), blockPosition.blockZ(), block, - new BlockHandler.PlayerPlacement(block, this, blockPosition, player, blockFace, cursorX, cursorY, cursorZ), null); + UNSAFE_setBlock(chunk, blockPosition.blockX(), blockPosition.blockY(), blockPosition.blockZ(), + placement.getBlock(), placement, null); return true; } diff --git a/src/main/java/net/minestom/server/instance/SharedInstance.java b/src/main/java/net/minestom/server/instance/SharedInstance.java index df2b80391..a4ae6eaaf 100644 --- a/src/main/java/net/minestom/server/instance/SharedInstance.java +++ b/src/main/java/net/minestom/server/instance/SharedInstance.java @@ -3,7 +3,7 @@ package net.minestom.server.instance; import net.minestom.server.coordinate.Point; import net.minestom.server.entity.Player; import net.minestom.server.instance.block.Block; -import net.minestom.server.instance.block.BlockFace; +import net.minestom.server.instance.block.BlockHandler; import org.jetbrains.annotations.NotNull; import java.util.Collection; @@ -28,9 +28,8 @@ public class SharedInstance extends Instance { } @Override - public boolean placeBlock(@NotNull Player player, @NotNull Block block, @NotNull Point blockPosition, - @NotNull BlockFace blockFace, float cursorX, float cursorY, float cursorZ) { - return instanceContainer.placeBlock(player, block, blockPosition, blockFace, cursorX, cursorY, cursorZ); + public boolean placeBlock(@NotNull BlockHandler.Placement placement) { + return instanceContainer.placeBlock(placement); } @Override diff --git a/src/main/java/net/minestom/server/instance/block/BlockHandler.java b/src/main/java/net/minestom/server/instance/block/BlockHandler.java index b07c5f8f4..acd794dc1 100644 --- a/src/main/java/net/minestom/server/instance/block/BlockHandler.java +++ b/src/main/java/net/minestom/server/instance/block/BlockHandler.java @@ -114,14 +114,16 @@ public interface BlockHandler { final class PlayerPlacement extends Placement { private final Player player; + private final Player.Hand hand; private final BlockFace blockFace; private final float cursorX, cursorY, cursorZ; @ApiStatus.Internal public PlayerPlacement(Block block, Instance instance, Point blockPosition, - Player player, BlockFace blockFace, float cursorX, float cursorY, float cursorZ) { + Player player, Player.Hand hand, BlockFace blockFace, float cursorX, float cursorY, float cursorZ) { super(block, instance, blockPosition); this.player = player; + this.hand = hand; this.blockFace = blockFace; this.cursorX = cursorX; this.cursorY = cursorY; @@ -132,6 +134,10 @@ public interface BlockHandler { return player; } + public @NotNull Player.Hand getHand() { + return hand; + } + public @NotNull BlockFace getBlockFace() { return blockFace; } diff --git a/src/main/java/net/minestom/server/listener/BlockPlacementListener.java b/src/main/java/net/minestom/server/listener/BlockPlacementListener.java index 92ca45854..662575af6 100644 --- a/src/main/java/net/minestom/server/listener/BlockPlacementListener.java +++ b/src/main/java/net/minestom/server/listener/BlockPlacementListener.java @@ -157,8 +157,8 @@ public class BlockPlacementListener { return; } // Place the block - instance.placeBlock(player, resultBlock, placementPosition, - blockFace, packet.cursorPositionX, packet.cursorPositionY, packet.cursorPositionZ); + instance.placeBlock(new BlockHandler.PlayerPlacement(resultBlock, instance, blockPosition, player, hand, blockFace, + packet.cursorPositionX, packet.cursorPositionY, packet.cursorPositionZ)); // Block consuming if (playerBlockPlaceEvent.doesConsumeBlock()) { // Consume the block in the player's hand From ebb179da7c2ad29b9dea95056861f4514a876621 Mon Sep 17 00:00:00 2001 From: TheMode Date: Mon, 30 Aug 2021 19:47:30 +0200 Subject: [PATCH 42/56] You didn't see anything --- .../net/minestom/server/listener/BlockPlacementListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/listener/BlockPlacementListener.java b/src/main/java/net/minestom/server/listener/BlockPlacementListener.java index 662575af6..1cc3a06a8 100644 --- a/src/main/java/net/minestom/server/listener/BlockPlacementListener.java +++ b/src/main/java/net/minestom/server/listener/BlockPlacementListener.java @@ -157,7 +157,7 @@ public class BlockPlacementListener { return; } // Place the block - instance.placeBlock(new BlockHandler.PlayerPlacement(resultBlock, instance, blockPosition, player, hand, blockFace, + instance.placeBlock(new BlockHandler.PlayerPlacement(resultBlock, instance, placementPosition, player, hand, blockFace, packet.cursorPositionX, packet.cursorPositionY, packet.cursorPositionZ)); // Block consuming if (playerBlockPlaceEvent.doesConsumeBlock()) { From 82916397491b7c45cd398bbb0db2639ed060fb54 Mon Sep 17 00:00:00 2001 From: Matt Worzala Date: Mon, 30 Aug 2021 17:51:19 -0400 Subject: [PATCH 43/56] add BundleMeta and full item to/from nbt --- .../net/minestom/server/item/ItemStack.java | 21 +++++ .../server/item/ItemStackBuilder.java | 1 + .../server/item/metadata/BundleMeta.java | 82 +++++++++++++++++++ src/test/java/demo/PlayerInit.java | 9 ++ 4 files changed, 113 insertions(+) create mode 100644 src/main/java/net/minestom/server/item/metadata/BundleMeta.java diff --git a/src/main/java/net/minestom/server/item/ItemStack.java b/src/main/java/net/minestom/server/item/ItemStack.java index 9d42260dc..ea98bea59 100644 --- a/src/main/java/net/minestom/server/item/ItemStack.java +++ b/src/main/java/net/minestom/server/item/ItemStack.java @@ -7,6 +7,7 @@ import net.minestom.server.item.rule.VanillaStackingRule; import net.minestom.server.tag.Tag; import net.minestom.server.tag.TagReadable; import net.minestom.server.utils.NBTUtils; +import net.minestom.server.utils.validate.Check; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -79,6 +80,18 @@ public final class ItemStack implements TagReadable, HoverEventSource { + + private final List items; + + protected BundleMeta(ItemMetaBuilder metaBuilder, + @NotNull List items) { + super(metaBuilder); + this.items = List.copyOf(items); + } + + public @NotNull List getItems() { + return items; + } + + public static class Builder extends ItemMetaBuilder { + private List items = new ArrayList<>(); + + public Builder items(@NotNull List items) { + this.items = items; + updateItems(); + return this; + } + + public Builder addItem(@NotNull ItemStack item) { + items.add(item); + updateItems(); + return this; + } + + public Builder removeItem(@NotNull ItemStack item) { + items.remove(item); + updateItems(); + return this; + } + + @Override + public @NotNull BundleMeta build() { + return new BundleMeta(this, items); + } + + private void updateItems() { + mutateNbt(compound -> { + NBTList itemList = new NBTList<>(NBTTypes.TAG_Compound); + for (ItemStack item : items) { + itemList.add(item.toItemNBT()); + } + compound.set("Items", itemList); + }); + } + + @Override + public void read(@NotNull NBTCompound nbtCompound) { + if (nbtCompound.containsKey("Items")) { + final NBTList items = nbtCompound.getList("Items"); + for (NBTCompound item : items) { + this.items.add(ItemStack.fromItemNBT(item)); + } + } + } + + @Override + protected @NotNull Supplier<@NotNull ItemMetaBuilder> getSupplier() { + return Builder::new; + } + } +} diff --git a/src/test/java/demo/PlayerInit.java b/src/test/java/demo/PlayerInit.java index aa833918b..cd867eb8a 100644 --- a/src/test/java/demo/PlayerInit.java +++ b/src/test/java/demo/PlayerInit.java @@ -29,6 +29,7 @@ import net.minestom.server.inventory.Inventory; import net.minestom.server.inventory.InventoryType; import net.minestom.server.item.ItemStack; import net.minestom.server.item.Material; +import net.minestom.server.item.metadata.BundleMeta; import net.minestom.server.monitoring.BenchmarkManager; import net.minestom.server.monitoring.TickMonitor; import net.minestom.server.utils.MathUtils; @@ -104,6 +105,14 @@ public class PlayerInit { .canDestroy(Set.of(Block.DIAMOND_ORE))) .build(); player.getInventory().addItemStack(itemStack); + + ItemStack bundle = ItemStack.builder(Material.BUNDLE) + .meta(BundleMeta.class, bundleMetaBuilder -> { + bundleMetaBuilder.addItem(ItemStack.builder(Material.DIAMOND).amount(5).build()); + bundleMetaBuilder.addItem(ItemStack.builder(Material.RABBIT_FOOT).amount(5).build()); + }) + .build(); + player.getInventory().addItemStack(bundle); }); static { From 5b8051e2e8feb6abf3024103e30c2690d2ba442d Mon Sep 17 00:00:00 2001 From: Matt Worzala Date: Mon, 30 Aug 2021 18:00:57 -0400 Subject: [PATCH 44/56] experimental tags, docs, shorten item creation --- .../java/net/minestom/server/item/ItemStack.java | 16 ++++++++++++++-- .../server/item/metadata/BundleMeta.java | 3 +++ src/test/java/demo/PlayerInit.java | 4 ++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/minestom/server/item/ItemStack.java b/src/main/java/net/minestom/server/item/ItemStack.java index ea98bea59..36d644eab 100644 --- a/src/main/java/net/minestom/server/item/ItemStack.java +++ b/src/main/java/net/minestom/server/item/ItemStack.java @@ -80,16 +80,22 @@ public final class ItemStack implements TagReadable, HoverEventSource { - bundleMetaBuilder.addItem(ItemStack.builder(Material.DIAMOND).amount(5).build()); - bundleMetaBuilder.addItem(ItemStack.builder(Material.RABBIT_FOOT).amount(5).build()); + bundleMetaBuilder.addItem(ItemStack.of(Material.DIAMOND, 5)); + bundleMetaBuilder.addItem(ItemStack.of(Material.RABBIT_FOOT, 5)); }) .build(); player.getInventory().addItemStack(bundle); From 868c5ba044e14c9edb6fe4f43db7b592d381825a Mon Sep 17 00:00:00 2001 From: Matt Worzala Date: Mon, 30 Aug 2021 18:04:46 -0400 Subject: [PATCH 45/56] add array copy --- src/main/java/net/minestom/server/item/metadata/BundleMeta.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/item/metadata/BundleMeta.java b/src/main/java/net/minestom/server/item/metadata/BundleMeta.java index 1ca219a75..387ef2b66 100644 --- a/src/main/java/net/minestom/server/item/metadata/BundleMeta.java +++ b/src/main/java/net/minestom/server/item/metadata/BundleMeta.java @@ -33,7 +33,7 @@ public class BundleMeta extends ItemMeta implements ItemMetaBuilder.Provider items = new ArrayList<>(); public Builder items(@NotNull List items) { - this.items = items; + this.items = new ArrayList<>(items); // defensive copy updateItems(); return this; } From 220a4db768a85998c4af0051b864a3753a3c939b Mon Sep 17 00:00:00 2001 From: Matt Worzala Date: Mon, 30 Aug 2021 18:09:08 -0400 Subject: [PATCH 46/56] mark BundleMeta experimental, shorten material nsid access --- src/main/java/net/minestom/server/item/ItemStack.java | 2 +- .../java/net/minestom/server/item/metadata/BundleMeta.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/item/ItemStack.java b/src/main/java/net/minestom/server/item/ItemStack.java index 36d644eab..8c8b55066 100644 --- a/src/main/java/net/minestom/server/item/ItemStack.java +++ b/src/main/java/net/minestom/server/item/ItemStack.java @@ -255,7 +255,7 @@ public final class ItemStack implements TagReadable, HoverEventSource { private final List items; From 063a4dc3927781f3a2e7508fd92138280f8ac807 Mon Sep 17 00:00:00 2001 From: iam4722202468 Date: Tue, 31 Aug 2021 03:45:54 -0400 Subject: [PATCH 47/56] Fix cursor item not being set properly when inventory click event is cancelled --- .../java/net/minestom/server/listener/WindowListener.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/listener/WindowListener.java b/src/main/java/net/minestom/server/listener/WindowListener.java index b84420504..2ac80cd70 100644 --- a/src/main/java/net/minestom/server/listener/WindowListener.java +++ b/src/main/java/net/minestom/server/listener/WindowListener.java @@ -66,8 +66,6 @@ public class WindowListener { successful = inventory.doubleClick(player, slot); } - // Prevent the player from picking a ghost item in cursor - refreshCursorItem(player, inventory); // Prevent ghost item when the click is cancelled if (!successful) { player.getInventory().update(); @@ -75,6 +73,10 @@ public class WindowListener { ((Inventory) inventory).update(player); } } + + // Prevent the player from picking a ghost item in cursor + refreshCursorItem(player, inventory); + // (Why is the ping packet necessary?) PingPacket pingPacket = new PingPacket(); pingPacket.id = (1 << 30) | (windowId << 16); From a69cefd8d42b29c36a92c57ad1284aa25fb9b3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:39:39 +0300 Subject: [PATCH 48/56] Fixing BoundingBoxes caching --- .../server/collision/BoundingBox.java | 155 ++++++++++++------ 1 file changed, 101 insertions(+), 54 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index 447d37430..5a1bc5b20 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -1,5 +1,6 @@ package net.minestom.server.collision; +import com.google.common.collect.ImmutableList; import net.minestom.server.coordinate.Point; import net.minestom.server.coordinate.Pos; import net.minestom.server.coordinate.Vec; @@ -7,8 +8,9 @@ import net.minestom.server.entity.Entity; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collections; import java.util.List; -import java.util.function.Supplier; +import java.util.concurrent.atomic.AtomicReference; /** * See https://wiki.vg/Entity_metadata#Mobs_2 @@ -18,13 +20,72 @@ public class BoundingBox { private final Entity entity; private final double x, y, z; - private volatile Pos lastPosition; - private List bottomFace; - private List topFace; - private List leftFace; - private List rightFace; - private List frontFace; - private List backFace; + private final CachedFace bottomFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace topFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + ); + } + }; + private final CachedFace leftFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace rightFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()) + ); + } + }; + private final CachedFace frontFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()) + ); + } + }; + private final CachedFace backFace = new CachedFace() { + @Override + @NotNull List producePoints() { + return ImmutableList.of( + new Vec(getMinX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + ); + } + }; /** * Creates a {@link BoundingBox} linked to an {@link Entity} and with a specific size. @@ -320,12 +381,7 @@ public class BoundingBox { * @return the points at the bottom of the {@link BoundingBox} */ public @NotNull List getBottomFace() { - this.bottomFace = get(bottomFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()))); - return bottomFace; + return bottomFace.get(); } /** @@ -334,12 +390,7 @@ public class BoundingBox { * @return the points at the top of the {@link BoundingBox} */ public @NotNull List getTopFace() { - this.topFace = get(topFace, () -> - List.of(new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()))); - return topFace; + return topFace.get(); } /** @@ -348,12 +399,7 @@ public class BoundingBox { * @return the points on the left face of the {@link BoundingBox} */ public @NotNull List getLeftFace() { - this.leftFace = get(leftFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()))); - return leftFace; + return leftFace.get(); } /** @@ -362,12 +408,7 @@ public class BoundingBox { * @return the points on the right face of the {@link BoundingBox} */ public @NotNull List getRightFace() { - this.rightFace = get(rightFace, () -> - List.of(new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()))); - return rightFace; + return rightFace.get(); } /** @@ -376,12 +417,7 @@ public class BoundingBox { * @return the points at the front of the {@link BoundingBox} */ public @NotNull List getFrontFace() { - this.frontFace = get(frontFace, () -> - List.of(new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()))); - return frontFace; + return frontFace.get(); } /** @@ -390,12 +426,7 @@ public class BoundingBox { * @return the points at the back of the {@link BoundingBox} */ public @NotNull List getBackFace() { - this.backFace = get(backFace, () -> List.of( - new Vec(getMinX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()))); - return backFace; + return backFace.get(); } @Override @@ -410,18 +441,34 @@ public class BoundingBox { return result; } - private @NotNull List get(@Nullable List face, Supplier> vecSupplier) { - final var lastPos = this.lastPosition; - final var entityPos = entity.getPosition(); - if (face != null && lastPos != null && lastPos.samePoint(entityPos)) { - return face; - } - this.lastPosition = entityPos; - return vecSupplier.get(); - } - private enum Axis { X, Y, Z } + private abstract class CachedFace { + + private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + + abstract @NotNull List producePoints(); + + @NotNull List get() { + return reference.updateAndGet(value -> { + Pos entityPosition = entity.getPosition(); + if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { + value.lastPosition = entityPosition; + value.points = producePoints(); + } + return value; + }).points; + } + + } + + private static class PositionedPoints { + + private @Nullable Pos lastPosition; + private @NotNull List points = Collections.emptyList(); + + } + } From 34ec59dc68d8955b195d9eadbedd2ffc0e4c213e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:52:38 +0300 Subject: [PATCH 49/56] ImmutableList.of() to List.of() --- .../net/minestom/server/collision/BoundingBox.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index 5a1bc5b20..d603979f0 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -1,6 +1,5 @@ package net.minestom.server.collision; -import com.google.common.collect.ImmutableList; import net.minestom.server.coordinate.Point; import net.minestom.server.coordinate.Pos; import net.minestom.server.coordinate.Vec; @@ -23,7 +22,7 @@ public class BoundingBox { private final CachedFace bottomFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMaxZ()), @@ -34,7 +33,7 @@ public class BoundingBox { private final CachedFace topFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), @@ -45,7 +44,7 @@ public class BoundingBox { private final CachedFace leftFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMinX(), getMaxY(), getMinZ()), new Vec(getMinX(), getMaxY(), getMaxZ()), @@ -56,7 +55,7 @@ public class BoundingBox { private final CachedFace rightFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), @@ -67,7 +66,7 @@ public class BoundingBox { private final CachedFace frontFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMinY(), getMinZ()), new Vec(getMaxX(), getMaxY(), getMinZ()), @@ -78,7 +77,7 @@ public class BoundingBox { private final CachedFace backFace = new CachedFace() { @Override @NotNull List producePoints() { - return ImmutableList.of( + return List.of( new Vec(getMinX(), getMinY(), getMaxZ()), new Vec(getMaxX(), getMinY(), getMaxZ()), new Vec(getMaxX(), getMaxY(), getMaxZ()), From 32b33d6bf970d92d63d8e90a5bfb19519ac18837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 15:55:24 +0300 Subject: [PATCH 50/56] Code review --- .../server/collision/BoundingBox.java | 112 +++++++----------- 1 file changed, 43 insertions(+), 69 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index d603979f0..dfe9370fb 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -10,6 +10,7 @@ import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; /** * See https://wiki.vg/Entity_metadata#Mobs_2 @@ -19,72 +20,42 @@ public class BoundingBox { private final Entity entity; private final double x, y, z; - private final CachedFace bottomFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace topFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()) - ); - } - }; - private final CachedFace leftFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace rightFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()) - ); - } - }; - private final CachedFace frontFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMinY(), getMinZ()), - new Vec(getMaxX(), getMaxY(), getMinZ()), - new Vec(getMinX(), getMaxY(), getMinZ()) - ); - } - }; - private final CachedFace backFace = new CachedFace() { - @Override - @NotNull List producePoints() { - return List.of( - new Vec(getMinX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMinY(), getMaxZ()), - new Vec(getMaxX(), getMaxY(), getMaxZ()), - new Vec(getMinX(), getMaxY(), getMaxZ()) - ); - } - }; + private final CachedFace bottomFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + )); + private final CachedFace topFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + )); + private final CachedFace leftFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMinY(), getMaxZ()) + )); + private final CachedFace rightFace = new CachedFace(() -> List.of( + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()) + )); + private final CachedFace frontFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMinY(), getMinZ()), + new Vec(getMaxX(), getMaxY(), getMinZ()), + new Vec(getMinX(), getMaxY(), getMinZ()) + )); + private final CachedFace backFace = new CachedFace(() -> List.of( + new Vec(getMinX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMinY(), getMaxZ()), + new Vec(getMaxX(), getMaxY(), getMaxZ()), + new Vec(getMinX(), getMaxY(), getMaxZ()) + )); /** * Creates a {@link BoundingBox} linked to an {@link Entity} and with a specific size. @@ -444,18 +415,21 @@ public class BoundingBox { X, Y, Z } - private abstract class CachedFace { + private class CachedFace { private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + private final Supplier<@NotNull List> faceProducer; - abstract @NotNull List producePoints(); + private CachedFace(Supplier<@NotNull List> faceProducer) { + this.faceProducer = faceProducer; + } @NotNull List get() { return reference.updateAndGet(value -> { Pos entityPosition = entity.getPosition(); if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { value.lastPosition = entityPosition; - value.points = producePoints(); + value.points = faceProducer.get(); } return value; }).points; From 224345853e2c665644e69073145df881a1239a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A8=D0=B0=D0=BD=D0=B4=D1=83=D1=80=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=BE=20=D0=9A=D0=BE=D0=BD=D1=81=D1=82=D0=B0=D0=BD=D1=82=D0=B8?= =?UTF-8?q?=D0=BD=20=D0=92=D0=BB=D0=B0=D0=B4=D0=B8=D0=BC=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B8=D1=87?= Date: Tue, 31 Aug 2021 16:16:43 +0300 Subject: [PATCH 51/56] BoundingBox#PositionedPoints is immutable now --- .../minestom/server/collision/BoundingBox.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/minestom/server/collision/BoundingBox.java b/src/main/java/net/minestom/server/collision/BoundingBox.java index dfe9370fb..7aca98339 100644 --- a/src/main/java/net/minestom/server/collision/BoundingBox.java +++ b/src/main/java/net/minestom/server/collision/BoundingBox.java @@ -417,7 +417,7 @@ public class BoundingBox { private class CachedFace { - private final AtomicReference<@NotNull PositionedPoints> reference = new AtomicReference<>(new PositionedPoints()); + private final AtomicReference<@Nullable PositionedPoints> reference = new AtomicReference<>(null); private final Supplier<@NotNull List> faceProducer; private CachedFace(Supplier<@NotNull List> faceProducer) { @@ -425,11 +425,11 @@ public class BoundingBox { } @NotNull List get() { + //noinspection ConstantConditions return reference.updateAndGet(value -> { Pos entityPosition = entity.getPosition(); - if (value.lastPosition == null || !value.lastPosition.samePoint(entityPosition)) { - value.lastPosition = entityPosition; - value.points = faceProducer.get(); + if (value == null || !value.lastPosition.samePoint(entityPosition)) { + return new PositionedPoints(entityPosition, faceProducer.get()); } return value; }).points; @@ -439,8 +439,13 @@ public class BoundingBox { private static class PositionedPoints { - private @Nullable Pos lastPosition; - private @NotNull List points = Collections.emptyList(); + private final @NotNull Pos lastPosition; + private final @NotNull List points; + + private PositionedPoints(@NotNull Pos lastPosition, @NotNull List points) { + this.lastPosition = lastPosition; + this.points = points; + } } From 9bb40435712090076de62ad81eb2952239d9ab4b Mon Sep 17 00:00:00 2001 From: TheMode Date: Wed, 1 Sep 2021 11:56:09 +0200 Subject: [PATCH 52/56] Remove legacy weirdness Signed-off-by: TheMode --- .../server/attribute/AttributeModifier.java | 4 +--- .../net/minestom/server/utils/UniqueIdUtils.java | 13 +++---------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/minestom/server/attribute/AttributeModifier.java b/src/main/java/net/minestom/server/attribute/AttributeModifier.java index 3cdd9be0f..2bcb99e08 100644 --- a/src/main/java/net/minestom/server/attribute/AttributeModifier.java +++ b/src/main/java/net/minestom/server/attribute/AttributeModifier.java @@ -1,10 +1,8 @@ package net.minestom.server.attribute; -import net.minestom.server.utils.UniqueIdUtils; import org.jetbrains.annotations.NotNull; import java.util.UUID; -import java.util.concurrent.ThreadLocalRandom; /** * Represent an attribute modifier. @@ -24,7 +22,7 @@ public class AttributeModifier { * @param operation the operation to apply this modifier with */ public AttributeModifier(@NotNull String name, float amount, @NotNull AttributeOperation operation) { - this(UniqueIdUtils.createRandomUUID(ThreadLocalRandom.current()), name, amount, operation); + this(UUID.randomUUID(), name, amount, operation); } /** diff --git a/src/main/java/net/minestom/server/utils/UniqueIdUtils.java b/src/main/java/net/minestom/server/utils/UniqueIdUtils.java index 09f0c70df..242a6a1b5 100644 --- a/src/main/java/net/minestom/server/utils/UniqueIdUtils.java +++ b/src/main/java/net/minestom/server/utils/UniqueIdUtils.java @@ -1,14 +1,15 @@ package net.minestom.server.utils; -import java.util.Random; +import org.jetbrains.annotations.ApiStatus; + import java.util.UUID; import java.util.regex.Pattern; /** * An utilities class for {@link UUID}. */ +@ApiStatus.Internal public final class UniqueIdUtils { - public static final Pattern UNIQUE_ID_PATTERN = Pattern.compile("\\b[0-9a-f]{8}\\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\\b[0-9a-f]{12}\\b"); /** @@ -20,12 +21,4 @@ public final class UniqueIdUtils { public static boolean isUniqueId(String input) { return input.matches(UNIQUE_ID_PATTERN.pattern()); } - - public static UUID createRandomUUID(Random random) { - long most = random.nextLong() & -61441L | 16384L; - long least = random.nextLong() & 4611686018427387903L | Long.MAX_VALUE; - - return new UUID(most, least); - } - } From 0f2d850dce56c5fc174e729cdea0f82f67c287dd Mon Sep 17 00:00:00 2001 From: TheMode Date: Wed, 1 Sep 2021 17:15:49 +0200 Subject: [PATCH 53/56] More Throwable handling Signed-off-by: TheMode --- .../java/net/minestom/server/exception/ExceptionManager.java | 1 + src/main/java/net/minestom/server/thread/ThreadProvider.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/minestom/server/exception/ExceptionManager.java b/src/main/java/net/minestom/server/exception/ExceptionManager.java index e8964bf34..6abeb3500 100644 --- a/src/main/java/net/minestom/server/exception/ExceptionManager.java +++ b/src/main/java/net/minestom/server/exception/ExceptionManager.java @@ -15,6 +15,7 @@ public final class ExceptionManager { * @param e the occurred exception */ public void handleException(Throwable e) { + // TODO handle OOM exceptions this.getExceptionHandler().handleException(e); } diff --git a/src/main/java/net/minestom/server/thread/ThreadProvider.java b/src/main/java/net/minestom/server/thread/ThreadProvider.java index 467d2be36..084b0c9a7 100644 --- a/src/main/java/net/minestom/server/thread/ThreadProvider.java +++ b/src/main/java/net/minestom/server/thread/ThreadProvider.java @@ -135,7 +135,7 @@ public abstract class ThreadProvider { return; try { chunk.tick(time); - } catch (Exception e) { + } catch (Throwable e) { MinecraftServer.getExceptionManager().handleException(e); } final var entities = chunkEntry.entities; @@ -148,7 +148,7 @@ public abstract class ThreadProvider { } try { entity.tick(time); - } catch (Exception e) { + } catch (Throwable e) { MinecraftServer.getExceptionManager().handleException(e); } } From 463a46ccc0e1ef352d5eef4eb1fe638e82fe2439 Mon Sep 17 00:00:00 2001 From: TheMode Date: Wed, 1 Sep 2021 18:21:51 +0200 Subject: [PATCH 54/56] Set socket size to max packet size Signed-off-by: TheMode --- src/main/java/net/minestom/server/network/socket/Server.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/network/socket/Server.java b/src/main/java/net/minestom/server/network/socket/Server.java index 36ad7a262..2725fac9f 100644 --- a/src/main/java/net/minestom/server/network/socket/Server.java +++ b/src/main/java/net/minestom/server/network/socket/Server.java @@ -18,8 +18,8 @@ public final class Server { public static final Logger LOGGER = LoggerFactory.getLogger(Server.class); public static final int WORKER_COUNT = Integer.getInteger("minestom.workers", Runtime.getRuntime().availableProcessors()); - public static final int SOCKET_BUFFER_SIZE = Integer.getInteger("minestom.buffer-size", 1_048_575); public static final int MAX_PACKET_SIZE = 2_097_151; // 3 bytes var-int + public static final int SOCKET_BUFFER_SIZE = Integer.getInteger("minestom.buffer-size", MAX_PACKET_SIZE); public static final boolean NO_DELAY = true; private volatile boolean stop; From 399eb860a7aa9aa7737d4ea0baf16317d297bd2a Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 2 Sep 2021 15:44:36 +0200 Subject: [PATCH 55/56] Improve performance when slow clients are connected Signed-off-by: TheMode --- .../player/PlayerSocketConnection.java | 68 ++++++++++++------- .../server/network/socket/Worker.java | 1 + .../server/utils/binary/BinaryBuffer.java | 17 +++-- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java index 2fbba6aba..84e09b34a 100644 --- a/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java +++ b/src/main/java/net/minestom/server/network/player/PlayerSocketConnection.java @@ -12,7 +12,6 @@ import net.minestom.server.network.packet.FramedPacket; import net.minestom.server.network.packet.server.ComponentHoldingServerPacket; import net.minestom.server.network.packet.server.ServerPacket; import net.minestom.server.network.packet.server.login.SetCompressionPacket; -import net.minestom.server.network.socket.Server; import net.minestom.server.network.socket.Worker; import net.minestom.server.utils.PacketUtils; import net.minestom.server.utils.Utils; @@ -32,10 +31,9 @@ import java.net.SocketAddress; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.channels.SocketChannel; -import java.util.Map; -import java.util.Objects; -import java.util.UUID; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.zip.DataFormatException; /** @@ -46,6 +44,8 @@ import java.util.zip.DataFormatException; @ApiStatus.Internal public class PlayerSocketConnection extends PlayerConnection { private final static Logger LOGGER = LoggerFactory.getLogger(PlayerSocketConnection.class); + private final static Queue POOLED_BUFFERS = new ConcurrentLinkedQueue<>(); + private final static int BUFFER_SIZE = 262_143; private final Worker worker; private final SocketChannel channel; @@ -73,7 +73,9 @@ public class PlayerSocketConnection extends PlayerConnection { private UUID bungeeUuid; private PlayerSkin bungeeSkin; - private final BinaryBuffer tickBuffer = BinaryBuffer.ofSize(Server.SOCKET_BUFFER_SIZE); + private final Object bufferLock = new Object(); + private final List waitingBuffers = new ArrayList<>(); + private BinaryBuffer tickBuffer = BinaryBuffer.ofSize(BUFFER_SIZE); private volatile BinaryBuffer cacheBuffer; public PlayerSocketConnection(@NotNull Worker worker, @NotNull SocketChannel channel, SocketAddress remoteAddress) { @@ -227,7 +229,7 @@ public class PlayerSocketConnection extends PlayerConnection { } public void write(@NotNull ByteBuffer buffer) { - synchronized (tickBuffer) { + synchronized (bufferLock) { if (!tickBuffer.canWrite(buffer.position())) { // Tick buffer is full, flush before appending flush(); @@ -246,7 +248,7 @@ public class PlayerSocketConnection extends PlayerConnection { } public void writeAndFlush(@NotNull ServerPacket packet) { - synchronized (tickBuffer) { + synchronized (bufferLock) { write(packet); flush(); } @@ -255,27 +257,42 @@ public class PlayerSocketConnection extends PlayerConnection { @Override public void flush() { if (!channel.isOpen()) return; - synchronized (tickBuffer) { - if (tickBuffer.readableBytes() == 0) return; - try { - if (encrypted) { - final Cipher cipher = encryptCipher; - // Encrypt data first - final int remainingBytes = tickBuffer.readableBytes(); - final byte[] bytes = tickBuffer.readRemainingBytes(); - byte[] outTempArray = new byte[cipher.getOutputSize(remainingBytes)]; + if (tickBuffer.readableBytes() == 0 && waitingBuffers.isEmpty()) return; + synchronized (bufferLock) { + if (tickBuffer.readableBytes() == 0 && waitingBuffers.isEmpty()) return; + if (encrypted) { + final Cipher cipher = encryptCipher; + // Encrypt data first + final int remainingBytes = tickBuffer.readableBytes(); + final byte[] bytes = tickBuffer.readRemainingBytes(); + byte[] outTempArray = new byte[cipher.getOutputSize(remainingBytes)]; + try { cipher.update(bytes, 0, remainingBytes, outTempArray); - this.tickBuffer.clear(); - this.tickBuffer.writeBytes(outTempArray); + } catch (ShortBufferException e) { + MinecraftServer.getExceptionManager().handleException(e); } - this.tickBuffer.writeChannel(channel); - } catch (IOException e) { - MinecraftServer.getExceptionManager().handleException(e); - } catch (ShortBufferException e) { - e.printStackTrace(); - } finally { this.tickBuffer.clear(); + this.tickBuffer.writeBytes(outTempArray); } + + this.waitingBuffers.add(tickBuffer); + Iterator iterator = waitingBuffers.iterator(); + while (iterator.hasNext()) { + BinaryBuffer waitingBuffer = iterator.next(); + try { + if (!waitingBuffer.writeChannel(channel)) break; + iterator.remove(); + waitingBuffer.clear(); + POOLED_BUFFERS.add(waitingBuffer); + } catch (IOException e) { + MinecraftServer.getExceptionManager().handleException(e); + } + } + // Update tick buffer + BinaryBuffer newBuffer = POOLED_BUFFERS.poll(); + if (newBuffer == null) newBuffer = BinaryBuffer.ofSize(BUFFER_SIZE); + newBuffer.clear(); + this.tickBuffer = newBuffer; } } @@ -299,6 +316,9 @@ public class PlayerSocketConnection extends PlayerConnection { @Override public void disconnect() { this.worker.disconnect(this, channel); + synchronized (bufferLock) { + POOLED_BUFFERS.addAll(waitingBuffers); + } } public @NotNull SocketChannel getChannel() { diff --git a/src/main/java/net/minestom/server/network/socket/Worker.java b/src/main/java/net/minestom/server/network/socket/Worker.java index 54b94d61d..eb23c87f8 100644 --- a/src/main/java/net/minestom/server/network/socket/Worker.java +++ b/src/main/java/net/minestom/server/network/socket/Worker.java @@ -84,6 +84,7 @@ public final class Worker extends Thread { socket.setSendBufferSize(Server.SOCKET_BUFFER_SIZE); socket.setReceiveBufferSize(Server.SOCKET_BUFFER_SIZE); socket.setTcpNoDelay(Server.NO_DELAY); + socket.setSoTimeout(30 * 1000); // 30 seconds this.selector.wakeup(); } diff --git a/src/main/java/net/minestom/server/utils/binary/BinaryBuffer.java b/src/main/java/net/minestom/server/utils/binary/BinaryBuffer.java index d0a60d40e..4689f624b 100644 --- a/src/main/java/net/minestom/server/utils/binary/BinaryBuffer.java +++ b/src/main/java/net/minestom/server/utils/binary/BinaryBuffer.java @@ -123,16 +123,15 @@ public final class BinaryBuffer { return nioBuffer.position(reader).slice().limit(writer); } - public void writeChannel(WritableByteChannel channel) throws IOException { - var writeBuffer = asByteBuffer(readerOffset, writerOffset); - while (writeBuffer.position() != writeBuffer.limit()) { - final int count = channel.write(writeBuffer); - if (count == -1) { - // EOS - throw new IOException("Disconnected"); - } - this.readerOffset += count; + public boolean writeChannel(WritableByteChannel channel) throws IOException { + var writeBuffer = asByteBuffer(readerOffset, writerOffset - readerOffset); + final int count = channel.write(writeBuffer); + if (count == -1) { + // EOS + throw new IOException("Disconnected"); } + this.readerOffset += count; + return writeBuffer.limit() == writeBuffer.position(); } public void readChannel(ReadableByteChannel channel) throws IOException { From d4e51f562c346fa1e2c8caa4b847ae16013dab0f Mon Sep 17 00:00:00 2001 From: TheMode Date: Thu, 2 Sep 2021 18:44:50 +0200 Subject: [PATCH 56/56] Remove threadlocal buffer cache in ChunkDataPacket Signed-off-by: TheMode --- .../network/packet/server/play/ChunkDataPacket.java | 3 +-- src/main/java/net/minestom/server/utils/PacketUtils.java | 8 +++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java b/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java index 2d552b831..5cd391611 100644 --- a/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java +++ b/src/main/java/net/minestom/server/network/packet/server/play/ChunkDataPacket.java @@ -33,7 +33,6 @@ public class ChunkDataPacket implements ServerPacket { public Map entries = new HashMap<>(); private static final byte CHUNK_SECTION_COUNT = 16; - private static final PacketUtils.LocalCache BLOCK_CACHE = PacketUtils.LocalCache.get("chunk-block-cache", 262_144); /** * Heightmaps NBT, as read from raw packet data. @@ -49,7 +48,7 @@ public class ChunkDataPacket implements ServerPacket { writer.writeInt(chunkX); writer.writeInt(chunkZ); - ByteBuffer blocks = BLOCK_CACHE.get(); + ByteBuffer blocks = PacketUtils.localBuffer(); Int2LongRBTreeMap maskMap = new Int2LongRBTreeMap(); diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index b8c4e93be..12de838ff 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/src/main/java/net/minestom/server/utils/PacketUtils.java @@ -38,6 +38,12 @@ public final class PacketUtils { private PacketUtils() { } + @ApiStatus.Internal + @ApiStatus.Experimental + public static ByteBuffer localBuffer() { + return COMPRESSION_CACHE.get(); + } + /** * Sends a packet to an audience. This method performs the following steps in the * following order: @@ -200,7 +206,7 @@ public final class PacketUtils { @ApiStatus.Internal public static FramedPacket allocateTrimmedPacket(@NotNull ServerPacket packet) { final var temp = PacketUtils.createFramedPacket(packet); - final var buffer= ByteBuffer.allocateDirect(temp.position()).put(temp.flip()).asReadOnlyBuffer(); + final var buffer = ByteBuffer.allocateDirect(temp.position()).put(temp.flip()).asReadOnlyBuffer(); return new FramedPacket(packet.getId(), buffer, packet); }