AgriCraft / AgriCraft

The source code for the Minecraft mod: AgriCraft
MIT License
213 stars 128 forks source link

Agricraft a21+InfinityLib 0.11.0 removes automation with PA Farmer #1084

Closed janagyjr closed 6 years ago

janagyjr commented 6 years ago

I have Agricraft 2.0.0-0.11.0-a21 with InfinityLib 0.11.0. Crops are no longer harvestable with Progressive Automation (but are still manually harvestable). PA farmer had no issue with prior version of Agricraft and infinitylib 0.10.0

CodesCubesAndCrashes commented 6 years ago

@janagyjr Interesting! Looking at the harvesting code in the compatibility file, I don't see an obvious gotcha. I'm going to take a closer look though. Small warning: compatibility is a lower priority until AgriCraft itself is fixed and stable. But if a recent change caused this, I'd like to handle that.

Follow up questions:

janagyjr commented 6 years ago

Agricraft a19, infinity lib 0.10, progressive automation 1.10.2-1.7.4, MC 1.10.2, Forge 2488, only thing that was updated was Agricraft. I should note the setup is for Mystical Agriculture seeds (Mystical Agriculture 1.10.2-1.5.9).

And yes, the PA farmer (with a Tinker's unbreakable mattock) was harvesting and planting. My tier three Tiny Progressions growth crystals still enhance the growth of the crops, and I can manually harvest with no issue (everything is using cropsticks for 10^3 crops)

janagyjr commented 6 years ago

Reverting back to the mentioned prior versions sees everything working again.

image

CodesCubesAndCrashes commented 6 years ago

@janagyjr Alrighty. Can you confirm that non-MA plants are effected as well? I expect so, but pinning down any quirks to the bug is good. Similarly, since this sounds like an existing save, testing in a new world would make sure that there's no weirdness from upgrading. Thank you! Appreciate the follow-ups. :)

janagyjr commented 6 years ago

I will check. I have some Agricraft potatoes I want to automate (for steak and chips) anyway. I will also double-check in a fresh world using the upgraded versions and non-upgraded versions.

janagyjr commented 6 years ago

Existing World

Non-upgraded Agricraft for potatoes: PA farmer is planting/harvesting (almost forgot the range upgrades, good thing I checked)

Upgraded version of Agricraft for potatoes: Not harvesting (I don't think the pa farmer replants crop-sticked items, so there's that, it just 'right-click' harvests them, which is neat).

image

Fresh World

Non-upgraded Agricraft for potatoes: Planted, harvested (similar set up; tier 3 growth crystal, diamond harvester, 8 diamond upgrades for range)

Upgraded Agricraft for potatoes: Planted, but won't harvest (similar set up, tier 3 growth crystal, diamond harvester, 8 diamond upgrades for range)

Only difference is in fresh world I did everything in creative (1^3 analyzed potatoes, creative rf engine for harvester, creatived in tier 3 growth crystal, dirt, crop sticks, etc.)

CodesCubesAndCrashes commented 6 years ago

Hmmm. I've traced the logic in TilePlanter, and everything down from ModHelper#HarvestPlant looks fine. I've seen some quirks in the logic, but none that should be preventing the harvesting. I've also been comparing the changes from a19 to a21 for AgriCraft, but that hasn't turned up the cause yet.

Thank you for the testing. Questions: 1) Do your Mystical Agriculture plants accept bonemeal? By default they don't, but you mentioned that the crystals sped up their growth. I might need to check how those work. [Edit: they use block updates, not a bonemeal effect.] 2) Did you make two fresh worlds, or did you make it for one version and then reuse it for the other? I want to make sure that a21 is showing the bug when tested from scratch. 3) The Planter does the harvesting, right? There is no Harvester? And the Farmer is for animals, not plants, yes? 4) You didn't mention the hoe in your comparisons, but there was one in the machine, yes? Since the potatoes were harvested in a19, I'm guessing that the hoe was in place. 5) Edit2: Can you confirm that you left IGrowable set to enabled in the AgriCraft config settings?

