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 switch_server method #15

Merged
merged 20 commits into from
Jun 18, 2022
Merged

Conversation

s4w3d0ff
Copy link
Contributor

still needs to handle the registered entity event listeners
not sure how to go about this but I'm assuming that you need to iterate through the current registered listeners, if the listener is an instance of EntityEvent then remove it.

also have no idea how to restart the socket after disconnecting it, is it await self.connect() or does it need to be added to the current asyncio loop?

still needs to handle the registered entity event listeners
@s4w3d0ff
Copy link
Contributor Author

I also haven't tested this yet

Copy link
Owner

@olijeffers0n olijeffers0n left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I have left a few comments but feel free to message on discord with regards to the entity listeners

rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@s4w3d0ff s4w3d0ff left a comment

Choose a reason for hiding this comment

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

I dont know what happened lol the tabs are 8 spaces wide and I cant get them to change to 4 spaces like how the rest of the repo is

Copy link
Owner

@olijeffers0n olijeffers0n left a comment

Choose a reason for hiding this comment

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

More Changes

rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
rustplus/api/base_rust_api.py Outdated Show resolved Hide resolved
@olijeffers0n
Copy link
Owner

That should be the Remote & Command stuff all done, just need to remove the entity events. They are set as attributes to the EventHandler iirc

@s4w3d0ff
Copy link
Contributor Author

from my understanding the attribute names are the ids of the entities? maybe vars(self.remote.event_handler) will work, or vars(self) if you want to add a method like EventHandler.clear_entity_events() then iterate the value of vars(self) and isinstance(x, EntityEvent)

@olijeffers0n
Copy link
Owner

olijeffers0n commented Jun 14, 2022

Thanks, I cannot test this right now but the diff looks good. One thing - are you expecting the tabs and spaces to be fine?

@s4w3d0ff
Copy link
Contributor Author

I just downloaded and ran this branch, rn it seems that method hangs on await self.disconnect at line 178 not sure why

@s4w3d0ff
Copy link
Contributor Author

I didnt get a tabs/space error so it seems to be fine

have no idea why 'close' doesn't work, it has a timeout param that defaults to 3 secs but still hangs after 3 secs
@s4w3d0ff
Copy link
Contributor Author

tested and seems to be working

@s4w3d0ff
Copy link
Contributor Author

The entity listeners and command handler I didn't test, my bot doesn't use them (but probably will in the future) and I didn't write a test script for them.

@olijeffers0n
Copy link
Owner

Seems ok for now. Command handler is fixed but haven't tested the entity events. If someone could that would be appreciated :)

@olijeffers0n
Copy link
Owner

Hmm, I am confused as to the root cause of super.close() hanging the thing. What does shutdown do differently?

@s4w3d0ff
Copy link
Contributor Author

From what I understand, close() (https://github.com/websocket-client/websocket-client/blob/master/websocket/_core.py#L458) sends some data to the connected server to tell it to stop sending data (sort of an "unsubscribe") while shutdown() will just close the socket immediately regardless. Could also try send_close() first before the shutdown() to still send that "unsubscribe" message https://github.com/websocket-client/websocket-client/blob/master/websocket/_core.py#L442

But I have no clue why close() isn't working

@s4w3d0ff
Copy link
Contributor Author

Just saw this issue: websocket-client/websocket-client#592 seems to be similar to what I had experienced.

Copy link
Owner

@olijeffers0n olijeffers0n left a comment

Choose a reason for hiding this comment

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

LGTM

@olijeffers0n olijeffers0n merged commit 1b3c106 into olijeffers0n:master Jun 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants