SuperiorSkyblock Integration by MC-Samuel · Pull Request #446 · DenizenScript/Depenizen

MC-Samuel · 2025-05-10T03:22:15Z

Requested in https://discord.com/channels/315163488085475337/1117046369434218517 and https://discord.com/channels/315163488085475337/1237853546070675527

Resolves #411

mcmonkey4eva · 2025-05-10T03:28:23Z

src/main/java/com/denizenscript/depenizen/bukkit/bridges/SuperiorSkyblockBridge.java

+        ObjectFetcher.registerWithObjectFetcher(SuperiorSkyblockIslandTag.class, SuperiorSkyblockIslandTag.tagProcessor);
+
+        // <--[tag]
+        // @attribute <island[<name>]>


should prefix this tag

mcmonkey4eva · 2025-05-10T03:28:43Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+
+    // <--[ObjectType]
+    // @name SuperiorSkyblockIslandTag
+    // @prefix island


should include the plugin name on the prefix here

mcmonkey4eva · 2025-05-10T03:29:58Z

...zenscript/depenizen/bukkit/properties/superiorskyblock/SuperiorSkyblockPlayerExtensions.java

+    public static void register() {
+
+        // <--[tag]
+        // @attribute <PlayerTag.bypass_mode>


these tags should be prefixed

davight · 2025-05-10T15:48:09Z

...zenscript/depenizen/bukkit/properties/superiorskyblock/SuperiorSkyblockPlayerExtensions.java

+        // Returns whether a player can build on islands that they are not a part of.
+        // -->
+        PlayerTag.tagProcessor.registerTag(ElementTag.class, "superiorskyblock_bypass_mode", (attribute, player) -> {
+            return new ElementTag(SuperiorSkyblockAPI.getPlayer(player.getUUID()).hasBypassModeEnabled());


could probably make a helper method for SuperiorSkyblockAPI.getPlayer(player.getUUID())? it is repeated quite a bit

davight · 2025-05-10T15:57:12Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // <SuperiorSkyblockIslandTag.name>
+        // -->
+        tagProcessor.registerMechanism("name", false, ElementTag.class, (object, mechanism, value) -> {
+            if (!mechanism.getValue().asString().isEmpty() && SuperiorSkyblockAPI.getIsland(mechanism.getValue().asString()) != null) {


is this if check correct? it looks like you're checking whether the island with this name exists

it's a way of preventing two islands from having the same name, since that's their unique identifier in the SuperiorSkyblockIslandTag.

yes but youre checking if the island with the given name is not null therefore if it exists
// I think that what do you want to do is check if the island with given name is null

mcmonkey4eva · 2025-05-10T20:37:48Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // -->
+        tagProcessor.registerMechanism("balance", false, ElementTag.class, (object, mechanism, value) -> {
+            if (mechanism.requireDouble()) {
+                object.getIsland().getIslandBank().setBalance(mechanism.getValue().asBigDecimal());


use value not mechanism.getValue()

mcmonkey4eva · 2025-05-10T20:38:45Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // <SuperiorSkyblockIslandTag.size>
+        // -->
+        tagProcessor.registerMechanism("size", false, ElementTag.class, (object, mechanism, value) -> {
+            if (mechanism.requireInteger() && mechanism.getValue().asInt() >= 1) {


this >= 1 check needs its own else with an echoError (so it doesn't just silently ignore incorrect inputs). Also doc the range limit in the meta

mcmonkey4eva · 2025-05-10T20:39:21Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // <SuperiorSkyblockIslandTag.name>
+        // -->
+        tagProcessor.registerMechanism("name", false, ElementTag.class, (object, mechanism, value) -> {
+            if (!mechanism.getValue().asString().isEmpty() && SuperiorSkyblockAPI.getIsland(mechanism.getValue().asString()) == null) {


needs an else echoError. Also document what the expected limitations are in the meta

- mechanism.echoError additions

…ided location

mcmonkey4eva · 2025-05-14T10:55:10Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // <SuperiorSkyblockIslandTag.name>
+        // -->
+        tagProcessor.registerMechanism("name", false, ElementTag.class, (object, mechanism, value) -> {
+            if (!mechanism.getValue().asString().isEmpty()) {


this check is free-floating - should have an error message

tal5 · 2025-05-15T14:35:50Z

src/main/java/com/denizenscript/depenizen/bukkit/bridges/SuperiorSkyblockBridge.java

+        // @returns SuperiorSkyblockIslandTag
+        // @plugin Depenizen, SuperiorSkyblock
+        // @description
+        // Returns the superiorskyblock island tag with the given name.


Add an object link, // Refer to <@link objecttype SuperiorSkyblockIslandTag>. on a newline is what most object constructor tags have

tal5 · 2025-05-15T14:40:37Z

...cript/depenizen/bukkit/events/superiorskyblock/SuperiorSkyblockIslandCreatedScriptEvent.java

+        this.<SuperiorSkyblockIslandCreatedScriptEvent, ElementTag>registerDetermination("name", ElementTag.class, (evt, context, name) -> {
+            evt.event.getIsland().setName(name.asString());
+        });


Having utility determinations is nice, but I'd probably prioritize the event's actual options - e.g. looking at the JD it seems to let you control whether the player will automatically teleport to their Island & get what schematic the island was created from.

tal5 · 2025-05-15T14:56:45Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+    // @prefix superiorskyblock_island
+    // @base ElementTag
+    // @format
+    // The identity format for group is <island_name>


group?
Also missing .

tal5 · 2025-05-15T14:58:01Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        if (island == null) {
+            return null;
+        }


This should probably have some error message? otherwise you'd have no idea what's going wrong if you provide an invalid name.

tal5 · 2025-05-15T15:05:55Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+    public static boolean matches(String string) {
+        string = string.replace("superiorskyblock_island@", "");
+        return SuperiorSkyblockAPI.getIsland(string) != null;
+    }


Having the prefix is usually treated as a pass, see e.g. WorldGuardRegionTag.

tal5 · 2025-05-15T17:07:01Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // Returns the description of the specified island, if any.
+        // -->
+        tagProcessor.registerTag(ElementTag.class, "description", (attribute, object) -> {
+            return object.getIsland().getDescription().isBlank() ? null : new ElementTag(object.getIsland().getDescription(), true);


A. any specific reason for #isBlank over #isEmpty?
B. Are we sure this should be nullable as opposed to matching what the plugin does and returning an empty element?

tal5 · 2025-05-15T17:17:37Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // <SuperiorSkyblockIslandTag.name>
+        // -->
+        tagProcessor.registerMechanism("name", false, ElementTag.class, (object, mechanism, value) -> {
+            if (mechanism.getValue().asString().isEmpty()) {


Use the value param

tal5 · 2025-05-15T17:20:37Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+                mechanism.echoError("You cannot have an island with no name.");
+            }
+            else if (SuperiorSkyblockAPI.getIsland(value.asString()) != null) {
+                mechanism.echoError("There is already an island with the name '" + value.asString() + "'.");


Nitpick, but no need for #asString here (Java automatically calls #toString on objects when concatenating them with strings)

tal5 · 2025-05-15T17:23:30Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        if (string.startsWith("superiorskyblock_island@")) {
+            string = string.substring("superiorskyblock_island@".length());
+        }
+        Island island = SuperiorSkyblockAPI.getIsland(string);


Islands seem to have both UUIDs and names, probably better to have the objects identify based on the UUIDs rather than names, as the names are changeable.

tal5 · 2025-05-15T17:39:18Z

...zenscript/depenizen/bukkit/properties/superiorskyblock/SuperiorSkyblockPlayerExtensions.java

+        // @input ElementTag(Boolean)
+        // @plugin Depenizen, SuperiorSkyblock
+        // @description
+        // Changes whether a player can build on islands that are not a part of.


islands that are not a part of

they*

mcmonkey4eva · 2025-05-16T14:45:39Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // @returns ElementTag
+        // @plugin Depenizen, SuperiorSkyblock
+        // @description
+        // Returns the description of this island, if any.


What does , if any mean here?

Aya previously asked B. Are we sure this should be nullable as opposed to matching what the plugin does and returning an empty element? and you didn't reply but just went and removed the null without changing the meta.

Reminder ftr that PR comment aren't commands, you're welcome to have answered "Yes null is better" if you had reason to think so.

…ulting in errors

…landtag, slight meta clarification

mcmonkey4eva · 2025-05-30T08:09:09Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // -->
+        tagProcessor.registerTag(ElementTag.class, "balance", (attribute, object) -> {
+            if (object.getIsland().isSpawn()) {
+                Debug.echoError("Spawn islands cannot have a balance.");


tal5 · 2025-06-18T10:11:33Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        if (string.startsWith("superiorskyblock_island@")) {
+            string = string.substring("superiorskyblock_island@".length());
+        }
+        Island island = string.equals("00000000-0000-0000-0000-000000000000") ? SuperiorSkyblockAPI.getSpawnIsland() : SuperiorSkyblockAPI.getIslandByUUID(UUID.fromString(string));


Can use CoreUtilities#tryParseUUID & null check it (and maybe an error message).
Also for this 00 UUID - is that the island's actual UUID? if not it will likely have problems in identify & persisting the object.

Yep, the 00 is the actual uuid of the spawn island.

tal5 · 2025-06-18T10:53:09Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        }
+        Island island = string.equals("00000000-0000-0000-0000-000000000000") ? SuperiorSkyblockAPI.getSpawnIsland() : SuperiorSkyblockAPI.getIslandByUUID(UUID.fromString(string));
+        if (island == null) {
+            Debug.echoError("No island with the uuid '" + string + "' exists.");


Should check the context for the error message, context == null || context.showErrors() - can see existing valueOf methods for examples.

tal5 · 2025-06-18T11:00:02Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // @returns ListTag(PlayerTag)
+        // @plugin Depenizen, SuperiorSkyblock
+        // @description
+        // Returns the members of this island, including the leader.


Nitpick, but probably best to match the rest of the meta here (leader -> owner), assuming it's the same value

tal5 · 2025-06-18T11:07:07Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        // Returns the current average rating of an island.
+        // -->
+        tagProcessor.registerTag(ElementTag.class, "total_rating", (attribute, object) -> {


If this is the average rating it'd probably make sense to name the tag after that?

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

tal5 · 2025-07-10T15:42:23Z

...a/com/denizenscript/depenizen/bukkit/objects/superiorskyblock/SuperiorSkyblockIslandTag.java

+        }
+        try {
+            UUID uuid = UUID.fromString(string);
+            UUID spawnUUID = UUID.fromString("00000000-0000-0000-0000-000000000000");


Should put the UUID in a public static final, this will parse it every time.

tal5 · 2025-07-10T15:54:33Z

...zenscript/depenizen/bukkit/properties/superiorskyblock/SuperiorSkyblockPlayerExtensions.java

+    public SuperiorSkyblockPlayerExtensions(PlayerTag player) {
+        this.player = player;
+    }
+
+    PlayerTag player;


Pretty sure this is redundant?

SuperiorSkyblock Compatibility

b077e0a

mcmonkey4eva reviewed May 10, 2025

View reviewed changes

plugin identifier added

77713ed

davight reviewed May 10, 2025

View reviewed changes

MC-Samuel added 2 commits May 10, 2025 10:58

helper method added

d3988fa

-_-

082a86e

mcmonkey4eva reviewed May 10, 2025

View reviewed changes

MC-Samuel added 2 commits May 10, 2025 13:45

- mechanism.getValue() -> value

1be5361

- mechanism.echoError additions

Added LocationTag.superiorskyblock_island to get island at the prov…

a96c4e1

…ided location

mcmonkey4eva reviewed May 14, 2025

View reviewed changes

fixed empty check

fb1e0ea

mcmonkey4eva approved these changes May 15, 2025

View reviewed changes

tal5 requested changes May 15, 2025

View reviewed changes

aya update

826bdf7

mcmonkey4eva reviewed May 16, 2025

View reviewed changes

meta update

2faa8fc

mcmonkey4eva approved these changes May 16, 2025

View reviewed changes

MC-Samuel added 2 commits May 28, 2025 20:49

preventing spawn islands from having mechanisms adjusted and tags res…

245f6ef

…ulting in errors

adjusted being able to input spawn uuid in the raw superiorskyblockis…

ba66262

…landtag, slight meta clarification

mcmonkey4eva reviewed May 30, 2025

View reviewed changes

attribute

8d2e3c4

MC-Samuel requested a review from tal5 June 3, 2025 04:32

tal5 requested changes Jun 18, 2025

View reviewed changes

MC-Samuel added 2 commits June 18, 2025 07:24

core tag changes, couple tag changes, and slight meta change

cd7d7b0

removed special spawn island handling

4dc2a71

revert previous commit

c34f2b1

tal5 requested changes Jul 10, 2025

View reviewed changes

removed redundant section and moved uuid

2c99ea3

MC-Samuel requested a review from tal5 July 10, 2025 18:39

shortened download link

e3a2831

tal5 approved these changes Jul 20, 2025

View reviewed changes

tal5 changed the title ~~SuperiorSkyblock Compatibility~~ SuperiorSkyblock Integration Jul 20, 2025

MC-Samuel requested a review from mcmonkey4eva July 23, 2025 03:50

MC-Samuel added 8 commits November 11, 2025 19:47

pom bump

d08a232

slight formatting changes

65267ff

cosmetic change

22680ed

added a few more tags and mechanisms

07f19b9

slight change to 'SuperiorSkyblockIslandTag.homes'

82fa337

minor error message fix

d6aeff6

added a new mechanism, fixed some small details

ef8ee36

slight condensation

b70b5aa

MC-Samuel requested a review from tal5 December 23, 2025 02:38

MC-Samuel added 3 commits January 12, 2026 22:08

moved getSuperiorPlayer to the bridge file

b1720f6

applied additional error checks and messages

ddeffb0

four new events

f382088

Uh oh!

Conversation

MC-Samuel commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davight May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcmonkey4eva May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MC-Samuel commented May 10, 2025 •

edited

Loading

davight May 10, 2025 •

edited

Loading

mcmonkey4eva May 10, 2025 •

edited

Loading