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

WebSockets for LiveSplitOne #9

Closed
wants to merge 4 commits into from
Closed

Conversation

Lucki
Copy link
Contributor

@Lucki Lucki commented Sep 8, 2021

This integrates lso-remote. See #7 for additional info.

Fixes #7

@christofsteel
Copy link
Owner

Hey, super cool. One thing, that seems to be missing is adding the dependencies to the setup.py.
On that note, I don't think we should be using the gevent webserver, but rely on the built in webserver of flask. While this would not be as performant and scalable as an external webserver, this application won't have the load to justify another dependency.
Also please don't use imports anywhere else than at the top of a module, please see PEP 8 for that.

I will have a look at your other pull requests, but realistically I won't be having time until the weekend.

@Lucki
Copy link
Contributor Author

Lucki commented Sep 9, 2021

On that note, I don't think we should be using the gevent webserver, but rely on the built in webserver of flask. While this would not be as performant and scalable as an external webserver, this application won't have the load to justify another dependency.

While I completely agree here I haven't found a reasonable way to do so. Every search lands either on python-sockets which needs gevent or python-socketio which needs eventlet or gevent. The latter looks cleaner but I wasn't able to get it working in this program. From the requirements of both modules I take that it needs some stuff handling the connections and I'd rather not re-implement that stuff by hand.

This was also the intention of putting the import into the class instead at the top - being an interpreter I wanted to make them optional.

I'm not particularly knowledged in the python ecosystem so if you know how to do that don't hesitate to adjust the stuff needed - I gave you permission to edit this PR.

@christofsteel
Copy link
Owner

Still a couple of issues with the setup.py. There is no package called gevent-websockets (I presume you mean gevent-websocket (https://pypi.org/project/gevent-websocket/) and no external package called threading (Do you mean the standard library threading (https://docs.python.org/3/library/threading.html).
I fixed that, but then there is no module named flask_sockets. The only fitting library I found would be https://pypi.org/project/Flask-Sockets/ .

Since flask-sockets depends on gevent-websocket, it should suffice to add exactly this to the dependencies, (and remove gevent-websockets and threading.

I initially thought the flask-socketio library could be used (because it can work with the integrated webserver of flask), but it turns out, this combination only works for SocketIO, which is different to WebSockets. (SocketIO can use WebSockets, but can also work without it, but LiveSplit One explicitly requires WebSockets).

After installing all the modules, there is unfortunately still one problem... after starting the webserver, the rest of the program is not launched...

@Lucki
Copy link
Contributor Author

Lucki commented Sep 9, 2021

Arg, now it's biting me that I've splitted that stuff into smaller PRs. I don't have a functional game configuration without the other two PRs.

@christofsteel
Copy link
Owner

make a new branch locally and merge the other branches into that?

@Lucki
Copy link
Contributor Author

Lucki commented Sep 9, 2021

That's what I have here and it works…

after starting the webserver, the rest of the program is not launched

That sounds like that threading is somehow blocking?

@christofsteel
Copy link
Owner

That sounds like that threading is somehow blocking?

No, but I narrowed it down a bit. It happens when I start VVVVVV. For that pyAutosplit registers itself as a debugging process and inserts a breakpoint into VVVVVV. I am pretty sure, that execution happens up until that breakpoint. The VVVVVV process has been launched, but no game window or anything is visible.
My theory is, that the webserver catches the SIGTRAP signal, so execution never continues...

@Lucki
Copy link
Contributor Author

Lucki commented Sep 9, 2021

Hm, that would explain why I'm not getting this issue, I'm not using rsp and no breakpoints are added.

@Lucki
Copy link
Contributor Author

Lucki commented Sep 9, 2021

@christofsteel Please try changing monkey.patch_all() to monkey.patch_all(signal=False).

There's even the advice to do this as early as possible "ideally before any other imports".

@christofsteel
Copy link
Owner

Sorry, did not read your suggestion up until now. Solved it by directly using eventlets, and it's already included in the main branch.

@Lucki Lucki deleted the websocket branch September 15, 2021 00:10
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.

Allow using LiveSplitOne through WebSockets
2 participants