Inventory Control Tweaks compatibility

[Felix14-v2]

Description It seems that Pick Block Pro overrides the ICT block picking behavior, described here, so with the both mods installed, you can't benefit from this feature:

https://github.com/Sjouwer/pick-block-pro/assets/75726196/148b229b-17f9-404d-bedb-8e6fc71a4df0

Is it possible to add support for this mod?

Related supersaiyansubtlety/inventory_control_tweaks#24

[Sjouwer]

Pick Block Pro pretty much changes every aspect of the vanilla Pick Block, that's why I had to fully replace it. Problem is that ICT hooks into the vanilla Pick Block. Not sure what's the best solution for this

[supersaiyansubtlety]

(ICT dev) afaict there are two solutions, neither ideal: 1) PBP adds an API to control some aspects of picking blocks: idk if you want to maintain that, and it might create the expectation that API support will be added to accommodate other mods 2) PBP re-implements ICTs block picking features: duplication is obviously far less than ideal, and a similar expectation of accommodation could still arise

[Sjouwer]

Thank you for the input. I can see myself adding some basic API support, or at least try to add support for the Fabric API events mentioned in #19. However I feel like more complicated features are better to be integrated fully into PBP for the best result. See for example the integration of the Pick-BlockState feature into PBP, it made it a lot more powerfull. Trying to accommodate for all kinds of special cases with an API would probably also become messy and a pain to maintain. Are you however ok with me copying the Pick Block feautures of ICT?

[supersaiyansubtlety]

I'd be fine with you copying ICT pick block features if no better solution can be found, but hopefully using FAPI's event works out as mentioned in #19.

[Sjouwer]

As explained in #19 I've added the 2 default Fabric API events at pretty much the same locations as Fabric API does. However in the case of PBP there are still 2 issues I see related to ICT:

• In survival the inventory manger checks if the item is inside the inventory, if not, then it'll also search through shulkers and bundles. This means it's possible that the item at the very end might be changed to a shulker or bundle
• PBP has the option to lock slots, the inventory manager checks for this and this can cause the slot to be very different from the slot the vanilla PB would use. This also seems to be rather conflicting with the option of ICT to always use the same slot

I could maybe add a PBP specific event that'll supply the slot it has decided to use and the final item at the very end. I believe that would be enough to solve the 'pick block fills stack' feature, though probably not the 'pick block never changes slot' one

[supersaiyansubtlety]

I was actually suggesting that you move PBP's functionality into a ClientPickBlockApplyCallback.

With your current approach of calling the FAPI event callbacks: For your first bullet (1), I think you're right, this can be an issue: if PBP finds a shulker/bundle on the hotbar, iiuc it would select that slot, and this would be after ICT's event callback so ICT couldn't enforce "pick block never changes slot".

For your second bullet (2), yes this is an issue; but I think if you expose which slots are locked it could be solved (except in the same case as (1)): I could check if the currently selected slot or getSlotWithStack is locked and do nothing if so.

If PBP's functionality could be put into a ClientPickBlockApplyCallback (1) could be solved, as I could mixin after the FAPI's event mixin to enforce "pick block never changes slot". PBP would still have to expose locked slots for ICT to respect them.

[Sjouwer]

Putting PBP into ClientPickBlockApplyCallback is not possible. If it was then I wouldn't need to override it like I do now.

Here's the flow:

1: PBP does a raycast to determine the block to pick, this enables to pick blocks from any distance

2: ClientPickBlockGatherCallback is called Mods that use this callback will now get the hit result from PBP and can possibly use this

3: PBP considers the item that's been supplied from the callback, however it's possible it'll be overriden (There is an overrides config file which will have priority)

4: PBP does all the item manuplation, adding nbt data etc (this could be with a supplied item from another mod that used the ClientPickBlockGatherCallback)

5: ClientPickBlockApplyCallback is called (This is the final chance for other mods to make changes to the item)

6: PBP does the inventory checks and manuplation

Every step in this flow is very different from the vanilla approach. Using the events like this would create the best intended effect for other mods that ues the events without knowing PBP is actually the one using the results. If I only put PBP into ClientPickBlockApplyCallback then this wouldn't be possible. I also would still need to add a mixin that would cut out the vanilla approach of adding the item into the inventory, which would again create the issues with ICT.

[Sjouwer]

Seems I switch the events arround in my #19 comment (code was correct), which confused me a bit, should be corrected now both there and here

[supersaiyansubtlety]

Ok, so with this approach ICT's "fill stack" option should work if I move it to a ClientPickBlockGatherCallback.

For ICT's "never change slots" option, iiuc it would need two API features: a) exposed locked slots b) an event after/during (6:) that allows modifying the movement of stacks in the inventory

a) seems reasonable for an API, b) seems rather contrived, maybe also convoluted

It's probably simplest for PBP to just re-implement ICT's "never change slot" option (which is fine with me), but if you want to implement b) I'd be fine with having ICT implement the feature from there (maybe you can come up with an event that isn't contrived, idk)

Any chance you can upload a jar compiled on the events branch so I can make sure ICT's "fill stack" option can work with it?

[Sjouwer]

It's probably better to use ClientPickBlockApplyCallback for that. That one supplies the final item right before the inventory manager does its job, which can still change before this point

As for the other one: I for sure wouldn't mind adding a), though I agree b) might not be that feasible. It probably works better if I try to implement that one into PBP

I've compiled the events branch and uploaded it here: https://github.com/Sjouwer/pick-block-pro/releases/tag/v1.7.20-beta

[supersaiyansubtlety]

Alright, this branch of ICT uses the FAPI events and disables its "Pick block never changes slot" feature if PBP is present: https://gitlab.com/supersaiyansubtlety/inventory_control_tweaks/-/tree/pbp-compat

It also adds a note to the "Pick block never changes slot" tooltip about PBP handling that feature if it's installed.

I checked with PBP: 1) no-key, ctrl, and alt picks fill the stack if a matching stack was already in the player's main hand 2) "Pick block never changes slot" does nothing 3) ICT doesn't interfere with PBP's bundle/shulker picking feature

There's also a compiled jar here if you'd like to do any of your own tests: https://gitlab.com/supersaiyansubtlety/inventory_control_tweaks/-/releases/pbp-compat-1

[Sjouwer]

Awesome, that sounds great, thank you. I haven't been able to check it out yet, but will do so very soon. Should have time this weekend.

[supersaiyansubtlety]

Updated version with fixed "Pick block never changes slot" behavior and 1.20.1 compatibility: https://gitlab.com/supersaiyansubtlety/inventory_control_tweaks/-/releases/pbp-compat-2

[supersaiyansubtlety]

Another updated version with 1.3.26 updates merged (there shouldn't be any changes to block picking functionality): https://gitlab.com/supersaiyansubtlety/inventory_control_tweaks/-/releases/pbp-compat-3

[Sjouwer]

Release that implements the feature and should hopefully resolve the compatibility issues: https://github.com/Sjouwer/pick-block-pro/releases/tag/v1.7.20-beta.2

[Sjouwer]

Fixed in release PBP 1.7.20 (and ICT 1.3.27)