AgriCraft

AgriCraft

30M Downloads

Cross compatibility with Garden Stuff

InkDragon opened this issue ยท 16 comments

commented

Garden Stuff: https://github.com/jaquadro/GardenCollection

Would it be possible to add compatibility for Garden Soil/Tilled Garden Soil and Farmland in garden stuff pots?

Currently you can whitelist garden stuff's Garden soil, but once it is inside a garden pot it doesn't register and you currently can not place crops(sticks) on soil in Garden Stuff pots.

commented

It is open source, so it should be doable. Low priority though.

commented

thank you <3

commented

Hi, I'm the Garden Stuff author. I've also been getting compatibility requests. I initially thought I could add add something to your registry on my end, but I don't think that's possible.

The garden pots mentioned are TileEntities and the farm state is stored as such, whereas I believe your registry is metadata-based? Would it be sufficient if I exposed some IFarmBlock interface with a method to query for valid farm state? And then that would get worked in .. some how.

I've seen these two mods mixing in more packs, including Jaded's upcoming Magic Farm 3, so I'm more interested in compatibility now.

commented

I have not had the time to look at your code yet, also I am not familiar with your mod, if I have time (probably tomorrow, and maybe this evening) I will see what I can do/what I'd need from you on your end. Thanks for your co-operation.

commented

This will be a lot harder to do then I first thought :s

commented

Depending on how cleanly you want to separate integration concerns, would something akin to this work?

In GrowthRequirements.isSoilValid, check if block is instance of IGardenBlock. If yes, ask it if it's a full block, and ask it for the block material that it's filled with. Then use that returned block to query your registry as normal. That should also play nicely with specific soil restrictions if you're planning to do that.

IGardenBlock is a theoretical API that doesn't exist yet, but I'll probably do some API work today if my back doesn't kill me first.

commented

No, I'm not creating any immediate pull requests. I need to do some API work on my side, which will follow what I suggested above, unless you have different requirements.

commented

I'd like to keep everything that's not vanilla/agricraft out of my packages and put it in a compat package. I guess this should be doable by forwarding the method to a compatability class. I briefly read trough your code this afternoon in uni (where I don't have access to my dev env). Also atm it's quite hectic because of some other stuff so your mod isn't even in my dev env yet. It'll probably be tomorrow when I start to work on it.

commented

I'm working under the assumption that you have compatibility helpers for a dozen mods under /compatibility and pull in APIs for several, and this would be handled the same way.

commented

I do yes, are you creating a pull request or working on your side? If you're doing a pull request I'll hold off working on my side until you're done.

commented

I've started to work on this, but I'll need one thing from your side: allowing any whitelisted soil in my GrowthRequirement class to be put into your pots.
Also am I correct in saying that only the large pots can/should be supported?

commented

I'll need to do a little rejiggering on the substrate stuff to check your whitelist directly, since I don't think I can count on post-init to scan it and populate my own registry. I don't think that's a problem for me to fix, but is there a technical reason outside of completeness that this would be a problem? I would assume you'd just be more restricted when you could place crops.

Garden Soil, Garden Farmland, and Large Pots are the current set of blocks that are valid and make sense. They all derive from the same base and implement the same API - things like soil just have a fixed substrate (themselves).

A heuristic you could use is any block that:

  • Its default slot will accept a plant classified as PlantSize.FULL (things like crops which need a full block and aren't allowed to share any space with other things)
  • The Y offset of the default slot is within 1/16 of 0.

I think that's all possible with the current API without needing to reflect onto concrete class types in the mod, but either works.

commented

I pulled down your compat branch and worked with it for a bit. It looks like I'm using the wrong metadata for the farmland I wanted, which is why it never matched. I've changed that and checked it in.

That allows the crop sticks to place down, but seeds still fail to plant because GrowthRequirement.java bypasses GrowthRequirements.java for its valid soil check.

commented

That is a bug then, because they should. I also noticed the metadata thing, I was going to add a ignore meta functionality to the soils, but seems I can delay that once more

commented

I saw your update, I'll continue work from my side this evening again. (Thank you for the cooperation btw)

commented

Finished what had to be done on my side: 57a2ac1