Copper Golem + Copper Block Support #8244
Conversation
…to dev/feature
…to dev/feature
…to dev/feature
…to dev/feature
Absolutionism
requested review from
Burbulinis and
TheMug06
and removed request for
a team
skriptlang-automation
bot
added
the
needs reviews
There was a problem hiding this comment.
this all already exists
Skript.classExists(), Skript.methodExists()...
There was a problem hiding this comment.
I know the 'exists' methods are in the Skript class, but these will cache any lookups for easier use for the other methods in this class. I'm not entirely sure how costly 'Class#forName' is but figured caching would be beneficial.
Are the 'exists' methods the only things in question? If not, where are the already existing versions of all the other methods I've added.
There was a problem hiding this comment.
Caching isn't really important if you only do the lookups at startup, which is what all of skript does, so I don't think this is really necessary.
It has some small benefit, though, so I'm ok with it, but it needs to not duplicate the existing methods. Skript.xxx should, at minimum, redirect to this, and I think that classes probably shouldn't talk to ReflectUtils directly, but rather through Skript. It would also be slightly better if this were an instance, rather than a bunch of static fields. Skript isn't reload safe currently, but making it an instance would mean less work in the future if we did want to work towards that goal.
src/main/java/ch/njol/skript/expressions/ExprCopperGolemOxidationTime.java
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/ExprCopperGolemOxidationTime.java
Show resolved
Hide resolved
| getOxidizingMethod = ReflectUtils.getMethod("org.bukkit.entity.CopperGolem", "getOxidizing"); | ||
| atTimeClass = ReflectUtils.getClass("org.bukkit.entity.CopperGolem$Oxidizing$AtTime"); | ||
| getTimeMethod = ReflectUtils.getMethod(atTimeClass, "time"); | ||
| newTimeMethod = ReflectUtils.getMethod("org.bukkit.entity.CopperGolem$Oxidizing", "atTime", long.class); | ||
| unsetMethod = ReflectUtils.getMethod("org.bukkit.entity.CopperGolem$Oxidizing", "unset"); |
There was a problem hiding this comment.
you'd be better served just not loading this class if the version's too low.
you could make a copper golem module, for example, or just put it somewhere it won't automatically be loaded
|
Forgot to write this in the last review, but this should all be in the skriptlang package. |
2 participants
Problem
No proper support for setting and getting attributes of copper golems and copper blocks.
Solution
Removes 'copper golem' from SimpleEntityData and adds CopperGolemData allowing users to specify the copper state of the copper golem and whether they're waxed.
Adds a condition to check if a copper golem or a copper block is waxed.
Adds an effect to wax or unwax a copper golem or a copper block.
Adds an expression to get/set the time until the next oxidation for a copper golem.
Adds an expression to get/set the pose of a copper golem statue.
Adds an expression to get/set the copper state of a copper golem or a copper block.
Adds 'CopperState' that mimics 'WeatheringCopperState' for the versions that don't have it. Allowing easy use for getting and setting the copper states.
Registers ClassInfo for copper golem statue poses
Registers ClassInfo for 'CopperState' or 'WeatheringCopperState' if it exists.
Adds 'ReflectUtils' that is a utility class for common reflection and caches where possible. Reflection was needed for 'CopperState', 'ExprCopperState', and 'ExprCopperGolemOxidationTime' which led to the creation of ReflectUtils.
Testing Completed
EntityData.sk
EffWax.sk
ExprCopperGolemOxidationTime.sk
ExprCopperGolemPose.sk
ExprCopperState.sk
EffSecSpawn.sk
Supporting Information
N/A
Completes: none
Related: none