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.

Sign up for GitHub

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

Jump to bottom

Heavy Grinding #171

Closed
4tw0ne opened this issue Nov 29, 2016 · 19 comments
Closed

Heavy Grinding #171

4tw0ne opened this issue Nov 29, 2016 · 19 comments
Labels
Type: Bug
Milestone
v1.7.0

Comments

@4tw0ne
Copy link

4tw0ne commented Nov 29, 2016

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
Copy link

SimonMoua commented Nov 29, 2016 •
edited
Loading

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.

  • 1rst transfer: +1
  • 2nd transfer: +2
  • 3rd transfer: +4
  • 4th transfer: +8
  • 5th transfer: +16
  • 6th transfer: +32
  • 7th transfer: +64
  • 8th transfer: +128
  • 9th transfer: +252
  • 10th transfer: did not showed, it stops at 90%.

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
Copy link
Owner

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
Copy link

SimonMoua commented Nov 29, 2016 via email

@vinnymac
Copy link
Owner

vinnymac commented Nov 29, 2016 •
edited
Loading

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
Copy link

SimonMoua commented Nov 29, 2016 •
edited
Loading

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 :

  • return sum + specie.pokemon.length

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
Copy link
Owner

vinnymac commented Nov 29, 2016 •
edited
Loading

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
Copy link

SimonMoua commented Nov 29, 2016 •
edited
Loading

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
Copy link
Owner

vinnymac commented Nov 29, 2016 •
edited
Loading

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
Copy link

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
Copy link
Owner

vinnymac commented Dec 6, 2016 •
edited
Loading

@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
Copy link

SimonMoua commented Dec 6, 2016 via email

@4tw0ne
Copy link
Author

4tw0ne commented Dec 6, 2016

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.

@vinnymac vinnymac added this to the v1.6.1 milestone Dec 6, 2016
@SimonMoua
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Owner

@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
Copy link

SimonMoua commented Dec 29, 2016 via email

@vinnymac
Copy link
Owner

@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.

@vinnymac vinnymac modified the milestones: v1.7.0, v1.6.1 Dec 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
v1.7.0
Development

No branches or pull requests
3 participants
@vinnymac @SimonMoua @4tw0ne