BreweryX (v3.3.6+) Support #430
Conversation
|
|
|
|
||
| @Override | ||
| public boolean couldMatch(ScriptPath path) { | ||
| return path.eventLower.startsWith("brewery chat distort"); |
There was a problem hiding this comment.
this is the legacy scriptevent format, please check the code in Denizen-Core, Denizen, or dDiscordBot for modern examples
|
|
||
| @Override | ||
| public ScriptEntryData getScriptEntryData() { | ||
| return new BukkitScriptEntryData(new PlayerTag(event.getPlayer()), null); |
There was a problem hiding this comment.
this should just be new BukkitScriptEntryData(event.getPlayer() iirc
|
|
||
| @Override | ||
| public boolean handleDetermination(ScriptPath path, String prefix, ObjectTag value) { | ||
| if (prefix.equals("message") && value instanceof ElementTag elementTag) { |
There was a problem hiding this comment.
the ElementTag cast is redundant
There was a problem hiding this comment.
Sorry, I didn't understand this one.
if (prefix.equals("message") && value instanceof ElementTag elementTag) { // FIXME - Cast redundant but .toString() returns class and hash?
| import org.bukkit.entity.Player; | ||
|
|
||
| // TODO: OfflinePlayer support | ||
| public class BPlayerTag implements ObjectTag { |
There was a problem hiding this comment.
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
| public class BRecipeTag implements ObjectTag { | ||
|
|
||
| // <--[ObjectType] | ||
| // @name BRecipeTag |
There was a problem hiding this comment.
should use the full BreweryRecipeTag
| // @description | ||
| // Returns the ID of the recipe as specified in the config. | ||
| // --> | ||
| if (attribute.startsWith("id")) { |
There was a problem hiding this comment.
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
|
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! |
| // <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. |
There was a problem hiding this comment.
ElementTag(String) doesn't exist, it's just ElementTag
| import org.bukkit.Bukkit; | ||
| import org.bukkit.entity.Player; | ||
|
|
||
| public class BreweryPlayerTag implements ObjectTag { |
There was a problem hiding this comment.
As noted before this object doesn't make sense, it should just be an extension
| 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. |
There was a problem hiding this comment.
this comment should just be a // comment not the weird line break multiline comment thing
|
|
||
| @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? |
There was a problem hiding this comment.
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()?
There was a problem hiding this comment.
I'm confused why you think toString doesn't work but yes asElement().asString() is the most proper option
There was a problem hiding this comment.
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
pom.xml
Outdated
| @@ -39,6 +39,10 @@ | |||
| <enabled>true</enabled> | |||
| </snapshots> | |||
| </repository> | |||
| <repository> | |||
| <id>spigot-repo</id> | |||
| public class BreweryRecipeTag implements ObjectTag { | ||
|
|
||
| // <--[ObjectType] | ||
| // @name BRecipeTag |
There was a problem hiding this comment.
the name is still wrong here
| // @returns ElementTag(Number) | ||
| // @plugin Depenizen, BreweryX | ||
| // @description | ||
| // Returns the distill runs of the recipe |
| // Returns the cooking time of the recipe. | ||
| // --> | ||
| tagProcessor.registerTag(ElementTag.class, "cooking_time", (attribute, object) -> { | ||
| return new ElementTag(object.bRecipe.getCookingTime()); |
There was a problem hiding this comment.
sounds like it should probably be a DurationTag?
src/main/java/com/denizenscript/depenizen/bukkit/objects/breweryx/BreweryRecipeTag.java
Outdated
Show resolved
Hide resolved
| // @returns ElementTag(Number) | ||
| // @plugin Depenizen, BreweryX | ||
| // @description | ||
| // Returns the amount of alcohol in a perfect potion. |
There was a problem hiding this comment.
What unit is this? Item stacks of an alcohol item, percentage, decimal, ...?
There was a problem hiding this comment.
"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
|
|
||
| // <--[tag] | ||
| // @attribute <BRecipeTag.effects> | ||
| // @returns ListTag(ElementTag) |
There was a problem hiding this comment.
if it's a list of element just write ListTag
| // @description | ||
| // Returns the quality of the brewery player's drunkenness (drunkeness * drunkeness). | ||
| // --> | ||
| PlayerTag.tagProcessor.registerTag(ElementTag.class, "quality", (attribute, object) -> { |
There was a problem hiding this comment.
should use registerOnlineOnlyTag if it requires online players
There was a problem hiding this comment.
Doesn't require them to be online, just for Brewery to have data about them
There was a problem hiding this comment.
then use getOfflinePlayer not playerEntity
|
Changed |
|
Please approve this, i need Depenizen x BreweryX ASAP! |
|
Been a bit. Any updates on this? |
| // @description | ||
| // Returns if the recipe, once created into a brew, has a glint effect. | ||
| // --> | ||
| tagProcessor.registerTag(ElementTag.class, "has_glint", ((attribute, object) -> { |
There was a problem hiding this comment.
stray double parens on these
| @Override | ||
| public boolean matches(ScriptPath path) { | ||
| return super.matches(path); | ||
| } |
There was a problem hiding this comment.
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.
| return switch (name) { | ||
| case "message" -> new ElementTag(event.getDistortedMessage()); | ||
| case "original_message" -> new ElementTag(event.getWrittenMessage()); | ||
| case "player" -> playerTag; |
There was a problem hiding this comment.
context.player isn't usually needed, it's provided as the linked player in getScriptEntryData.
| @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); | ||
| } |
There was a problem hiding this comment.
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).
| // <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. |
There was a problem hiding this comment.
Need to document the prefix here, see existing event's meta docs for examples.
|
|
||
| // <--[event] | ||
| // @Events | ||
| // brewery drink |
There was a problem hiding this comment.
This feels a little unclear, maybe brewery player drinks brew? or just brewery player drinks?
| // @returns ElementTag | ||
| // @plugin Depenizen, BreweryX | ||
| // @description | ||
| // Returns if the recipe, once created into a brew, has a glint effect. |
There was a problem hiding this comment.
Returns whether (same for is_valid)
| public class BreweryPlayerExtensions { | ||
|
|
||
| public static void register() { | ||
| // <--[tag] |
|
|
||
| // <--[tag] | ||
| // @attribute <BreweryRecipeTag.has_glint> | ||
| // @returns ElementTag |
There was a problem hiding this comment.
ElementTag(Boolean) (same for other boolean tags)
| public static void register() { | ||
| // <--[tag] | ||
| // @attribute <PlayerTag.brewery_drunkenness> | ||
| // @returns ElementTag |
| // @returns ElementTag | ||
| // @plugin Depenizen, BreweryX | ||
| // @description | ||
| // Returns the drunkenness of the brewery player or null if Brewery has no data on the player. |
There was a problem hiding this comment.
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?
|
Ok, going to get started on this :) |
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/