Worth saying: the isGrown logic that PA uses for Vanilla, Right-Clicking, and AgriCraft depends on the IGrowable#canGrow method. But technically `!canGrow' is not a synonym for "this is mature". That method name is really misleading, it has a more specific meaning as used in vanilla. :-/

Edit2.5: To recap and summarize: If IGrowable is enabled, then the potatoes are not getting harvested because of the isGrown logic. But the MA plants wouldn't suffer from the same bug because if they don't accept bonemeal then they would instead always return true for isGrown.

janagyjr commented 6 years ago

1) Yes they do accept bone meal. The tier 3 growth crystals (Tiny Progressions) AND crop dusting mod speed up their growth (the growth crystal more so, it also enables night-time growth) just as much. To show you this I'd have to record a video and OBS + single-player MC = bad fps 2) I did one new world, but tested a21 first, actually (since I had already updated it for the saved world test on the potatoes) and then downgraded to a19 3) (Yes, meant planter, d'oh, too many damn blocks not named more obviously :-p) P.A. Planter does planting and harvesting, but once in crop sticks it doesn't replant, just harvests 4) Yes, there was an unbreakable tinker's mattock in all tests (I didn't use a vanilla hoe since my setup doesn't use a vanilla hoe because Tinker's)

image

Worth saying: I am not a programmer. I can write some basic Perl and bash scripts, and that's about it. All of what you said is geek to me. ;)

CodesCubesAndCrashes commented 6 years ago

Scripting with perl and bash isn't nothing. :) I miss being good with perl, I've grown too rusty. Also, part of my writing is thinking aloud and documenting my observations, so no worries.

Since the MA plant JSONs are modified to enable bonemeal, that simplifies things and explains why they aren't working with the Planter. I stand by the behavior of the crops, they're implementing IGrowable correctly because a plant on an AgriCraft crop can spread to neighboring crop sticks even if it's mature. Getting PA properly working with AgriCraft is going to need a change to their compatibility code. I think AgriCraft maybe could also provide a better interface though.

If this is your primary plant-farming setup, then for now you can disable the IGrowable compatibility in AgriCraft's config. That'll trick PA's isGrown into returning true, and let it harvest those plants again. It will break support with some other mods though. Be warned. Oh, and if this is a multiplayer server, you'll need to change the config on the server manually, not using the in game menu. Let me know if that helps!

janagyjr commented 6 years ago

It probably will, but I'll just stick to using a19 for now.

I kind of noticed that behavior, it's why I keep my cropsticked plants at least one row apart with a non-AC crop in between or just a normal dirt block (see reference image two posts ago). I will keep this in mind, though, when I get to working on my 1.10.2 modpack some more. Thanks for the help though, glad to know it's not anything really broken. Perhaps I should open up a bug in PA's issue tracker and reference this. :thinking:

CodesCubesAndCrashes commented 6 years ago

Thank you again for working through this with me. :) Get quick and detailed answers made it a lot easier to figure things out! And I'm sorry that this change is a deal breaker. If later you try disabling IGrowable (which wasn't really working <a21) and you find something else that it breaks, I'd love to hear about that too.

I'll follow up with PA, since I see another couple reports on there that I could also look into. Plus I want to offer a productive solution in addition to the heads up.

I'm debating leaving this issue open on my side, since it feels like more people read the open issues list than use the search feature. -.- Hm hm.

Have a good one!

janagyjr commented 6 years ago

Up to you. I have no issues with how you want to do that.

The change isn't a deal breaker. I just am too lazy to mess with the configs. I look forward to the next few updates. (:

Joly0 commented 6 years ago

I want to add, that the MinefactoryReloaded Harvester doesnt work either, i dont know if this intended to be so, i only know, that both mods worked perfectly together in 1.7.10 and i didnt tested other version of these mods then the earliest 1.10.2 versions. I also dont say, that this problem belongs to Agricraft only, maybe the Mfr code has changed and now it doesnt work anymore, but maybe there is a chance to workaround this problem.

RlonRyan commented 6 years ago

I think this has been fixed with the latest release.