BreweryX (v3.2.0+) Support #430

Jsinco · 2024-06-06T22:11:56Z

Many many months ago in the Denizen Discord, I wanted to figure out how to start the execution of a script from another plugin so I could include Denizen support in BreweryX. After the discussion I decided to implement BreweryX into Depenizen instead. Everything should be compatible starting with release 3.2.0 of BreweryX (which I plan to release soon after this pull request). Hopefully there isn't much I need to change here, I'm a first-time contributer but I did my best to follow the style of other plugin bridges.

BreweryX Github: https://github.com/Jsinco/BreweryX
Spigot page: https://www.spigotmc.org/resources/breweryx.114777/

# Conflicts: # lib

CLAassistant · 2024-06-06T22:12:01Z

All committers have signed the CLA.

mcmonkey4eva · 2024-06-06T22:14:53Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+
+    @Override
+    public boolean couldMatch(ScriptPath path) {
+        return path.eventLower.startsWith("brewery chat distort");


this is the legacy scriptevent format, please check the code in Denizen-Core, Denizen, or dDiscordBot for modern examples

mcmonkey4eva · 2024-06-06T22:15:14Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+
+    @Override
+    public ScriptEntryData getScriptEntryData() {
+        return new BukkitScriptEntryData(new PlayerTag(event.getPlayer()), null);


this should just be new BukkitScriptEntryData(event.getPlayer() iirc

mcmonkey4eva · 2024-06-06T22:15:38Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+
+    @Override
+    public boolean handleDetermination(ScriptPath path, String prefix, ObjectTag value) {
+        if (prefix.equals("message") && value instanceof ElementTag elementTag) {


the ElementTag cast is redundant

Sorry, I didn't understand this one.

if (prefix.equals("message") && value instanceof ElementTag elementTag) { // FIXME - Cast redundant but .toString() returns class and hash?

mcmonkey4eva · 2024-06-06T22:16:55Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BPlayerTag.java

+import org.bukkit.entity.Player;
+
+// TODO: OfflinePlayer support
+public class BPlayerTag implements ObjectTag {


This whole object doesn't make sense -- this should just be an extension of PlayerTag

also identifying players by name hasn't been permissible in Minecraft in around 10 years now, since Minecraft 1.7.6 added UUIDs

mcmonkey4eva · 2024-06-06T22:17:29Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BRecipeTag.java

+public class BRecipeTag implements ObjectTag {
+
+    // <--[ObjectType]
+    // @name BRecipeTag


should use the full BreweryRecipeTag

mcmonkey4eva · 2024-06-06T22:18:32Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BRecipeTag.java

+        // @description
+        // Returns the ID of the recipe as specified in the config.
+        // -->
+        if (attribute.startsWith("id")) {


This is the legacy tag system, again look at modern code or DenizenScript/Denizen#2355 or https://guide.denizenscript.com/guides/contributing/property-dev.html

Jsinco · 2024-06-07T07:59:23Z

I fixed a bunch of the commented issues. Not sure if I got everything 100% but just let me know what needs to be fixed and I'll get on it!

mcmonkey4eva · 2024-06-07T19:01:23Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+    // <context.player> Returns a PlayerTag of the player that had their chat distorted.
+    //
+    // @Determine
+    // ElementTag(String) to set the message to be sent after being distorted.


ElementTag(String) doesn't exist, it's just ElementTag

mcmonkey4eva · 2024-06-07T19:02:27Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryPlayerTag.java

+import org.bukkit.Bukkit;
+import org.bukkit.entity.Player;
+
+public class BreweryPlayerTag implements ObjectTag {


As noted before this object doesn't make sense, it should just be an extension

mcmonkey4eva · 2024-06-07T19:03:12Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        tagProcessor.registerTag(ElementTag.class, "id", (attribute, object) -> {
+            /*
+            This being optional was infrastructure added by the original authors and is not used
+            in Brewery. It will be deprecated and replaced soon.


this comment should just be a // comment not the weird line break multiline comment thing

mcmonkey4eva · 2024-06-07T19:04:04Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+
+    @Override
+    public boolean handleDetermination(ScriptPath path, String prefix, ObjectTag value) {
+        if (prefix.equals("message") && value instanceof ElementTag elementTag) { // FIXME - Cast redundant but .toString() returns class and hash?


I think I might just be confused here. If I don't cast the ObjectTag to an ElementTag I can't do value.asString(). When you told me the cast was redundant I assumed you meant I could just do ObjectTag#toString(). Just looked again and I'm guessing you wanted me to do value.asElement().asString()?

I'm confused why you think toString doesn't work but yes asElement().asString() is the most proper option

Going to the declaration of value.toString() brought me to the Object.java class which is why I why I figured it wouldn't work. Sorry about that

mcmonkey4eva · 2024-06-07T19:04:27Z

pom.xml

@@ -39,6 +39,10 @@
                <enabled>true</enabled>
            </snapshots>
        </repository>
+        <repository>
+            <id>spigot-repo</id>


this shouldn't be here

mcmonkey4eva · 2024-06-07T19:05:56Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+public class BreweryRecipeTag implements ObjectTag {
+
+    // <--[ObjectType]
+    // @name BRecipeTag


the name is still wrong here

mcmonkey4eva · 2024-06-07T19:06:28Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        // @returns ElementTag(Number)
+        // @plugin Depenizen, BreweryX
+        // @description
+        // Returns the distill runs of the recipe


mcmonkey4eva · 2024-06-07T19:06:53Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        // Returns the cooking time of the recipe.
+        // -->
+        tagProcessor.registerTag(ElementTag.class, "cooking_time", (attribute, object) -> {
+            return new ElementTag(object.bRecipe.getCookingTime());


sounds like it should probably be a DurationTag?

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

mcmonkey4eva · 2024-06-07T19:08:55Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        // @returns ElementTag(Number)
+        // @plugin Depenizen, BreweryX
+        // @description
+        // Returns the amount of alcohol in a perfect potion.


What unit is this? Item stacks of an alcohol item, percentage, decimal, ...?

"alcohol: Absolute amount of alcohol 0-100 in a perfect potion (will be added directly to the player, where 100 means fainting)"

Just returns the int written in Brewery's config for the specific recipe. I'll add that longer description to the comment

mcmonkey4eva · 2024-06-07T19:09:44Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+
+        // <--[tag]
+        // @attribute <BRecipeTag.effects>
+        // @returns ListTag(ElementTag)


if it's a list of element just write ListTag

mcmonkey4eva · 2024-06-07T19:10:12Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+            }
+            ListTag cmds = new ListTag();
+            for (Tuple<Integer, String> tuple : object.bRecipe.getPlayercmds()) {
+                cmds.addObject(new ElementTag(tuple.second()));


When constructing an ElementTag just from general plain text mark it explicitly as plaintext with , true

Jsinco · 2024-06-07T20:06:42Z

Fixed the new issues

mcmonkey4eva · 2024-06-08T01:09:23Z

...n/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryChatDistortScriptEvent.java

+
+public class BreweryChatDistortScriptEvent extends BukkitScriptEvent implements Listener {
+
+


stray double newline

mcmonkey4eva · 2024-06-08T01:10:24Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+
+    // <--[ObjectType]
+    // @name BreweryRecipeTag
+    // @prefix brecipe


the prefix is still wrong

mcmonkey4eva · 2024-06-08T01:10:39Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

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


name in meta is still wrong

mcmonkey4eva · 2024-06-08T01:11:07Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        tagProcessor.registerTag(ElementTag.class, "id", (attribute, object) -> {
+
+            // This being optional was infrastructure added by the original authors and is not used
+            // in Brewery. It will be deprecated and replaced soon.


this still shouldn't have all the stray newlines here

this still has a stray newline in the middle of it

mcmonkey4eva · 2024-06-08T01:11:35Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+
+        // <--[tag]
+        // @attribute <BRecipeTag.cooking_time>
+        // @returns ElementTag(Number)


return type here is outdated

mcmonkey4eva · 2024-06-08T01:11:55Z

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java

+        // @returns ElementTag(Number)
+        // @plugin Depenizen, BreweryX
+        // @description
+        // Returns the absolute amount of alcohol 0-100 in a perfect potion (will be added directly to the player, where 100 means fainting)


mcmonkey4eva · 2024-06-08T01:12:25Z

...ain/java/com/denizenscript/depenizen/bukkit/properties/breweryx/BreweryPlayerExtensions.java

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


this labeling is wrong

mcmonkey4eva · 2024-06-08T01:12:42Z

...ain/java/com/denizenscript/depenizen/bukkit/properties/breweryx/BreweryPlayerExtensions.java

+        // @description
+        // Returns the drunkness of the brewery player.
+        // -->
+        PlayerTag.tagProcessor.registerTag(ElementTag.class, "drunkenness", (attribute, object) -> {


format the tags like brewery_drunkenness

mcmonkey4eva · 2024-06-08T01:13:56Z

...ain/java/com/denizenscript/depenizen/bukkit/properties/breweryx/BreweryPlayerExtensions.java

+        // Returns the drunkness of the brewery player.
+        // -->
+        PlayerTag.tagProcessor.registerTag(ElementTag.class, "drunkenness", (attribute, object) -> {
+            BPlayer bPlayer = BPlayer.hasPlayer(object.getPlayerEntity()) ? BPlayer.get(object.getPlayerEntity()) : null;


If this can null out, that should be explain in the tag meta. If this shouldn't have in practice, shouldn't have all the redundant checks

mcmonkey4eva · 2024-06-08T01:14:18Z

...ain/java/com/denizenscript/depenizen/bukkit/properties/breweryx/BreweryPlayerExtensions.java

+        // @description
+        // Returns the quality of the brewery player's drunkenness (drunkeness * drunkeness).
+        // -->
+        PlayerTag.tagProcessor.registerTag(ElementTag.class, "quality", (attribute, object) -> {


should use registerOnlineOnlyTag if it requires online players

Doesn't require them to be online, just for Brewery to have data about them

then use getOfflinePlayer not playerEntity

Jsinco · 2024-06-08T01:33:38Z

Changed

szarkans · 2024-06-08T22:25:40Z

Please approve this, i need Depenizen x BreweryX ASAP!

Jsinco added 6 commits February 29, 2024 14:40

working on it

b681351

working on it

7820362

laptop push

fceacdb

should be ready for pull

dadac47

Merge remote-tracking branch 'origin/master'

b89a1d8

# Conflicts: # lib

BreweryX support

92b1f86

mcmonkey4eva reviewed Jun 6, 2024

View reviewed changes

Jsinco added 4 commits June 6, 2024 21:33

laptop push

ab820b4

code fixes

09bc856

rename classes

3e3b437

add BreweryX dependency

89399ef

add BreweryX dependency

477394b

mcmonkey4eva reviewed Jun 7, 2024

View reviewed changes

src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java Show resolved Hide resolved

mcmonkey4eva reviewed Jun 7, 2024

View reviewed changes

Jsinco added 3 commits June 7, 2024 15:37

fix review issues

1814caa

turn bplayer obj to extension

529119c

Merge branch 'master' of https://github.com/Jsinco/Depenizen

4c78d7f

Jsinco added 2 commits June 7, 2024 17:16

Merge branch 'master' of https://github.com/Jsinco/Depenizen

e1cfe5e

Merge branch 'master' of https://github.com/Jsinco/Depenizen

8d1993d

mcmonkey4eva reviewed Jun 8, 2024

View reviewed changes

review changes

45d41d0

review changes

9862906

stray newline

cdaf64f

mcmonkey4eva approved these changes Jun 9, 2024

View reviewed changes

Jsinco and others added 2 commits June 8, 2024 21:19

register tagProcessor with object init

68644f0

Merge branch 'DenizenScript:master' into master

827ed9b


		public class BreweryChatDistortScriptEvent extends BukkitScriptEvent implements Listener {

BreweryX (v3.2.0+) Support #430

Are you sure you want to change the base?

BreweryX (v3.2.0+) Support #430

Conversation

Jsinco commented Jun 6, 2024

CLAassistant commented Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jsinco commented Jun 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jsinco commented Jun 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jsinco commented Jun 8, 2024

szarkans commented Jun 8, 2024

CLAassistant commented Jun 6, 2024 •

edited

Loading