Its-Haze / league-rpc

Just a better Rich Presence for League of Legends. Showcasing Live Game Stats, Summoner Icons, Rank Emblems and Champion Skins on Discord.
GNU General Public License v3.0
28 stars 6 forks source link

LobbyInfo #10

Closed Bloodiko closed 10 months ago

Bloodiko commented 11 months ago

Adds Lobby details to RPC

Queue Name Current Player Count Max Player Count

curious if we manage to add a Join Lobby button. kekw - should be possible with the api maybe.

Its-Haze commented 11 months ago

If we are going to add additional features to the "in client" case, Then we should atleast have the following in place

  1. large_image should be reflected based on what type of lobby the user is in.. OR it should be set to the summoners icon of the user.
  2. We should try to use lcu-driver if we can (but if we have good reasons not to, then we can try to do things on our own)
  3. The RPC should be very similar to how windows version does it, BUT we should make our slightly better. (Meaning that whatever they have, we should also have but slightly more)
  4. If possible, We could try to use websockets instead of http to let the RPC only change when the user actually does a change. I believe that if we should do this, and use lcu-driver. then that process needs to be in it's own thread, and have it's own queue of responses from the (wss)
  5. Having a "Join lobby" button on discord would be nice to have but it should be optional.
  6. Customizability: We should let users decide what information they want to show, and what to keep private. (Maybe they don't want to have a button asking people to join) or they just don't want people to know that they are in queue of a ranked game.
Bloodiko commented 11 months ago

Should the pypresence update also happen inside the separate thread ? - or keeping updating to main thread only, passing data back to main when requested in the 10 sec interval

marking this as draft, until i made the necessary changes.

Bloodiko commented 11 months ago

i have adjusted most of the points.

Adjustability would be for a later update - I guess this should be configureable inside a config file, as not to have 1 million config flags for each option ?

Its-Haze commented 11 months ago

Bug 1

Custom games & Practice tool does not change RPC.

Steps to reproduce

  1. Start league-rpc-linux
  2. Create a Custom lobby / practice tool
  3. Observe your discord presence.

Expected result

rpc showing that you are creating a custom/practice tool game.

Actual result

rpc not updating.

Its-Haze commented 11 months ago

Bug 2

discord rpc not updating to "In Client" after game finishes.

Steps to reproduce

  1. Start league-rpc-linux
  2. Start a practice tool game
  3. wait a few seconds for discord rich presense to show that you are currently in game.
  4. Now exit the practice tool game (to come back to the client)
  5. Observe discord rich presence.

Expected result

Discord rich presence to show that i am in the league client.

Actual result

discord rich presence still showing that i am in a practice tool game.

Bloodiko commented 11 months ago

Bug 1

fixed

Bug 2

also fixed

Its-Haze commented 11 months ago

I tested it locally now and honestly, good job! It feels good and seems to work well.

However there are a few key points i would like to be fixed.


Do you think it makes sense to have a command line argument for "--no-icon or --no-summoners-icon" to let people opt out of showing their ingame summoners icon.. (It should then default back to the original league images that we have)?

Bloodiko commented 11 months ago

Do you think it makes sense to have a command line argument for "--no-icon or --no-summoners-icon" to let people opt out of showing their ingame summoners icon.. (It should then default back to the original league images that we have)?

I'd suggest an config file. Then when running with command line arg --set config.stuff=value or similar - this will require an additional help page, and some more work though.

I've implemented your comments. - Idle --> Online, typehints, ... Issue is, i'm unable to get rid of the Error when using the class approach. it worked when not using classes. - Not sure where the issue exactly is. PYTHONDEBUGASYNCIO=1 is also really vage about whats happening. Sometimes its in the rpc library - sometimes inside the lcu_driver library.
But the Update on the RPC side still happens - so even though i really dont like it, maybe we just ignore it until someone fixes asyncio / nested-asyncio?

Maybe its a weird interaction between nest_asyncio and self inside classes ? - this whole asyncio lib feels somehow unfinished. you need to await any async function inside an async function unless you schedule it with asyncio.run yourself, but thats not what we need. - is that really the purpose of async ? - that seems weird.

(leaving the error here, for google search references eventually. )

Exception in callback _SelectorSocketTransport._read_ready()
handle: <Handle _SelectorSocketTransport._read_ready()>
Traceback (most recent call last):
  File "/usr/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
RuntimeError: cannot enter context: <_contextvars.Context object at 0x7f16400229c0> is already entered
Bloodiko commented 11 months ago

I'd suggest rolling back to the non-classes approach. And see from there. As this does not work at all. Tons of error I don't know where they come from.

Its-Haze commented 11 months ago

Yea i am not an expert in asyncio or event loops but something does not seem to go hand in hand together.. Not sure if that is because of how we use it. Or if we are going with the wrong approach.. If you believe we should stick with the old version (without classes) then let's do that. We have tried classes and it clearly gives more problems than it was expected to solve.

Its-Haze commented 11 months ago

I can help out with type hinting. and some of the other details i commented on

Please rollback/revert the class change and go back to the commit where it was working as expected. and let's continue from there.

Bloodiko commented 11 months ago

Updated PR.

Its-Haze commented 10 months ago

Thank you for your pull request @Bloodiko !