Storage Drawers: Forestry Pack

It seems that the canItemBeStored method in DrawerData does not take into account lock state

StorageDrawers/src/com/jaquadro/minecraft/storagedrawers/storage/DrawerData.java

Lines 136 to 142 in 1ba1d14

    
           @Override 
        
           public boolean canItemBeStored (ItemStack itemPrototype) { 
        
               if (protoStack == nullStack) 
        
                   return true; 
        
               return areItemsEqual(itemPrototype); 
        
           }

If the drawer is locked but not set to a item an item should not be able to be stored there. CompDrawerData has the same issue

so I assume the other mods we talked about (Like Thermal Dynamics) in the pull request (RS485/LogisticsPipes#839) implement their own checking for this?

It seems so. I can't really find where that code would be but it would make more sense for the checking to be done within StorageDrawers it self.

The original design of locking allowed items to be stored in empty locked drawers. This can be fixed at the API level.

So I guess the question is if changing that will break anything.

If not, I definitely vote to change it to that, it would definitely make some other interactions much easier.

@jaquadro

I guess I'd prefer to support having automated methods such as LP and AE2 not put into these types of drawers, as I think it has a higher likely-hood to break something than it does to actually be useful to someone.

@jaquadro do you think this should be implemented by changing the API behavior, or adding a check to Logistics Pipes?

I don't remember exactly how I was inserting the items (probably a Thaumcraft magic mirror going into a hopper on a controller slave...), but I recall having a 2x2 locked drawer with one unused slot that often had random items put into that slot. 👍 for more strictness on locked drawers.

Well I actually see where jaquadro is coming from. Other mass storage mods like JABBA support locking where if you lock a empty barrel you can still add a item to it.

So In the case in reference to RS485/LogisticsPipes#839 Logic Pipes should check if a drawer is locked and empty with no item set and disallow items to be added.

Well, you can manually insert items into a locked drawer, but from my observations a lot of people found automated input dropping things into empty locked drawers to interfere with their systems. So it was changed.

Thermal Dynamics and the like are unaffected because they just use the normal IInventory interface, which is where that behavior was changed. Only special integrations like LP and AE2 rely on the API.

@jaquadro

@jaquadro Do you think there would be any problem with making this change to the API? I really think it would be beneficial for logistics pipes, but I'm not sure what else it might effect. (AE?)

If you think there might be other issues, I can make some changes to instead implement it just from the Logistics Pipes side.

Sorry, I intended to update the API this past weekend, but it didn't happen. It should be fixed, and I don't think it would impact AE2 (rather, it would probably improve AE2's behavior in the same way).

I submitted a PR with this.

Obviously, I'm not as familiar with the code as you, but as far as I could trace it back this covers all bases.

Definitely works in a cursory test with every common use case I could think of.

	@Override
	public boolean canItemBeStored (ItemStack itemPrototype) {
	if (protoStack == nullStack)
	return true;

	return areItemsEqual(itemPrototype);
	}

Share to