fix: Prevent vine growth on the same block if vine-grow is false - #4930
fix: Prevent vine growth on the same block if vine-grow is false#4930CAG2Mark wants to merge 2 commits into
vine-grow is false#4930Conversation
| @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) | ||
| public void onBlockGrow(BlockGrowEvent event) { | ||
| if (event instanceof BlockFormEvent) { | ||
| return; // handled below |
There was a problem hiding this comment.
I would be fine with handling all growth types of vines under this event handler and removing the vine checks from BlockSpreadEvent. Feels cleaner than keeping vine spread logic in multiple places imo
There was a problem hiding this comment.
It seems this doesn't work. The BlockSpreadEvent doesn't appear to trigger handlers for BlockGrowEvent (despite the former being the latter's superclass). Vines would spread to other blocks in my testing when only handling BlockGrowEvent.
| if (plot == null) { | ||
| return; | ||
| } | ||
| switch (event.getBlock().getType().toString()) { |
There was a problem hiding this comment.
If we only handle vines in this handler we could potentially think about moving the material check to the top (we don't really do that currently, but that checks is most certainly cheaper than getting the plot area and plot - especially for such a potentially high frequency event)
There was a problem hiding this comment.
In principle, this check could be moved to the top for the other handlers too, but it might be a bit less clean
There was a problem hiding this comment.
No longer relevant as I've moved the code to onGrow, which was already used for crop handling.
| return; | ||
| } | ||
| switch (event.getBlock().getType().toString()) { | ||
| case "VINE": |
There was a problem hiding this comment.
The current handler for the VineGrowthFlag handles more than just vines, so if the other handler gets removed (1st comment) this should contain all types (currently this makes sense as no other type should be able to grow in a single block space)
There was a problem hiding this comment.
No longer relevant as per my comment here: #4930 (comment)
| return; | ||
| } | ||
| switch (event.getBlock().getType().toString()) { | ||
| case "VINE": |
There was a problem hiding this comment.
And is there any reason for using the enum name instead of the field? (would only make sense if there were any material renames in our specific supported version range)
There was a problem hiding this comment.
I don't know. I just followed what was done in the other handlers for consistency.
There was a problem hiding this comment.
Enum names were used due to compatibility issues with older versions of Minecraft. Given vines have been around for over 10 years, we don't need to use the string version of the enum.
| Block block = event.getBlock(); | ||
| Location location = BukkitUtil.adapt(block.getLocation()); | ||
| if (location.isPlotRoad()) { | ||
| event.setCancelled(true); |
There was a problem hiding this comment.
This should check road-flags, no? Unsure in this case tbh, as the default flag value is true I think, which would be unexpected for roads...
There was a problem hiding this comment.
All the other related handlers seem to do the same check as this, so I don't know.
|
Ah, I didn't see there is already a handler for EDIT: It seems |
|
Please see my comments as they contain some information. Still not sure what to do about road flags (and it seems many of the other handlers don't deal with this anyway, or maybe it is implicitly handled somehow, not sure) |
3 participants
Overview
Prevents vines from growing on the same block if the
vine-growflag is set tofalse.Description
A single vine placed in a 1x1x1 hole can spread to all available faces in that hole, and this triggers
BlockGrowEventinstead ofBlockSpreadEvent. This PR checks for vine growth that is fired underBlockGrowEvent. Note that vines spreading to another block is still handled underBlockSpreadEvent, a subclass ofBlockGrowEvent, and this is handled separately fromBlockFormEvent(see line 415 inBlockEventListener.java).Submitter Checklist
@since TODO.