Skip to content
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

Add inv_ready event to signalize when the inventory can be used #135

Merged
merged 1 commit into from
Sep 19, 2015

Conversation

Gjum
Copy link
Member

@Gjum Gjum commented Sep 11, 2015

A new inv_ready event is emitted when the inventory got refreshed by the server, which happens after connection and when a new window is opened or closed.

Also sends inv_open_window when the player's inventory is opened initially.

The inv_open_window is delayed until all slots of the window are updated, so that a handler does not work with the wrong inventory.

Closes #118.

@@ -144,6 +145,16 @@ def __init__(self, ploader, settings):
# stores the last click action for confirmation
self.last_click = None

# to know when the inventory got synced with server
self.is_synchronized = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a situation where can is_synchronized can be undefined? Should it be set in the class definition or initializer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be an instance property, so it should be undefined when no instance (and thus no inventory to be synchronized) exists.

By the way, other plugins cannot access this anyways, as it is in InventoryPlugin, not in InventoryBase.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I actually misread the diff and thought it wasn't being set in __init__!

@gamingrobot
Copy link
Member

Needs a better name than inv_ready, due to the fact we dont really know what the event entails.

@gamingrobot
Copy link
Member

I would like @nickelpro to review and merge if he thinks this is good.

@nickelpro
Copy link
Member

Only thing I'm unclear on @Gjum is the condition you're using to signal inventory synchronization. What are the significance of those id constants? I'm sure your behavior is correct I'd just like to understand before I merge

@Gjum
Copy link
Member Author

Gjum commented Sep 18, 2015

@nickelpro When the server notices that the clients inventory state is wrong, it sends a window items, a set slot for the cursor, and a set slot for every non-empty slot. So by hooking into the cursor set slot, we know that at this point the clients inventory state is correct again.

@Gjum
Copy link
Member Author

Gjum commented Sep 18, 2015

TODO: when merged, change all event names from inv_ to inventory_ (see also #139).

EDIT: done in #146

nickelpro pushed a commit that referenced this pull request Sep 19, 2015
Add inv_ready event to signalize when the inventory can be used
@nickelpro nickelpro merged commit 5525f0d into SpockBotMC:master Sep 19, 2015
@Gjum Gjum deleted the window-open-feedback branch September 20, 2015 14:15
@Gjum Gjum restored the window-open-feedback branch September 20, 2015 14:15
@Gjum Gjum deleted the window-open-feedback branch September 20, 2015 14:15
@Gjum Gjum added the api label Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an event for when the inventory is loaded
4 participants