Closed NuclearDonut47 closed 5 months ago
Bug fix for meats. Works and will be uploaded to main soon. Also not a problem - just changed the event a listener was using. Please approve.
That's fine. I doubt it's gonna cause a problem, but I'll just tweak those next time I make a commit. It's not gonna cause a problem for now, but I did it that way (in contrast to BetterConcrete which simply checked after 1 second whether the item entity was in the cauldron) because I wanted to avoid an implementation where you could throw the blocks into a cauldron and NOT end up seeing it transformed just because it entered the cauldron at the hard-coded time. The solution to that was tracking its movement for when the item entity comes to rest, but that could be a long time (if hypothetically dropped from a large height or being carried by water for some reason for example), hence the long timeout.
I can check far less than every tick, though. I also have no real reference for when the amount of bukkit runnables that are going start to cause a problem. Still, though, until the runnable terminates, it evaluates 4 if statements and does 3 operations per iteration. That's less than 1,000 total operations per tick for 100 of these running (an already ridiculous number), which I can't imagine being THAT terrible on performance. It should also be noted that it doesn't count individual blocks, it counts the stack as one entity. Dropping a stack of concrete counts for one of these bukkit runnables to start going, not 64.
On top of the fact that I check for a valid input block before starting the scheduler, I think this is pretty optimized, but like I said, I'll merge this PR and next time I make a commit drop it to one check every 10 ticks. An at most half second delay between the blocks coming to rest in the cauldron before transforming is probably not gonna really bother anyone.
Actually, we don't care about releases for this plugin. I'll fix it now.
There. Thanks for the other addition preventing merging stacks, too. Oversight on my part, great thinking on yours. The item.isValid() check would have stopped the runnable after the stacks merged, but the merged stack would of course not been converted.
This has been tested to work correctly, and has been added to main already. Just looking for an approval.