Buildinggadgets: Support for Block-Providing Items
Hello there!
I'm the Author of Builder's Bag, a mod (for 1.12.2) which adds a bag that can craft, chisel and do much more with blocks that you provide in its inventory. I've been getting requests for integration with BuildingGadgets and so I looked through your code, but I don't think as it currently stands I have the possibility to add compat on my end.
Ideally, I'd have something like this (from BetterBuildersWands):
https://github.com/portablejim/BetterBuildersWands/blob/1.12/src/main/java/portablejim/bbw/api/IContainerHandler.java
Perhaps it could also have a simulate boolean, to first check if it has the requiredBlocks before actually using them.
I think an interface like this would generally be useful to have, so that perhaps other backpack mods can integrate their pack, or perhaps the botania wands can work with the gadget.
Please let me know if I need to provide more information.
This is my source: https://github.com/Tschipp/BuildersBag
Tschipp
All 35 comments
BG uses Forge's IItemHandler for interop. Just give your Item and IItemHandler cap and you should be fine.
MajorTuvok
on 11 May 2020
Hm, sadly this would only allow it to take blocks that it "currently has". What is special about the bag, is that it can create its materials from various sources. Maybe a gif demonstrates it best:
Tschipp
on 11 May 2020
I see no differnce there (though the uncommented interface you linked also has the Player as an argument). If you provide Items (whether they are created or not really doesn't matter), you can expose the IItemHandler's extractItem Method.
I'm not a fan of 1.12's InventoryManipulation stuff - it might depend on getStackInSlot, which would of course imply you'd have to list everything that is craftable in one slot each (which will be very imperformant, as Dire is doing a new linear search everytime an Item is asked for!!!).
I have some uncommitted code for the 1.15 rewrite which resolves this Problem und would allow you to hook in with a much nicer interface - but 1.12 is only getting bug fixes and not features.
MajorTuvok
on 11 May 2020
If you want to, I can try again to finally find the time to setup the repo Mike moved the rewrite to tomorrow (I've got a lot to do, so I don't know If I'd manage), so that you can take a look at this. Bear in mind that it is only for 1.15 though...
MajorTuvok
on 11 May 2020
Well, I have the tools and already do generate a list of everything that is craftable (the performance isn't that bad, because I use a pre-initialized recipe tree to generate the craftable blocks. I also fixed the continuous querying by doing a lot of caching, as well as some probabilistics). Currently, the bag does not directly have an internal IItemHandler, but only one that's integrated in my own cap. I suppose I could add an IItemHandler, but I am afraid that this will have unforseen consequences, as LittleTiles (which I also have compat for right now) also does some stuff with IItemHandlers.
I suppose I can do a test of it, but I still think an interface would be the easiest also for future compatibilities.
I completely understand if you don't have the time to do this, I know the situation... If no new features are being added for 1.12 and this approach doesn't work, I suppose there'll only be compat for 1.15+
Tschipp
on 11 May 2020
The performance would be bad on BG's end not on yours...
I still think an interface would be the easiest also for future compatibilities.
What I have results in you exposing an interface (cap) which is asked for an IndexKey (with an overload that you can delegate to, which accepts an ItemStack) and a count. Additionally the player who is performing the action is also provided as I need him internally for caching purposes. It would also allow for a more efficient approach of collecting the crafting operations in one combined Transaction which is then committed in the end... (the latter solves the performance concerns I have, of exposing every craftable item as one slot. Even though you could just ignore it, if you want, and do the actions everytime the transaction get's another Item added. As you already cache it, I see no performance benefits in doing that lazily there.)
MajorTuvok
on 11 May 2020
Sorry, I'm not 100% sure I follow. Are you saying I should expose an interface that you can use?
Also, when does BG request blocks? does it do that every frame (for rendering) and once on click, or even more than that?
Tschipp
on 11 May 2020
Our system for 1.12 will scan your inventory, linked inventories and also attempt to scan nested inventories, I can't remember the order exactly, it's in a change log somewhere :P I think it starts with nested inventories. Nested inventories are items that explose an IItemHandler as a Cap.
You could expose an IItemHandler on your end that would fake / use your internal systems to either say "yes I can supply this" or "no" by modifying the Insert and Extract. You'd likely just have to return false on inserting as our system would try and place the block back in your inventory unless you supported destructuring the block down to the core blocks you hold in the bag.
Effectively it can be done but it'll be a pain in the ass for you so sorry about that. For 1.12 it's an uncached search and it's done on every frame unfortunatly... The 1.12 system isn't great. I'm open to recommendation.
MichaelHillcox
on 11 May 2020
Are you saying I should expose an interface that you can use?
That's for 1.15
more than that
Possibly, it depends where in the search you appear. But the worst case is once per block (for that block's required Items) for both a build action and the render.
^ this linear search is what I hope to improve in 1.15. If you already have an index for the crafting results, then you could actually benefit from that, inestead of letting inventory manipulation iterate it.
MajorTuvok
on 11 May 2020
@MichaelHillcox if InventoryManipulation does check via getStackInSlot, which I know it did before your changes (not sure how it is now), it will be even worse. That's due to it expecting to see every available stack appear in some slot...
Do you know whether this is still checked that way?
MajorTuvok
on 11 May 2020
It does but you could fix this by only exposing a single slot and handling all the logic on the actual insert and extract methods.
MichaelHillcox
on 11 May 2020
Effectively it can be done but it'll be a pain in the ass
Haha, you should see what I had to do to get Chisels and Bits working (it was not pretty)
But yeah, I'm gonna give it some thought and also work out a better system for caching. As it is now I just implemented the features quite quickly and the newest code isn't particularly clean.
I also plan on adding a functionality that will be able to expose an entire AE2 or RS storage system to the bag, which will add so many more stacks to the pool... it will be overwhelming, and querying this every frame is impossible.
It does but you could fix this by only exposing a single slot and handling all the logic on the actual insert and extract methods.
Yes but only if I know what block should be provided, otherwise I don't know what slot to show.
Tschipp
on 11 May 2020
It does but you could fix this by only exposing a single slot and handling all the logic on the actual insert and extract methods.
@MichaelHillcox as soon as it does compare the stack in getStackInSlot you can no longer do this. If it were to only use insert/extract: yes. But as you said it compares for the stack in the slot, and he'd only be able to expose a single stack that way, aka only one of the many craftable items...
MajorTuvok
on 11 May 2020
Yes but only if I know what block should be provided, otherwise I don't know what slot to show
@MichaelHillcox ... Just as he said...
MajorTuvok
on 11 May 2020
This is true, the only other option would be to have a dynamic slot size based on the amount of times you can currently build so when getStackInSlot and getSlots are called you'd allow the mod to currectly make the right comparisons. Simple is always the best way so if you had a list of items you can provide then each item in that list would be repersented by a slot so when getStackInSlot is called you'd just return then lists item based on the index passed. That's how I'd do it in my head anyway :D
MichaelHillcox
on 11 May 2020
Jup, so we all agree again XD. Discussions are awesome :)
MajorTuvok
on 11 May 2020
Nice, lol. I'll give it a spin and see how bad the performance is and if I can't do some hacks with caching on my end.
Tschipp
on 11 May 2020
Yeah, highly suggest some level of caching on your end. We should cache on our end too but I don't feel like changing the 1.12 code.
This solutaiton should also work on 1.14 too :D
MichaelHillcox
on 11 May 2020
Yeah I completely understand that you don't want to greatly modify the 1.12 code. I'm just stuck in the past I guess but yeah I'll update eventually
Tschipp
on 11 May 2020
I've actually made some great progress on this! :D
(sorry for the terrible quality, had to compress the hell out of it)
Tschipp
on 19 Jun 2020
That's really cool to see, it's a pain in the ass, If you wanted you could make a PR to our mod to add a better interface for other mods to use but all the code is a bit of a mess... Does look like you're getting somewhere though :+1: Love the mod idea, hopefully in 1.15/1.16 (rewrite) I'm going to make the inventory system completely API ready so I can just give modders the ability to register their own IInventory like solutions. But that's not any time soon sadly. I am 100% open to Pr's though :D
MichaelHillcox
on 19 Jun 2020
Yes I will think about a way to maybe make a PR, thanks!
Tschipp
on 19 Jun 2020
would dearly love to see this work with the Botania Rod of the Lands and Rod of the Depths if they are in your inventory (or hotbar depending on how you want to handle it)
WebDragon
on 23 Jun 2020
I'm working on a tiny PR that adds this kind of interface that would allow this. Botania would have to implement that of course.
Tschipp
on 23 Jun 2020
Or alternatively, you can try this: The Builder's Bag supports The rod of the Lands (and all others), so you can place it in the bag and then building gadgets can pull from the bag and indirectly pull from the rod. It's all quite magical when it happens :D
Tschipp
on 23 Jun 2020
I presume you're doing that for 1.12, right?
I'm sorry I can't continue with the 1.15 stuff atm - lecture time is so much already.
Might not even have the time to review it :(
MajorTuvok
on 24 Jun 2020
I'm happy for him to make a PR for 1.12, I'm just not wanting to actively work on that code as it's extreamly hard to work with and not end up breaking things. We have a few issues created from last time I touched 1.12 haha. As for 1.15, I'll be dropping support for the old version of the mod 1.16 backwards as soon as v4 is available and I'll be backporting v4 as far back as 1.14 at the very least. 1.15 and 1.14 support were both thrown together and are both complete messes. 1.16 might get a "quick have it" version but that'll only be a minor version upgrade and only if 1.16 comes out very soon as I want to put more time into v4 in the coming days.
Sorry ramble over. Considering your above, my idea for support would be third party but if that fails then first party. Meaning that I'll manually add support for a mod if it's on the right version or I think it greatly improves the way the two mods can work with each other like yours. But that'll only happen if the other modders won't add it themselves once we have a simple way for them to add support.
MichaelHillcox
on 24 Jun 2020
I really wish they would slow down the release times for new Minecraft versions with all these major breaking differences between them so that mod developers could keep up. It's quite stressful for you folks, not to mention distressing for us to not have our favorite mods on tap for the MC version we want.
WebDragon
on 24 Jun 2020
Yeah, my suggestion would be to drop 1.14, almost nobody even plays that version anymore
Tschipp
on 24 Jun 2020
Pretty sure that 1.14 doesn't get updates anymore...
@MichaelHillcox ?
MajorTuvok
on 24 Jun 2020
1.14 is LTS for forge so we'll keep updating it every now and then if it's needed. But yeah, I've mostly ignored 1.14 as it was a short lived version. Regarding the backport, if I was to back port it, it would be a pretty simple port so It would just be a "here have it" kind of thing as BG for 1.14 is a bloody mess. 1.15 isn't much better either
MichaelHillcox
on 24 Jun 2020
Made a PR :)
Tschipp
on 15 Jul 2020
To keep things tidy, we now have master list of feature requests, we thank you for your suggestion and we will use this issue for future reference. Closing in preference for https://github.com/Direwolf20-MC/BuildingGadgets/issues/534
MichaelHillcox
on 24 Oct 2020
That's a good idea, should do that on some of my repos too 馃 Did you use any tool or do it manually?
Tschipp
on 24 Oct 2020
Manually :cry: but I'm working on a tool to do it for me automatically when the enchancement tag is put on a issue. The github API is pretty simple to do but I want to make the tool more robust before I let it lose on this repo :P
MichaelHillcox
on 24 Oct 2020
Related issues
delta2k
路
3Comments
perlguy
路
3Comments
MajorTuvok
路
3Comments
WenXin20
路
3Comments
P3rf3ctXZer0
路
5Comments
Most helpful comment
This is true, the only other option would be to have a dynamic slot size based on the amount of times you can currently build so when getStackInSlot and getSlots are called you'd allow the mod to currectly make the right comparisons. Simple is always the best way so if you had a list of items you can provide then each item in that list would be repersented by a slot so when getStackInSlot is called you'd just return then lists item based on the index passed. That's how I'd do it in my head anyway :D
https://github.com/Direwolf20-MC/BuildingGadgets/blob/d3259bf4e8cbdc449f490141a8eb39e936655f7f/src/main/java/com/direwolf20/buildinggadgets/common/tools/InventoryManipulation.java#L133-L151