search

josephcsible / Oreberries

A Minecraft mod that brings back a configurable version of 1.7.10 Tinkers' Construct oreberries
https://minecraft.curseforge.com/projects/oreberries
Other
3 stars 4 forks source link

BlockOreberryBush::harvest FakePlayer issues with Thaumcraft interactions #18

Open noobanidus opened 5 years ago

noobanidus commented 5 years ago

While it's a bit of a fuss to get the bushes working with Thaumcraft (IMC doesn't seem to be working properly, and I just resorted to manually adding "tile.oreberries.oreberry_bush3" to CropUtils.clickableCrops to get golems to properly interact with them instead of them just smashing them dramatically.

So, currently, both the Thaumcraft golem and the ExtraUtilities2 mechanical user function in the same way: the fake player triggers onBlockActivated, and eventually ItemHandlerHelper.giveItemToPlayer is called. It's not an issue with the mechanical user because the inventory of the FakePlayer is copied back into the mechanical user (or is linked, my Java isn't actually that comprehensive).

However, for the Thaumcraft golems, it's basically ignored. I've filed an issue on the Thaumcraft tracker (https://github.com/Azanor/thaumcraft-beta/issues/1524) covering it, but after I noticed the comment on BlockOreberryBush.java:160 re: FakePlayers and dropping in 1.7.10, I thought I'd let you know one of the potential interaction issues.

However, I think this is actually an issue for Thaumcraft -- it should handle ItemHandlerHelper.giveItemToPlayer a little more gracefully.

The only other potentially related issue is that Thaumcraft checks canGrow to determine if the plant is ready to be harvested. If bonemealGrowthChance is set to the default of 0.0, this check will always fail and the plant will always be presumed to be "fully grown". I'm not sure whether this is an issue or not, but it was rather amusing to see golems flying around smashing all the bushes!

josephcsible commented 5 years ago

IMC doesn't seem to be working

What IMC are you referring to? My mod doesn't contain any IMC.

However, I think this is actually an issue for Thaumcraft -- it should handle ItemHandlerHelper.giveItemToPlayer a little more gracefully.

I agree.

The only other potentially related issue is that Thaumcraft checks canGrow to determine if the plant is ready to be harvested. If bonemealGrowthChance is set to the default of 0.0, this check will always fail and the plant will always be presumed to be "fully grown". I'm not sure whether this is an issue or not, but it was rather amusing to see golems flying around smashing all the bushes!

I could change this, but the problem is that if I make canGrow return true when bonemeal is 0.0, then players will be able to waste bonemeal on these crops, and may not realize that it will never actually grow them. (Although I may be able to work around this with a BonemealEvent, or making it only implement IGrowable sometimes.)

noobanidus commented 5 years ago

Sorry, the IMC I was referring to was the Thaumcraft IMC for registering new "clickable" crops. Upon further investigation this may simply be a failure of my own with regards to when I was doing the IMC call -- but the IMC handler in the Thaumcraft proxy never actually triggered.

I could change this, but the problem is that if I make canGrow return true when bonemeal is 0.0, then players will be able to waste bonemeal on these crops, and may not realize that it will never actually grow them. (Although I may be able to work around this with a BonemealEvent, or making it only implement IGrowable sometimes.)

I understand. I went digging for some clues in the original 1.7.10 source for Tinker's Construct simply to see how it handled the IMC calls (which don't appear to have changed significantly across versions) and noticed it implemented IPlantable rather than IGrowable. However, I have no idea what interfaces were like in 1.7.10 or if this is even comparable.

As it stands, there are a number of other "plants" (Botania glimmering flowers for example) that golems will seek out and break for the Harvest sigil, so it isn't an issue exclusive to your code.

I just wanted you to be aware of this in case someone ever came along with an obscure issue, allowing you to refer it back to Thaumcraft.

Although improving the issue with bone meal may also resolve the problems with Steve's Cart as outlined in #14

josephcsible commented 5 years ago

I understand. I went digging for some clues in the original 1.7.10 source for Tinker's Construct simply to see how it handled the IMC calls (which don't appear to have changed significantly across versions) and noticed it implemented IPlantable rather than IGrowable.

In 1.7.10, bonemeal wasn't supported on oreberries at all. IGrowable is purely to make bonemeal work. Anyway, I did implement my one idea of making IGrowable go away when bonemeal is 0.0. Try building what I just pushed to GitHub (or I can do a release now if you can't get it to compile for some reason) and see if it improves matters.