thesadru / genshinstats

PLEASE USE GENSHIN.PY A python library that can get the stats of Genshin Impact players using Mihoyo's API. PLEASE USE GENSHIN.PY
https://thesadru.github.io/pdoc/genshinstats/
MIT License
265 stars 38 forks source link

Fixed issue with using multiple cookies #67

Closed mgundelfinger closed 1 year ago

mgundelfinger commented 1 year ago

Hey there!

My main goal is to make a discord bot that is able to give any user the opportunity to register their cookie and token data to be able to get info about their account like current resin, characters, etc. as well as be able to redeem codes from discord chat through a command.

So far this wasn't working since every time more than one cookie was registered through the set_cookies function, only the earliest registered cookie would work. I found out this is because an exception is thrown anytime a cookie is checked against a UID that doesn't match the cookie, which stops the program from looping through the rest of the registered cookies.

I went ahead and made the according changes. If you have any questions or concerns please let me know!

I see you are working on Genshin.py now, but haven't seen any indication that it is meant to work for multiple users / cookies. Do correct me if i'm wrong, but if I'm right, I would like to ask why that is the case.

Thank you so much!

GauravM512 commented 1 year ago

You store the cookies in a database, and make function which gets the cookie with the client call so when the user does wants resin it will only use users cookie.

Multicookie manager can be used to get not user related data like record card and calculator data etc

GauravM512 commented 1 year ago

Also why not use asynchronous package genshin.py for discord bot . It would be faster and non blocking

mgundelfinger commented 1 year ago

Thanks for the reply @GauravM512!

So is there no difference to the ratelimit between saving all the cookies with set_cookies and just sending the user's cookie with each request? The way I understood it is that the set_cookies function was meant to bypass the ratelimit problem.

And I would love to use genshin.py but I also need to know how it handles multiple cookies and the ratelimit issue.

thesadru commented 1 year ago

genshin.py has all the features genshinstats has, just with several improvements thanks to asyncio.

GauravM512 commented 1 year ago

Thanks for the reply @GauravM512!

So is there no difference to the ratelimit between saving all the cookies with set_cookies and just sending the user's cookie with each request? The way I understood it is that the set_cookies function was meant to bypass the ratelimit problem.

And I would love to use genshin.py but I also need to know how it handles multiple cookies and the ratelimit issue.

It has rate limits but to get user resin related information , user character ,teapot info etc you have to use specific users cookie. To get information like activate days , spiral overview (detailed requires user cookie) , any character info using calculator you set a multi cookie manager with mock accounts

thesadru commented 1 year ago

The common usage is to have a shared public client with multiple cookies and then a database of cookies for individual users.

async def get_client(self, user_id: int) -> genshin.Client:
    cookies = await self.database.fetch_user_cookies(user_id)
    if not cookies:
        return self.public_client

    return genshin.Client(cookies)

async def show_user(self, context, uid: int):
    client = await self.get_client(context.invoker.id)
    user = await client.get_genshin_user(uid)
    ...

You can do more fine-tuning by also storing the language and maybe also using different cookies based on what resource/user is being requested.

mgundelfinger commented 1 year ago

Okay, so the rate limit is 30 request per cookie per day? Or am I misunderstanding that part? But thank you for the explanation, @GauravM512, that makes a lot of sense!

My change makes it possible to register cookies for multiple users to get user-specific data without having to send the cookie with each request! Is that something you would consider adding instead of (or alongside) a user database, @thesadru ?

That way we wouldn't need to specify the cookies with each request, it's not a necessary change, but maybe it's something worth considering.

Either way, thank you so much for the help with this, I will try to use genshin.py for my bot!

thesadru commented 1 year ago

If you plan to use it to not have to set cookies every time this is a very bad approach. You would be forced to cycle through cookies making the request take considerably more time to execute.

The common (and recommended) approach is to "wrap" a library to add this functionality yourself by also getting the cookies from your database at the same time.

Here's an example of the most popular bot doing this: https://github.com/KT-Yeh/Genshin-Discord-Bot/blob/master/utility/GenshinApp.py (Please disregard the camelCase and OOP abuse)

mgundelfinger commented 1 year ago

The goal isn't to eliminate the need for sending the cookie with each request. The goal is to circumvent the rate limiting. I was unclear how the rate limit works, but it seems you can make 30 requests per cookie per day, correct?

Closing, since my change would not solve any issue as the original behavior is intended!

thesadru commented 1 year ago

My issue with it was specifically that it would also try cookies that should be known are invalid.

mgundelfinger commented 1 year ago

Yeah it makes sense now! But did I understand correctly how the rate limit works?