PoGo-Devs / PoGo

UWP Client for Pokemon Go
https://pogo-windows.com/
MIT License
38 stars 9 forks source link

API behavior during specific types of requests #30

Open robertmclaws opened 8 years ago

robertmclaws commented 8 years ago

The new system should already minimize unnecessary requests from getting on the queue. But I want one more safeguard. There are certain times (catching a Pokémon, for instance) where we absolutely do not want duplicate requests to go out. If you "throw" the same Pokeball twice, for example. I imagine that kind of stuff would get us banned. I would like there to be safeguards higher up for this, but the queue is the "point of no return" so-to-speak.

The queue needs some kind of "No Duplicates" mode when adding to the queue. That would check the current message payload against any queued message payload, and if the request is a "No Duplicates" type and is a duplicate, just ignore it entirely.

ST-Apps commented 8 years ago

To be honest I'm not really 100% sure that this is needed. Talking about your example, we're sending Pokeball's id which is just Pokeball's type (instance of the ItemId enum), so basically each throw with the same Pokeball type will look as a duplicate.

If we're going to need it, I think that just defining Equals method for each non-duplicate payload may work. We just check if a duplicate is present when adding, if yes we refuse to add the item to the queue. Queue size should be small enough to let us check for duplicates without any issue.

Can you give me some more examples on where this kind of situation can happen?

robertmclaws commented 8 years ago

There have been several times in the current app where I have accidentally thrown a second Pokeball while the first throw is still processing, and the app then burns through both inventory. The new GameClients will be smart enough not to allow that to happen. BUT it may also be a good idea for certain things (like using Items) to have some way to not allow additional calls to be made until the first one has fully returned.

As far as other examples, I'm not sure at the moment. But that one is the most obvious to me because it's where I've been burned before.

skyne commented 8 years ago

we should define key properties for these type of requests (for catching the encounter id maybe?) so these can be checked (through an interface maybe? ) for duplication. Or maybe, shouldn't we just check for type? For the catching example, if one is "in progress" theoretically starting another catch is not even possible or not?

robertmclaws commented 8 years ago

Yeah, that's what I'm talking about. And while you're catching, certain other requests are not allowed at all. I want to enforce that kind of stuff at the queue level.

skyne commented 8 years ago

what kind of queue do you use? if we can define a dictionary of wwhich types are not allowed together, a HashSet or something like that should be enough, or if don't want to abuse Equals (which is reasonable in this case) cheking at "AddToQueue"?

robertmclaws commented 8 years ago

Right now we're using a ConcurrentQueue backed by a BlockingCollection. That way stuff can be dumped on from multiple threads, and the iterator waits for new items to come in. It's a super-useful pattern.

I'll probably use the same pattern I'm already using to check which requests are batched, and to map request types to response types. There are certain cases where we may need to look deeper than just a RequestType check, but I'm confident we'll be able to handle that reasonably.