New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Pocket API working outside of player inventory #331

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@SquidDev
Contributor

SquidDev commented Jun 22, 2017

This was cherry picked from #314 - see there for an explanation of the changes made and why.

@Iunius118

This comment has been minimized.

Iunius118 commented Jun 22, 2017

Thank you very much!

}
EntityPlayer player = (EntityPlayer) m_computer.getEntity();
InventoryPlayer inventory = player.inventory;
if( !m_computer.inInventory() ) return new Object[] { false, "Not in an inventory" };

This comment has been minimized.

@dan200

dan200 Jun 28, 2017

Owner

Why is this check necessary? Why does m_computer.getEntity() return the player when the player isn't holding the item anymore? That's what needs to be fixed.

This comment has been minimized.

@SquidDev

SquidDev Jun 28, 2017

Contributor

That would be better, then I'm not entirely sure of the best course of action here. We can't really track when the pocket computer enters/exits a player's inventory - the only notification we get is the item's update method.

Consequently, our only real strategy to determine whether the entity is valid is to scan the entire inventory for the pocket computer stack. We could add such a check to the getEntity method, returning null if it is not in our current entity's inventory. However, that does mean it is no longer thread-safe, which may be a problem in some situations.

This comment has been minimized.

@Iunius118

Iunius118 Jul 1, 2017

How about finding the non-updated pocket computers on ServerTickEvent(Phase.END)? And if they are found, their privete fields are set to null.

@Iunius118

This comment has been minimized.

Iunius118 commented Jul 7, 2017

This problem (pocket computer not held by a player isn't updated) may affect not only Pocket API but also pocket upgrade peripherals. They can't keep up to date if their IPocketUpgrade.update() isn't called.

@SquidDev

This comment has been minimized.

Contributor

SquidDev commented Jul 8, 2017

@Iunius118 Yeah, that is an issue I'm aware of. I'm not really sure of a feasible solution - I guess you could fire the update methods in the ServerComputer instead, but that isn't an ideal solution.

@Iunius118

This comment has been minimized.

Iunius118 commented Aug 3, 2017

We currently use only Item.onUpdate() to update pocket computer stack but we also can use Item.onEntityItemUpdate() to update the stack dropped on the ground.

Also, we may use TickEvent.PlayerTickEvent and its event.player.inventory.getItemStack() to update the stack held by mouse. However, this approach is not available for the creative inventory because its item stacks are handled by GuiContainerCreative on the Client.

@SquidDev

This comment has been minimized.

Contributor

SquidDev commented Sep 10, 2017

@dan200 I've rebased onto master, which fixes issues introduced by the 1.11.2 transition.

It's become apparent that the current proposed fix isn't ideal. I'm wondering if you have any thoughts on the above comments & questions, so we can find a better solution to these problems.

Iunius118 and others added some commits Jun 21, 2017

Fix Pocket API working outside of player inventory
This makes Pocket API not equip/unequip upgrades when the pocket
computer is outside of the player inventory (e.g. dragging,
dropped, placed in a chest).
Check whether the stack is in the inventory
This means we're guaranteed to edit the correct stack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment