Multi lead search for gens 3, 4 and 5 - #591
Conversation
|
I'm not sure how I feel about this. I intentionally didn't bring over "Any" lead from RNGReporter. If I were to I would do it for Gen 3/4 more akin to how it was done there which is very different from what you did. As for Gen 5 I never did like it much especially when we consider there isn't really a Searcher class as it just generates SHA1 seeds then wrap around the generator. Almost seems like a bastardization of the generator concept to do it this way. I would probably have similar thoughts about the #598. |
Being able to search with multiple leads at once is just a major convenience upgrade; it's a just a full net positive for the user experience. If people don't want to use it, they can still just search with 1 or no leads. But if they do, they don't have to swap lead 10 times to get all possible results for e.g. a specific spread on a specific date. If you have a better way to go about implementing this so that its still the same benefit to the user and still reasonably fast then sure but this is just how I did it and it works fine enough |
|
There literally is a better way to do it for gen 3/4 and is in RNG Reporter. The way you have done it calls search() for each lead so its X times slower where X is the number of leads. I won't accept the changes as is on performance alone. As for Gen 5 refer to the other comment I left on the grotto PR about being injected as a after thought. I am also not entirely sure what this hashing approach is, granted I didn't spend a lot of time reviewing it but RNG Reporter also had a any lead search for gen 5 I would probably be more willing to accept. Seems like it also calls generate() for every lead which I won't accept on performance reasons as a starting point. |
|
Alright should be RNG reporters implementation now + a tiny bit more (also ignore the commit message typo lol its late) |
2 participants
This PR lets users search for targets using multiple leads at the same time instead of having to search 1 by 1