vinnymac / PokeNurse

💉 A tool for Pokémon Go to aid in transferring and evolving Pokémon
284 stars 55 forks source link

Heavy Grinding #171

Closed 4tw0ne closed 7 years ago

4tw0ne commented 7 years ago

When grinding large amounts of Pokemon, the "leftover bits" tend to junk up the system.

Note in the screenshot the number of Pokemon. The actual number held is '357', not sure where or what number it adds to PokemonCount when a transfer occurs. It can range from a low difference added in to a large one.

This can happen randomly with ~20+ selected for transfer [without favs]. This was a recent download of the program and used the windows installer, although this issue was occurring on v1.3 as well. This may be in relation to #132 but I am getting different errors.

Please let me know if I can provide any more details.

pokenurseerrortransfer

PokeNurseErrorTransfer.log.txt

SimonMoua commented 7 years ago

I noticed how it occurs. I ran a quick test, and when transferring 10 pokemons, the total number of pokemon augmented at each transfers by a 2^n.

So if possible, either completely disable, or freeze the counter, as it throw an Out of Bound error, or see the cause of this runaway counter.

This error also happens when we Evolve the selected pokemons.

vinnymac commented 7 years ago

Something likely changed in the API response. Ill have to do some debugging. I dont have time to play the game, so if someone has a lot of real data to test with that would be convenient.

Do you receive any new things from transferring that you didnt used to? Maybe eggs? Since they are considered pokemon it could possibly break things.

SimonMoua commented 7 years ago

No, it's the same as before, where we receive only xp. The amount of pokemons should stay the same when evolving, and diminish by 1 for each transfer, or just update after all transfer are finished, as it does now.

On Nov 29, 2016 08:51, "Vincent Taverna" notifications@github.com wrote:

Something likely changed in the API response. Ill have to do some debugging. I dont have time to play the game, so if someone has a lot of real data to test with that would be convenient.

Do you receive any new things from transferring that you didnt used to? Maybe eggs? Since they are considered pokemon it could possibly break things.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vinnymac/PokeNurse/issues/171#issuecomment-263574808, or mute the thread https://github.com/notifications/unsubscribe-auth/AIFqZdAezcbICWbo-e46fSb7EAZV1mHMks5rDC3PgaJpZM4K-ivD .

vinnymac commented 7 years ago

Thanks for explaining that things are the same as they used to be. It puts my suspicions to rest then.

The error Unhandled rejection Error: [INTERNAL] in the log file that @4tw0ne attached is an internal error from pogobuf. It happens when pogobuf fails to send a payload via the pogo api and it attempts a retry of the failed request. We recently upgraded to the latest version of pogobuf, so if a previous one was working more consistently we could try to downgrade, but as the API changes, it won't be a viable solution forever.

The counting logic has not changed. In the case of the pokemon counter it just counts the length of a particular species pokemon array, and adds that number to the total count. In the end you get the total number of pokemon and we proceed to draw that value.

If it is that common I should be able to reproduce it and see some weird data appear in the pokemon array. If anyone finds any oddities in the datasets it would be useful to know.

SimonMoua commented 7 years ago

Exactly. The problem is with handlePokemonCounter. This error is there since the very start of the project, and happens whenever you transfer or evolve anything. I suspect that the line :

must be the wrong way to go, as the sum seems to increase by 2^n, where n = number of transfer/evolve. Also, unless the sum can sometime be negative (as it would be for the transfers), the transfer should maybe call a different function that only substract 1 each time.

This function is called on each transfer/evolve, and I really suggest calling it once it's finished only, or trying to replace "sum" by a flat +1 or -1, depending if it's a transfer or evolve.

I don't see where the sum is calculated... The problem should come from there. To be clear, I'm talking about this part:

handlePokemonCounter(monsters) { const initialCount = monsters.eggs.length return monsters.species.reduce((sum, specie) => { if (specie.count > 0) { return sum + specie.pokemon.length } return sum }, initialCount) }

vinnymac commented 7 years ago

The code you pasted is where the sum is calculated. We used to calculate things using +1 or -1 but it is very error prone compared to just recounting your dataset, this way everything is immutable and you can trust your data more. This is especially important when dealing with a terribly complex API like pogo. Reduce is a very efficient method that doesn't carry a huge loss in performance as we just have to iterate over an array for each transfer/evolve. It is counting, so given 8 eggs, it would start with 8, and then as long as a specie has some number of pokemon, we measure the length of the array and add that to the sum until we end up with the total count of pokemon.

This is the same logic that runs when you first sign into the app, and seeing as it works at that time, it must be something that happens/changes during the transfer/evolve process.

The logic is correct in that file as far as the math goes, what could be wrong is the lengths of the egg array or the lengths of the pokemon array. If for some reason they are mutated or given bad data (bad API response) then the counts would be off.

You can test the logic out and play with it here for yourself.

SimonMoua commented 7 years ago

Ok. If the math is correct and the API response is bad, is there a way to not try to display this value when we transfer/evolve? Maybe just not calling a refresh on the value when we transfer/evolve, as it does re-update correctly after the operation is finished.

Currently, it forces me to only transfer or evolve about 10 or 15 pokemons at a time, compared to the 70-80 I'd often want to transfer or evolve.

Thanks!

vinnymac commented 7 years ago

I think there are 2 separate issues here. One is that the count is wrong, and the other is that evolves/transfers are failing. The count being wrong is most likely a symptom of the failures. I don't think the count is the particular issue we should focus on. If we wanted we could set an upper bound on the count since we know it should never be higher than 1k or whatever the limit is nowadays. (What is the limit?)

