From 29ec3fe2d5cbf86d38a3cf9434b8d40df7f92915 Mon Sep 17 00:00:00 2001 From: Phillipp Glanz <6745190+TheMeinerLP@users.noreply.github.com> Date: Mon, 27 May 2024 17:41:37 +0200 Subject: [PATCH] [#34] Better Notification System (#35) * [#34] Implement new notification system with tests * [#34] Update demo to use new notification system * [#34] Add deprecation text to old notification system * [#34] Improve javadocs and rename builder implementation * [#34] Improve javadocs and fix tests * [#34] Remove empty lines * [#34] Add package description --- .../java/net/minestom/demo/PlayerInit.java | 24 ++-- .../demo/commands/NotificationCommand.java | 12 +- .../server/advancements/AdvancementTab.java | 4 +- .../notifications/Notification.java | 4 + .../notifications/NotificationCenter.java | 4 + .../server/notifications/Notification.java | 117 ++++++++++++++++++ .../notifications/NotificationBuilder.java | 60 +++++++++ .../notifications/NotificationImpl.java | 58 +++++++++ .../server/notifications/package-info.java | 12 ++ .../NotificationIntegrationTest.java | 58 +++++++++ 10 files changed, 335 insertions(+), 18 deletions(-) create mode 100644 src/main/java/net/minestom/server/notifications/Notification.java create mode 100644 src/main/java/net/minestom/server/notifications/NotificationBuilder.java create mode 100644 src/main/java/net/minestom/server/notifications/NotificationImpl.java create mode 100644 src/main/java/net/minestom/server/notifications/package-info.java create mode 100644 src/test/java/net/minestom/server/notifications/NotificationIntegrationTest.java diff --git a/demo/src/main/java/net/minestom/demo/PlayerInit.java b/demo/src/main/java/net/minestom/demo/PlayerInit.java index 81974b74979..0aecc87244d 100644 --- a/demo/src/main/java/net/minestom/demo/PlayerInit.java +++ b/demo/src/main/java/net/minestom/demo/PlayerInit.java @@ -3,8 +3,6 @@ import net.kyori.adventure.text.Component; import net.minestom.server.MinecraftServer; import net.minestom.server.advancements.FrameType; -import net.minestom.server.advancements.notifications.Notification; -import net.minestom.server.advancements.notifications.NotificationCenter; import net.minestom.server.adventure.MinestomAdventure; import net.minestom.server.adventure.audience.Audiences; import net.minestom.server.coordinate.Pos; @@ -19,7 +17,15 @@ import net.minestom.server.event.entity.EntityAttackEvent; import net.minestom.server.event.item.ItemDropEvent; import net.minestom.server.event.item.PickupItemEvent; -import net.minestom.server.event.player.*; +import net.minestom.server.event.player.AsyncPlayerConfigurationEvent; +import net.minestom.server.event.player.PlayerBlockInteractEvent; +import net.minestom.server.event.player.PlayerBlockPlaceEvent; +import net.minestom.server.event.player.PlayerDeathEvent; +import net.minestom.server.event.player.PlayerDisconnectEvent; +import net.minestom.server.event.player.PlayerPacketEvent; +import net.minestom.server.event.player.PlayerPacketOutEvent; +import net.minestom.server.event.player.PlayerSpawnEvent; +import net.minestom.server.event.player.PlayerUseItemOnBlockEvent; import net.minestom.server.event.server.ServerTickMonitorEvent; import net.minestom.server.instance.Instance; import net.minestom.server.instance.InstanceContainer; @@ -33,6 +39,7 @@ import net.minestom.server.item.metadata.BundleMeta; import net.minestom.server.monitoring.BenchmarkManager; import net.minestom.server.monitoring.TickMonitor; +import net.minestom.server.notifications.Notification; import net.minestom.server.utils.MathUtils; import net.minestom.server.utils.time.TimeUnit; import net.minestom.server.world.DimensionType; @@ -115,12 +122,11 @@ public class PlayerInit { player.getInventory().addItemStack(bundle); if (event.isFirstSpawn()) { - Notification notification = new Notification( - Component.text("Welcome!"), - FrameType.TASK, - Material.IRON_SWORD - ); - NotificationCenter.send(notification, event.getPlayer()); + Notification notification = Notification.builder() + .frameType(FrameType.TASK) + .title(Component.text("Welcome!")) + .icon(Material.IRON_SWORD).build(); + notification.send(player); } }) .addListener(PlayerPacketOutEvent.class, event -> { diff --git a/demo/src/main/java/net/minestom/demo/commands/NotificationCommand.java b/demo/src/main/java/net/minestom/demo/commands/NotificationCommand.java index d5fc7453ed1..e5667972278 100644 --- a/demo/src/main/java/net/minestom/demo/commands/NotificationCommand.java +++ b/demo/src/main/java/net/minestom/demo/commands/NotificationCommand.java @@ -2,12 +2,9 @@ import net.kyori.adventure.text.Component; import net.minestom.server.advancements.FrameType; -import net.minestom.server.advancements.notifications.Notification; -import net.minestom.server.advancements.notifications.NotificationCenter; import net.minestom.server.command.builder.Command; import net.minestom.server.entity.Player; import net.minestom.server.item.Material; -import org.jetbrains.annotations.NotNull; public class NotificationCommand extends Command { public NotificationCommand() { @@ -15,9 +12,12 @@ public NotificationCommand() { setDefaultExecutor((sender, context) -> { var player = (Player) sender; - - var notification = new Notification(Component.text("Hello World!"), FrameType.GOAL, Material.DIAMOND_AXE); - NotificationCenter.send(notification, player); + var notification = net.minestom.server.notifications.Notification.builder() + .title(Component.text("Hello World!")) + .frameType(FrameType.GOAL) + .icon(Material.DIAMOND_AXE) + .build(); + notification.send(player); }); } } diff --git a/src/main/java/net/minestom/server/advancements/AdvancementTab.java b/src/main/java/net/minestom/server/advancements/AdvancementTab.java index 8228a5ddd1e..0f39baf1431 100644 --- a/src/main/java/net/minestom/server/advancements/AdvancementTab.java +++ b/src/main/java/net/minestom/server/advancements/AdvancementTab.java @@ -155,10 +155,8 @@ private void addPlayer(@NotNull Player player) { */ private void removePlayer(@NotNull Player player) { final UUID uuid = player.getUuid(); - if (!PLAYER_TAB_MAP.containsKey(uuid)) { - return; - } Set tabs = PLAYER_TAB_MAP.get(uuid); + if (tabs == null) return; tabs.remove(this); if (tabs.isEmpty()) { PLAYER_TAB_MAP.remove(uuid); diff --git a/src/main/java/net/minestom/server/advancements/notifications/Notification.java b/src/main/java/net/minestom/server/advancements/notifications/Notification.java index 4d543a0aae9..e84bf5339aa 100644 --- a/src/main/java/net/minestom/server/advancements/notifications/Notification.java +++ b/src/main/java/net/minestom/server/advancements/notifications/Notification.java @@ -8,7 +8,11 @@ /** * Represents a message which can be sent using the {@link NotificationCenter}. + * @since 1.0.0 + * @deprecated As of Minestom 22a8ccabfae38c53df0605000aa7eed49765c1ab, because the Maintainability is very hard and + * can break everytime from Mojang side because bad api design use {@link net.minestom.server.notifications.Notification#builder()} instead. */ +@Deprecated(since = "1.4.1", forRemoval = true) public record Notification(@NotNull Component title, @NotNull FrameType frameType, @NotNull ItemStack icon) { public Notification(@NotNull Component title, @NotNull FrameType frameType, @NotNull Material icon) { this(title, frameType, ItemStack.of(icon)); diff --git a/src/main/java/net/minestom/server/advancements/notifications/NotificationCenter.java b/src/main/java/net/minestom/server/advancements/notifications/NotificationCenter.java index 5c5326b3c35..968c75fbd21 100644 --- a/src/main/java/net/minestom/server/advancements/notifications/NotificationCenter.java +++ b/src/main/java/net/minestom/server/advancements/notifications/NotificationCenter.java @@ -9,12 +9,16 @@ import java.util.List; /** + * @since 1.0.0 * Used to send one or multiples {@link Notification}. *

* Works by sending a completed advancement and remove it immediately. *

* You can simply create a {@link Notification} object and call {@link #send(Notification, Player)}. + * @deprecated As of Minestom 22a8ccabfae38c53df0605000aa7eed49765c1ab, because the Maintainability is very hard and + * can break everytime from Mojang side because bad api design use {@link net.minestom.server.notifications.Notification#builder()} instead. */ +@Deprecated(since = "1.4.1", forRemoval = true) public final class NotificationCenter { private static final String IDENTIFIER = "minestom:notification"; private static final AdvancementsPacket REMOVE_PACKET = new AdvancementsPacket(false, List.of(), List.of(IDENTIFIER), List.of()); diff --git a/src/main/java/net/minestom/server/notifications/Notification.java b/src/main/java/net/minestom/server/notifications/Notification.java new file mode 100644 index 00000000000..90ee6fc4dbc --- /dev/null +++ b/src/main/java/net/minestom/server/notifications/Notification.java @@ -0,0 +1,117 @@ +package net.minestom.server.notifications; + +import net.kyori.adventure.text.Component; +import net.minestom.server.advancements.FrameType; +import net.minestom.server.entity.Player; +import net.minestom.server.item.ItemStack; +import net.minestom.server.item.Material; +import net.minestom.server.network.packet.server.play.AdvancementsPacket; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; +import java.util.List; + +/** + * Is used to send temporary advancements to the client, which are called notifications. + *
+ * Here is an example of its use: + * 


+ * Notification notification = Notification.builder()
+ *  .frameType(FrameType.TASK)
+ *  .title(Component.text("Welcome!"))
+ *  .icon(Material.IRON_SWORD).build();
+ * notification.send(player);
+ *
+ * + * The constant {@link #IDENTIFIER} is used for the advancement packet + * The constant {@link #REMOVE_PACKET} is used to remove previous notifications + * @since 1.4.1 + */ +public sealed interface Notification permits NotificationImpl { + + String IDENTIFIER = "minestom:notification"; + AdvancementsPacket REMOVE_PACKET = new AdvancementsPacket(false, List.of(), List.of(IDENTIFIER), List.of()); + + /** + * Creates a new builder instance + * @return an instance of the builder + */ + @Contract(pure = true) + static @NotNull Builder builder() { + return new NotificationBuilder(); + } + + /** + * Send the notification to the client + * @param player to get be sent + */ + void send(@NotNull Player player); + + /** + * Send the notification to a collection of clients + * @param players to get be sent + */ + void send(@NotNull Collection<@NotNull Player> players); + + /** + * Gets the title of the notification as a {@link Component} + * @return the title {@link Component} + */ + @NotNull Component title(); + + /** + * Get the {@link FrameType} of the notification + * @return the type + */ + @NotNull FrameType type(); + + /** + * Get the displayed icon of the notification as {@link ItemStack} + * @return the {@link ItemStack} + */ + @NotNull ItemStack icon(); + + /** + * @since 1.4.1 + */ + sealed interface Builder permits NotificationBuilder { + + /** + * Set the title for a notification as component. + * + * If you're using a resource pack you can use {@link Component#translatable(String)} + * + * @param component to get send to the client + * @return the builder + */ + Builder title(@NotNull Component component); + + /** + * Set the frame typ of the notification + * @param frameType to showed for the client + * @return the builder + */ + Builder frameType(@NotNull FrameType frameType); + + /** + * Set the {@link Material} for the icon + * @param material to be shown to the client + * @return the builder + */ + Builder icon(@NotNull Material material); + + /** + * Set the {@link ItemStack} for the icon + * @param itemStack to be shown to the client + * @return the builder + */ + Builder icon(@NotNull ItemStack itemStack); + + /** + * Returns an instance of the creation notification + * @return the instance + */ + Notification build(); + } +} diff --git a/src/main/java/net/minestom/server/notifications/NotificationBuilder.java b/src/main/java/net/minestom/server/notifications/NotificationBuilder.java new file mode 100644 index 00000000000..97452ee3a55 --- /dev/null +++ b/src/main/java/net/minestom/server/notifications/NotificationBuilder.java @@ -0,0 +1,60 @@ +package net.minestom.server.notifications; + +import net.kyori.adventure.text.Component; +import net.minestom.server.advancements.FrameType; +import net.minestom.server.item.ItemStack; +import net.minestom.server.item.Material; +import org.jetbrains.annotations.NotNull; + +/** + * {@inheritDoc} + */ +final class NotificationBuilder implements Notification.Builder { + private Component title; + private FrameType type; + private ItemStack icon; + + /** + * {@inheritDoc} + */ + @Override + public Notification.Builder title(@NotNull Component component) { + this.title = component; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public Notification.Builder frameType(@NotNull FrameType frameType) { + this.type = frameType; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public Notification.Builder icon(@NotNull Material material) { + this.icon = ItemStack.of(material); + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public Notification.Builder icon(@NotNull ItemStack itemStack) { + this.icon = itemStack; + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public Notification build() { + return new NotificationImpl(title, type, icon); + } +} diff --git a/src/main/java/net/minestom/server/notifications/NotificationImpl.java b/src/main/java/net/minestom/server/notifications/NotificationImpl.java new file mode 100644 index 00000000000..8f03e724c55 --- /dev/null +++ b/src/main/java/net/minestom/server/notifications/NotificationImpl.java @@ -0,0 +1,58 @@ +package net.minestom.server.notifications; + +import net.kyori.adventure.text.Component; +import net.minestom.server.advancements.FrameType; +import net.minestom.server.entity.Player; +import net.minestom.server.item.ItemStack; +import net.minestom.server.network.packet.server.play.AdvancementsPacket; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; +import java.util.List; + +/** + * {@inheritDoc} + */ +record NotificationImpl(@NotNull Component title, @NotNull FrameType type, + @NotNull ItemStack icon) implements Notification { + /** + * {@inheritDoc} + */ + @Override + public void send(@NotNull Player player) { + player.sendPacket(createPacket()); + player.sendPacket(REMOVE_PACKET); + } + + /** + * {@inheritDoc} + */ + @Override + public void send(@NotNull Collection<@NotNull Player> players) { + players.forEach(this::send); + } + + /** + * Create the advancement packet that simulates the notification. + * It's not private because integration tests + * @return the packet + */ + @NotNull AdvancementsPacket createPacket() { + final var displayData = new AdvancementsPacket.DisplayData( + title(), Component.empty(), + icon(), type(), + 0x6, null, 0f, 0f); + + final var criteria = new AdvancementsPacket.Criteria("minestom:some_criteria", + new AdvancementsPacket.CriterionProgress(System.currentTimeMillis())); + + final var advancement = new AdvancementsPacket.Advancement(null, displayData, + List.of(new AdvancementsPacket.Requirement(List.of(criteria.criterionIdentifier()))), + false); + + final var mapping = new AdvancementsPacket.AdvancementMapping(IDENTIFIER, advancement); + final var progressMapping = new AdvancementsPacket.ProgressMapping(IDENTIFIER, + new AdvancementsPacket.AdvancementProgress(List.of(criteria))); + return new AdvancementsPacket(false, List.of(mapping), List.of(), List.of(progressMapping)); + } +} diff --git a/src/main/java/net/minestom/server/notifications/package-info.java b/src/main/java/net/minestom/server/notifications/package-info.java new file mode 100644 index 00000000000..059b6016d8c --- /dev/null +++ b/src/main/java/net/minestom/server/notifications/package-info.java @@ -0,0 +1,12 @@ +/** + * This module contains logic about notification system for the minecraft client. + *

+ * It allows developers to show a toast in the right upper corner with 3 different frame types: + * {@link net.minestom.server.advancements.FrameType#GOAL}, {@link net.minestom.server.advancements.FrameType#TASK} or {@link net.minestom.server.advancements.FrameType#CHALLENGE} + *

+ * + * @since 1.4.1 + * @author TheMeinerLP + * @version 1.0 + */ +package net.minestom.server.notifications; \ No newline at end of file diff --git a/src/test/java/net/minestom/server/notifications/NotificationIntegrationTest.java b/src/test/java/net/minestom/server/notifications/NotificationIntegrationTest.java new file mode 100644 index 00000000000..d7bd61093c2 --- /dev/null +++ b/src/test/java/net/minestom/server/notifications/NotificationIntegrationTest.java @@ -0,0 +1,58 @@ +package net.minestom.server.notifications; + +import net.kyori.adventure.text.Component; +import net.minestom.server.advancements.FrameType; +import net.minestom.server.coordinate.Pos; +import net.minestom.server.item.ItemStack; +import net.minestom.server.item.Material; +import net.minestom.server.network.packet.server.play.AdvancementsPacket; +import net.minestom.testing.Collector; +import net.minestom.testing.Env; +import net.minestom.testing.EnvTest; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; + +@EnvTest +class NotificationIntegrationTest { + + @Test + void testBuilder(Env env) { + var notification = Notification.builder() + .icon(Material.ITEM_FRAME) + .title(Component.text("unit test")) + .frameType(FrameType.TASK) + .build(); + assertEquals(FrameType.TASK, notification.type()); + assertEquals(ItemStack.of(Material.ITEM_FRAME), notification.icon()); + assertEquals(Component.text("unit test"), notification.title()); + } + + @Test + void testSend(Env env) { + var instance = env.createFlatInstance(); + var connection = env.createConnection(); + Collector advancementsPacketCollector = connection.trackIncoming(AdvancementsPacket.class); + var player = connection.connect(instance, new Pos(0, 42, 0)).join(); + var notification = Notification.builder() + .icon(Material.ITEM_FRAME) + .title(Component.text("unit test")) + .frameType(FrameType.TASK) + .build(); + notification.send(player); + advancementsPacketCollector.assertCount(2); + AdvancementsPacket advancementsPacket = advancementsPacketCollector.collect().get(1); + assertNotNull(advancementsPacket); + Optional advancementMapping = advancementsPacket.advancementMappings().stream().findFirst(); + advancementMapping.ifPresent(advancementMapping1 -> { + AdvancementsPacket.Advancement advancement = advancementMapping1.value(); + assertFalse(advancement.sendTelemetryData()); + var displayData = advancement.displayData(); + assertEquals(ItemStack.of(Material.ITEM_FRAME), displayData.icon()); + assertEquals(Component.text("unit test"), displayData.title()); + assertEquals(FrameType.TASK, displayData.frameType()); + }); + } +}