AdaptorPlayerInventory.isItemValidForSlot is always true? #572

Closed
fnuecke opened this Issue Dec 10, 2014 · 3 comments

Comments

Projects
None yet
4 participants
@fnuecke
Contributor

fnuecke commented Dec 10, 2014

I ran into an odd issue earlier, which finally boiled down to AdaptorPlayerInventory.isItemValidForSlot always returning true. Is there a reason for this, and not calling src.isItemValidForSlot instead?

I'm aware that this isn't usually an issue, because normally all slots of a player's inventory (except armor slots, which aren't part of the mainInventory) accept all items. However, certain fake players (in my case, OC robots) may have slots in their inventory that don't. I've worked around this by adding a validity check in my setInventorySlotContents method for now, dropping the stack into the world in case of an invalid insertion, but it'd be nice if this could be avoided.

The specific use-case where this was the problem? Robots activating conversion monitors to extract items. Which is really borderline, I know. So I'd understand if you'd just close this. But I'd just like to know if there's a reason for this (performance?), or if it might have been an oversight?

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Dec 10, 2014

Member

It might be a way to make it harder for fake players to use it.

Or just an oversight based on the assumption that only real player would use it.
If it is an oversight, than getSizeInventory is probably one and also affecting the WrapperChainedInventory.

Probably a question for @AlgorithmX2

Member

yueh commented Dec 10, 2014

It might be a way to make it harder for fake players to use it.

Or just an oversight based on the assumption that only real player would use it.
If it is an oversight, than getSizeInventory is probably one and also affecting the WrapperChainedInventory.

Probably a question for @AlgorithmX2

@AlgorithmX2

This comment has been minimized.

Show comment
Hide comment
@AlgorithmX2

AlgorithmX2 Dec 10, 2014

Contributor

All I can say is that I remember Player not really providing a usable isItemvalidForSlot implementation ( this might be false ) - Other then that I doubt there's a reason, if using that that method fixes it sounds reasonable to me.

Most of the inventory adapators are ports from AE1 to an extent, since I didn't feel like re-testing them and most of them works reasonably well, so they are ancient.

Contributor

AlgorithmX2 commented Dec 10, 2014

All I can say is that I remember Player not really providing a usable isItemvalidForSlot implementation ( this might be false ) - Other then that I doubt there's a reason, if using that that method fixes it sounds reasonable to me.

Most of the inventory adapators are ports from AE1 to an extent, since I didn't feel like re-testing them and most of them works reasonably well, so they are ancient.

@thatsIch thatsIch closed this in 72b3be0 Dec 22, 2014

thatsIch pushed a commit that referenced this issue Dec 22, 2014

thatsIch
Merge pull request #616 from thatsIch/e-572-valid-inventory
Fixes #572 Enable inventory validation for any kind of Player
@fnuecke

This comment has been minimized.

Show comment
Hide comment
@fnuecke

fnuecke Dec 22, 2014

Contributor

Thanks!

Contributor

fnuecke commented Dec 22, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment