BreweryX (v3.3.6+) Support by Jsinco · Pull Request #430 · DenizenScript/Depenizen

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

            </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

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

Changed

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

Please approve this, i need Depenizen x BreweryX ASAP!

Jsinco · 2024-08-24T16:15:45Z

Been a bit. Any updates on this?

…full_name

mcmonkey4eva · 2024-12-05T07:17:01Z

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

+        // @description
+        // Returns if the recipe, once created into a brew, has a glint effect.
+        // -->
+        tagProcessor.registerTag(ElementTag.class, "has_glint", ((attribute, object) -> {


stray double parens on these

tal5

Sorry for the delay on the reviews :/, hopefully we can get this merged soon

tal5 · 2025-01-12T15:28:25Z

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

+    @Override
+    public boolean matches(ScriptPath path) {
+        return super.matches(path);
+    }


This looks like a redundant override? although if you want to, can add location matching to these events - just check runInCheck in matches with the player's location and add @Location true to the meta; this isn't a requirement or anything though, so can just leave it out for now if you want to.

tal5 · 2025-01-12T15:29:06Z

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

+        return switch (name) {
+            case "message" -> new ElementTag(event.getDistortedMessage());
+            case "original_message" -> new ElementTag(event.getWrittenMessage());
+            case "player" -> playerTag;


context.player isn't usually needed, it's provided as the linked player in getScriptEntryData.

tal5 · 2025-01-12T15:30:29Z

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

+    @Override
+    public boolean handleDetermination(ScriptPath path, String prefix, ObjectTag value) {
+        if (prefix.equals("message")) {
+            event.setDistortedMessage(value.asElement().asString());
+            return true;
+        }
+        return super.handleDetermination(path, prefix, value);
+    }


Determinations aren't handled that way anymore, there's registerDetermination methods you can call in the constructor (can look at existing updated events for examples).

tal5 · 2025-01-12T15:34:45Z

...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 to set the message to be sent after being distorted.


Need to document the prefix here, see existing event's meta docs for examples.

tal5 · 2025-01-12T15:41:03Z

src/main/java/com/denizenscript/depenizen/bukkit/events/breweryx/BreweryDrinkScriptEvent.java

+
+    // <--[event]
+    // @Events
+    // brewery drink


This feels a little unclear, maybe brewery player drinks brew? or just brewery player drinks?

tal5 · 2025-01-12T16:27:04Z

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

+        // @returns ElementTag
+        // @plugin Depenizen, BreweryX
+        // @description
+        // Returns if the recipe, once created into a brew, has a glint effect.


Returns whether (same for is_valid)

tal5 · 2025-01-12T16:27:20Z

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

+public class BreweryPlayerExtensions {
+
+    public static void register() {
+        // <--[tag]


Newline above meta block

tal5 · 2025-01-12T16:28:10Z

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

+
+        // <--[tag]
+        // @attribute <BreweryRecipeTag.has_glint>
+        // @returns ElementTag


ElementTag(Boolean) (same for other boolean tags)

tal5 · 2025-01-12T16:28:34Z

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

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


ElementTag(Number)

tal5 · 2025-01-12T16:29:37Z

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

+        // @returns ElementTag
+        // @plugin Depenizen, BreweryX
+        // @description
+        // Returns the drunkenness of the brewery player or null if Brewery has no data on the player.


if Brewery has no data on the player. can probably be simplified to if the player isn't drunk? or is brewery's handling more complicated than that?

Jsinco · 2025-01-15T22:19:53Z

Ok, going to get started on this :)

tal5 · 2025-05-04T11:48:42Z

Marking this as a draft for now, feel free to ask here/on Discord if you have any questions about the changes requested

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

pom.xml Outdated

</snapshots>

</repository>

<repository>

<id>spigot-repo</id>

Copy link

Member

mcmonkey4eva Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be here

mcmonkey4eva reviewed Jun 7, 2024

View reviewed changes

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

mcmonkey4eva reviewed Jun 7, 2024

View reviewed changes

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

TomLewis mentioned this pull request Aug 18, 2024

Support CommandHelper Scripts as well as Depenizen BreweryTeam/BreweryX#22

Closed

Merge branch 'DenizenScript:master' into master

0e573d7

Jsinco mentioned this pull request Nov 21, 2024

Conditions for drinking/contexts BreweryTeam/BreweryX#23

Open

Jsinco and others added 2 commits November 21, 2024 23:06

Merge branch 'DenizenScript:master' into master

ba65850

Update BreweryX dependency, fix a couple of things after Aya's review.

49b681d

Jsinco changed the title ~~BreweryX (v3.2.0+) Support~~ BreweryX (v3.3.6+) Support Nov 22, 2024

fix identifier, get names based on quality. Add is_valid, has_glint, …

e96c8c4

…full_name

mcmonkey4eva reviewed Dec 5, 2024

View reviewed changes

Jsinco and others added 9 commits December 5, 2024 12:27

remove double stray parenthesis

2e87411

Merge branch 'DenizenScript:master' into master

877bd48

change wood comment and include BreweryX in BukkitPlugins.md

2dfb00a

Merge remote-tracking branch 'origin/master'

740d367

Merge branch 'DenizenScript:master' into master

a659be6

change where BRecipeTag is created

9254240

oops, left an extra ';'

cf57064

change comment on this event

ed22938

add type context comment to BrewModifyEvent

5f92726

tal5 requested changes Jan 12, 2025

View reviewed changes

Useless overrides, cleanup : 2/25

61b9c38

tal5 marked this pull request as draft May 4, 2025 11:46

Uh oh!

Conversation

Jsinco commented Jun 6, 2024

Uh oh!

CLAassistant commented Jun 6, 2024 • 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!

Choose a reason for hiding this comment

Uh oh!

Jsinco commented Jun 7, 2024

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!

Choose a reason for hiding this comment

Uh oh!

Jsinco commented Jun 8, 2024

Uh oh!

szarkans commented Jun 8, 2024

Uh oh!

Jsinco commented Aug 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tal5 left a comment

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

CLAassistant commented Jun 6, 2024 •

edited

Loading