Whatever is causing the arrays to grow to larger than 2^32 is the problem. When you do a small transfer you are less likely to run into API failures, and that is why these larger transfers see this problem so often. It also makes it that much harder to debug, because reproducing the issue requires a lot of test data. If someone with a lot of pokemon could run the app in dev mode by cloning this repo on the develop branch that would help a lot.

I just attempted a transfer with 10, and I saw no issues whatsoever, so my guess is that when an API failure does occur we get the exponential issue you described, which eventually breaks the app. In the latest release I let the app continue whenever we see internal errors in pogobuf instead of completely failing. I could undo that change and we could see if it helps, but then we would just be reverting the fix for #160.

SimonMoua commented 7 years ago

The upper limit is 1K, yes. I'm currently transferring pokemons, and I just noticed it incremented by 10, 20, 40, 80, 160... etc.

For running the app in dev mode... I cloned the repo, opened it in VisualStudio2015, tried to run it, nothing happens... I must confess I have no idea what I'm doing, I've been a C# dev for the last 2 years, not really have any idea how to run this. But I do have lots of pokemons, so I'd be happy to test it. If you want to, I suggest you download POKIIMAP, add your account there, and capture a few stuff around you by tapping on the pokemon scanned. It's like a manual bot for android. Not something I recommend for a real account, but go ahead for a test account.

vinnymac commented 7 years ago

@SimonMoua I think I have a new lead on this bug, I think the dependencies listed here are out of date. They might be causing the problem. I will try to fix them and do a patch release for v1.6.1 Let me know if it fixes things!

SimonMoua commented 7 years ago

Thanks, will do!

On Dec 5, 2016 22:27, "Vincent Taverna" notifications@github.com wrote:

@SimonMoua https://github.com/SimonMoua I think I have a new lead on this bug, I think the dependencies listed here are out of date. They might be causing the problem. I will try to fix them and do a patch release for v1.6.1 Let me know if it fixes things!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vinnymac/PokeNurse/issues/171#issuecomment-265050854, or mute the thread https://github.com/notifications/unsubscribe-auth/AIFqZa9Hpp97wJSi3c5HkEllc6YU3bqAks5rFNY2gaJpZM4K-ivD .

4tw0ne commented 7 years ago

I am more than happy to test anything out for the cause. I have been filling up my bag of 1000 Pokemon, its about 840 right now. I will do small tests then increment to larger ones. Coding wise my java is limited, but I can follow instructions well.

SimonMoua commented 7 years ago

So far, transferring 28 has worked well, the counter for total pokemons did not incremented like crazy, and no error were thrown. I'll catch all the random shit I can find and transfer over 100 later to test. But the fact that the counter does not increment seems to indicate the bug is fixed.

SimonMoua commented 7 years ago

Of wait... I selected 50 for transfer, it only transferred 25, said "complete" after, and did incremented the counter... I'll try again after catching more stuff image

SimonMoua commented 7 years ago

At the start of a mass-transfer:

image

And the end: image

So a mass transfer of 57 pokemons worked, no problems... Or so I thought. After I click OK, I see this: image

And upon closing-re-opening pokenurse, I see only 33 were actually transferred: image

SimonMoua commented 7 years ago

PoGo just updated to allow mass transfer (not mass evolves, though). If you can call their function for this directly, without handling it yourself, it could at least fix this problem for the transfers. As for evolves, you'd still need to handle them one by one.

vinnymac commented 7 years ago

@SimonMoua @4tw0ne I am not sure if you know how to build the develop branch, but I just pushed up batch support for transfers. If you can test it out for me I would appreciate it 👍 I started a bot account so I can now do more testing on real world data. It works for me, but I would like another user to try it out before shipping it.

SimonMoua commented 7 years ago

Sorry, but after downloading the branch and trying to run it in visual studio I got an error... I don't really know what I'm doing here. In any case, if you're testing and it works, great. I'm guessing you are referring to the mass transfer functionality that was added in the game a few weeks ago. I hope it will eventually work with evolves too, as the current version still increase the pokemon count exponentially and prevent me to evolve more than 15-20 pokemon at a time. The goal will be to eventually pop a lucky egg, select 70-80 pokemon to evolve, and not even have to monitor the process to know it will work during the next 30 minutes.

On Tue, Dec 27, 2016 at 12:45 PM, Vincent Taverna notifications@github.com wrote:

@SimonMoua https://github.com/SimonMoua @4tw0ne https://github.com/4tw0ne I am not sure if you know how to build the develop branch, but I just pushed up batch support for transfers. If you can test it out for me I would appreciate it 👍 I started a bot account so I can now do more testing on real world data. It works for me, but I would like another user to try it out before shipping it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vinnymac/PokeNurse/issues/171#issuecomment-269358510, or mute the thread https://github.com/notifications/unsubscribe-auth/AIFqZatNx-ARVKi_B2TAuDavVUZZOaciks5rMU7QgaJpZM4K-ivD .

vinnymac commented 7 years ago

@SimonMoua I decided to try to use the same batch mechanism for evolves that I was using for transfers, and it does indeed work. I evolved 10 pidgeys at the same time and it went fine, and I also transferred 30 pokemon and everything was good to go. I then did a transfer of 103 pokemon and there was an error, but all the pokemon transferred just fine despite the strange RPC error (just like the other ones we would get). The downside to this mechanism is that I can't give a time estimate anymore, so I had to remove the time estimate for an indeterminate spinner instead. In the future maybe I will add a guesstimate like '~80 mins left' or something.

If you are ever on discord and want to try to debug the error you see in VS let me know. I will push this code to the develop branch later today.