Storage Drawers: Forestry Pack

Storage Drawers: Forestry Pack

6M Downloads

Add support for the Refined Relocation Sorting System

Dynious opened this issue ยท 11 comments

commented

I'd be awesome if you could add support for the Refined Relocation Sorting System. The Refined Relocation Sorting System automatically sorts items to neighboring blocks, this would work great with your mod. For more info about the Sorting System you can (for example) watch Udaldors spotlight: https://youtu.be/qh_c0PsFzvI?t=2m4s .

Implementing a Sorting System version of your block should be relatively simple. Here are the two classes used to make JABBA work with the Sorting System: https://github.com/Dynious/RefinedRelocation/blob/master/src/main/java/com/dynious/refinedrelocation/block/BlockSortingBarrel.java & https://github.com/Dynious/RefinedRelocation/blob/master/src/main/java/com/dynious/refinedrelocation/tileentity/TileSortingBarrel.java

The API can be found on GitHub: https://github.com/Dynious/RefinedRelocation/tree/master/src/main/java/com/dynious/refinedrelocation/api and can be downloaded from the CurseForge page on the mod: http://minecraft.curseforge.com/mc-mods/75811-refined-relocation/files/2233470

Thank you :)

P.S. Both our mods are in Jadedcats Agrarian Skies 2. I think this kind of mod compatibility could be a great way to increase our user base.

commented

I'd like to try and get compatibility, yes.

My main question right now, is: Does your API necessitate that I create separate Sorting versions of blocks, or is it possible to implement it on an existing block as a "dormant" feature until the proper upgrade is applied? And if yes, would the APIs all behave correctly as Optional?

commented

I think there's a way to make Sorting versions a part of existing blocks, but you will differ from the standard, so users might be confused. To make your tile not part of the grid you can basically not call onTileAdded() and when an upgrade is applied call this (and also when the block loads with the upgrade already installed). I have not tested this behavior though, so unexpected things might happen.

The FML Optional functions should work fine, I don't see a reason they won't.

commented

Hey, I'm currently in the middle of implementing this, but I've hit a little snag with the API. This method:

public void alterStackSize(int alteration);

Has no defined meaning for my inventories. It's not referring to slots or items, and drawers are multi-slotted and may have same or different items.

Also wanted to further comment on the above. The reason why I want to implement it directly on existing blocks and rely on upgrades is because I already expose 31 storage blocks consuming 6 IDs, and doubling that feels really icky.

commented

Oh, I see the problem. This extension of the API was made to make Barrels work with AE (which can extract more than a stack at a time) and apparently I didn't really think it through enough. I will make sure the next version (which I will release today) of the API will have a slot parameter.

I see why a new block wouldn't be a good option. I do think it's good to have sorting versions of the blocks appear in NEI or the creative menu when searching on 'sorting', otherwise it's very hard for users to find out what blocks can be part of the Sorting System and what can't.

You could make the Sorting block type recipe add a NBTTag to the ItemStack which will create a Sorting version of the block when its used to place the block. This ItemStack could then be a separate listing in the creative inventory / NEI.

EDIT: Made the change in the API, if there's nothing else you need I'll release a new version. Dynious/RefinedRelocation@8853f0d

commented

I don't think there's anything else I need. Won't know until I actually get to testing.

I don't know if any other mod implements this API currently, but this is a breaking change. Are/will you inform devs if there are other changes like this?

commented

I'm aware that this will break stuff, luckily no mods (that I'm aware of) use the ISpecialSortingInventory interface yet. This was introduced relatively soon and is only useful on barrel-type blocks. The JABBA implementation is on my side.

I do have a list of mods using the RR API and when a breaking change occurs for one of these I will inform them (hasn't happened yet).

EDIT: New version is up: http://minecraft.curseforge.com/mc-mods/75811-refined-relocation/files/2235511

commented

I had to relent and implement this as separate blocks/tiles. I couldn't use the architecture I wanted (implement a separate class as ISpecialSortingInventory that proxies back to the TileEntity). Though the registration asks for the interface, it casts back to a TileEntity internally. I also had poor luck making this conditional on my existing TileEntity, and just corrupted a bunch of blocks in my test world.

Still doing a bunch of cleanup before checking in, but it appears to be working.

I see a slight problem when the drawer is almost full. If I place a stack into the sorting chest that's larger than the remaining inventory space, RR doesn't show that the stack was split until I pick it up again or close the inventory.

commented

First off, thank you for keeping me informed with the problems you encounter. You're helping me a bunch with making the API better.

'Though the registration asks for the interface, it casts back to a TileEntity internally.'
After checking the code for a couple of minutes I think I found the issue. I think it's just the other way around. It asks for a TileEntity and it assumes it's an ISortingInventory (when registered as an inventory). This is not ideal and it's technically possible to split it (would need an extra ISortingInventory parameter when registering), but it's fairly logical to register the Handler on the class that needs the Handler (the docs state this requirement). I would prefer to force both TileEntity and ISortingInventory, but I don't think Java allows that.

'I also had poor luck making this conditional on my existing TileEntity, and just corrupted a bunch of blocks in my test world.'
Hmm, I don't know a reason why this would happen. Using Optional can be a bit derpy and breaks easily.

I'll take a look at the stack splitting bug. By not requiring the Containers of blocks to be altered for RR syncing items got a lot harder (Minecraft doesn't handle adding and immediately removing items for a container correctly), but this should be solvable.

Thank you for taking the time to do this!

commented

I've fully implemented support for Refined Relocation in 1.4.0-alpha2. It would be great if you could do some testing on it.

Sorting blocks will appear under a separate Storage Drawers tab.

commented

It works wonderfully! Thank you :D
The splitting bug is there, but is an issue on my side and will be fixed soon. Other than that everything works like it should be!

Do you mind if I tweet about this, or do you want me to wait until you release a stable version?

commented

I just released the stable 1.4.0, so feel free to tweet about it. I also threw in a check to simply not load the integration if the min version requirement is not met.