So on version a17 on 1.10.2 I notice that you can bonemeal crops that shouldn't be able to be bonemealed. The same thing happens with empty crop sticks.

So did one of those two commits fix the issue with crops where bonemeal has been disabled?

@Dilnu As far as I can tell, yes it did. I tested fertilizer-mutations both on and off at each run. And for both I would test an empty regular crop, a cross crop, a mature plant, and a baby plant. The cross crop would only use up bonemeal if fertilizer-mutations was on. And after a few tries you'd get a crossover.

Edit: I had assumed that everyone could read my mind and knew I had done that. Oops, hehe.

Empty crop sticks can be bonemealed for weeds, if for some reason you would want that.

Haha, I was looking into that. Currently the code treats cross crops and regular crops differently. Which kind of makes sense at first, since the setting is about fertilizers being able to cause mutations. But now the mutation engine has the SpreadStrategy too, and to a simple observer that would look a lot like regular spreading. So... I was thinking maybe the fertilizer mutations could be renamed more generically, and could occur on regular crops as well. Especially since vanilla minecraft teaches that bonemeal = growth tick, and regular crops + growth tick = fun. I can ask @RlonRyan if he'd prefer that gameplay change.

Thanks.

For the sake of documentation, I can also now better summarize the chain of events that lead to why this bug is happening:

• The method BlockCrop::onBlockActivated initially tries to handle the game event of a crop being right clicked by a player.
• If the item being clicked with is something that the FertilizerRegistry recognizes, like vanilla bonemeal, then the code jumps to that adapter's implementation of the IAgriFertilizer::applyFertilizer method. With bonemeal, it calls IAgriFertilizable::acceptsFertilizer followed maybe by IAgriFertilizable::onApplyFertilizer. And afterwards, applyFertilizer returns a boolean indicating if the application was successful. This is just the normal process for AgriCraft's fertilizer system.
• The first catch is that onBlockActivated also returns a boolean, but this one signals if the click has been handled, or if the caller (PlayerControllerMP::processRightClickBlock in this case) should try other alternative handlers.
• And currently, oBA returns false if the item is a fertilizer and it can't be applied, for any of several reasons: the plant is mature, the plant isn't allowed to be bonemealed, there is no seed planted, it's a cross crop but fertilizer-mutations are disabled or the fertilizer isn't one that can trigger mutations.
• That means that in all of those cases, the right click event will continue to be evaluated, and typically that means that the onItemUse method will get invoked and have a chance to do things.
• In the case of bonemeal (i.e. ItemDye that's white), that method calls applyBonemeal.
• And inside that method, it checks if the clicked-on block is an instance of IGrowable, checks if it canGrow, checks if it canUseBonemeal, and then finally calls grow on the Block if all of that has been true.
• The second catch is that those two 'can' methods are somewhat misleadingly named, at least in terms of how ItemDye responds to them. There's a good discussion and explanation over at http://www.minecraftforge.net/forum/topic/22365-understanding-igrowable/. Basically, the canUseBonemeal method isn't about ability or permission, it's the plant requesting whether or not the growth attempt should continue. The caller then should call the grow method, or not, based on that result.
• ItemDye specifically concludes that if a target canGrow, then bonemeal is allowed to be used on it. That means that anything that canGrow will always make the the stack of bonemeal shrink itself by one.
• There's things like saplings and mushrooms, where canGrow always returns true but the return value of canUseBonemeal is randomized.
• The bug is that, in order to block vanilla bonemeal still triggering growth on AgriCraft crops after BlockCrop::onBlockActivated and the BonemealWrapper together decided that the bonemeal shouldn't be used on the target crop, there was a commit [2] that made canUseBonemeal return false instead of true. While that sounds correct, and does cause bonemeal to not call grow on the crops, it doesn't prevent the bonemeal from being consumed, or the particles from being spawned/animated.
• It also didn't fully prevent mods from (intentionally or unintentionally) bypassing AgriCraft's fertilizer system, and thereby fully control them. Oops. :(
• So those two quirks/typos/bugs combined together allow vanilla bonemeal to get uselessly used on and consumed by AgriCraft's crops!

References:

• [1]: move growth logic to the TE. Also shows how previously the canGrow method needed to allow
• [2]: attempt to block bonemeal use on AgriCraft crops.
• [3]: changed weeds from being a different class, to being a plant like other normal plants, but with special configurations.

Misc:

• canGrow == will bonemeal be consumed by this right-click.
• canUseBonemeal == will grow be called, after consuming the bonemeal, but only if canGrow.
• Honestly? I think cUB shouldn't exist, and when grow is called, the target should also decide if maybe there's zero growth, and not just a range of random but non-zero growth. Alternatively, have an applyBonemeal method which directly calls a standardized grow method a random number of times. But that's a vanilla interface, so it's unlikely to ever change. :) Hence AgriCraft's alternative interface.
• Anywho, I've also mentioned this in the PR, but my hope is to make the IGrowable interface intentionally toggle-able in the configuration. And as part of that, it would let the user fully block the vanilla mechanic (and mods that use it) on AgriCraft crops, or enable them. As part of the fix in the PR, the bonemeal item itself would still be controlled by the wrapper class, and so would be separately configurable.
• Finally, I think I can turn this summary into a page in the wiki, to improve the web's documentation on the vanilla mechanics. :)

Oops, wow. I hadn't checked this thread recently, forgot that my reference to the issue number in the commit message would lead to a link, and that rebases and such would multiply everything out.

I came here to say is that the a21 branch that will be released next has the fix for this. :)