vinnymac / PokeNurse

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

Starred pokemons cant be selected #98

Closed iPaulis closed 8 years ago

iPaulis commented 8 years ago

The issue is simple. Usually I star the pokemons with high IVs so I can see at a glance which ones I would like to evolve. Favourited pokemons are now protected against transferring, but at the same time it is impossible to select them for evolving.

I suggest the program lets us select whatever we want, besides, as more features are added selections might be neccesary for other actions. If you wanna protect favourited ones, just add a warning if there are favourited pokemons selected when you click transfer: "The favourited pokemons in the transference queue wont be transfered". So the users are forced to unfavourite them if they really wanna get rid of them. This way we are allowed to select any pokemon including starred ones for other actions such as evolving.

iPaulis commented 8 years ago

I just realized that if you click in select all checkbox, starred pokemons are also selected, which means they are not very well protected against transferring. And selecting all and then unchecking all the not-starred ones for evolving is not very confortable. Unfavouriting, selecting, and favouriting again is not either.

mackhankins commented 8 years ago

I designed it so you can't transfer or evolve a favorite. They're selected when you check all, but if you look closely it's still disabled.

I chose this path for a reason. How does that software know what your action is going to be? if it isn't disabled the action (transfer/evolve) is going to collect the checked boxes. This is a UI issue, but that's the problem to solve to fix this..

The only fix I can think of off the top of my head is not checking for disabled checkboxes on evolve, but that's very confusing for people who notice it's disabled.

mackhankins commented 8 years ago

@vinnymac What if we could target the checkbox and add a class like isFavorite? Then we could check for that class and not collect it instead of :disabled. @iPaulis is right that the current approach is troublesome for other future features like batch renaming.

  _handleEvolved () {
    if (runningCheck()) return

    var selectedPokemon = document.querySelectorAll('input[type="checkbox"]:checked:not(#checkall):not(:disabled)')

    if (ipc.sendSync('confirmation-dialog', 'transfer').success) {
      running = true
      selectedPokemon.forEach((pokemon, index) => {
        ipc.send('transfer-pokemon', pokemon.value, index * randomDelay(2, 3))
      })
      this._countDown('Transfer', selectedPokemon.length * 2.5)
    }
  },
iPaulis commented 8 years ago

I understand the reason to protect favourited pokemons from transferring, but I don't understand why anyone would want to protect them from evolving. I mean, it doesnt make any sense, if they are your favourite means you prefer these pokemons over the rest and these are the ones you would evolve and not the others. Your proposal of actually checking favourite status instead of disabling checkbox seems the most logical approach to me. Well, that is just my opinion, but now I would like to explain the experience I just had. I had a huge problem with my mass evolve while my lucky egg was in countdown. I saw the checkboxes where diabled, but that only means they are not affected when they are clicked, and disabled doesn't mean unchecked so I thought that if they are already checked (as it is shown visually) they would count as the others. Whether they are really selected or not, the user is not warned in the whole proccess, the status changes and the countdown beggins so it is reasonable to think that everything is going well, but that was not the case. After 5 mins of my lucky egg I realized PokeNurse was not doing any actions because my experience in the game was not rising and the pokemon list ingame was not changed (there were no new evolved pokemons). So, just to be clear, PokeNurse did not evolve any favourited or not favourited pokemon. I had to restart the app and manually check the checkboxes one by one, as using select all would select the starred ones too and the problem could happen again without any warning. This led me to a question: how does the app behave when the user gives a command while the app is already performing a previous command? does it work like a queue system? does it add the actions sequentially or something like that? if it does, I didn't know and I think it is not clear for the user that the app can be used like that. If it doesn't, I think it could be a great feature.

Sorry for the long comment.

mackhankins commented 8 years ago

What you've discovered is a bug I just found tonight with large batches. It has nothing to do with this problem, but with not proper error reporting which is something we're trying to solve. (I'm betting this has been an issue from the get-go, but I can't confirm that)

Here is a the console output from 22 pidgey's selected and 7 favorited for a total of 15 evolves using check all.

[+] Retrieving player's Pokemons and Calculating Evolves
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Pokemon favorite status set to true
[+] Retrieving player's Pokemons and Calculating Evolves
[+] Pokemon favorite status set to false
[+] Pokemon favorite status set to false
[+] Evolved Pokemon with id: 8514695613794689009
[+] Evolved Pokemon with id: 1302878540481838990
[+] Evolved Pokemon with id: 16626312957905165455
[+] Evolved Pokemon with id: 17419346232043259381
[+] Evolved Pokemon with id: 13126792288149383579
[+] Evolved Pokemon with id: 6238286256194684777
[+] Evolved Pokemon with id: 13676866178295834777
[+] Evolved Pokemon with id: 6017313738193652890
[+] Evolved Pokemon with id: 12319937740216676508
[+] Evolved Pokemon with id: 11258959193237734418
[+] Evolved Pokemon with id: 6653936776966730330
[+] Evolved Pokemon with id: 5299445839125426032
[+] Evolved Pokemon with id: 10511435715331358389
[+] Evolved Pokemon with id: 5054201071463494323
[+] Evolved Pokemon with id: 8646797213163950158
[+] Retrieving player's Pokemons and Calculating Evolves

It works like a queue system by collecting all the checked boxes and filtering out the checkall box and disabled. It's really just a loop mentioned in a code snippet in a previous comment.

I totally agree this favorite / evolve thing is a problem, but so much of this will improve once we get #76 finished.

iPaulis commented 8 years ago

Thanks for confirming the bug, at least now I know what happened. About the queueing system, I meant what happens if I select 10 pidgies and click evolve, then select another 10 pidgies and click evolve again? does the program queue them up? replace the previous queue of 10 pidgies with the new queue? or doesn't allow you to proceed with another action until the last one is finished?

mackhankins commented 8 years ago

Ah, sorry. It won't let you. It has an is_running function which keeps that from happening. Queued batches sounds cool though. Ha, we gotta solve these problems first... I'm getting ahead of myself.

iPaulis commented 8 years ago

It sounds very cool indeed. I have some other nice ideas that I will share tomorrow when I have more time. I would like to help you a bit and support my request with the data you would need in a JSON.

vinnymac commented 8 years ago

@mackhankins now that we are keeping track of the selected checks on develop, how about we solve these problems another way.

Solutions

A. Instead of preventing favorites from being checked. We build another solution such as a confirmation modal. In the confirmation modal you will be able to choose whether you want to transfer all, transfer without favorites, or cancel. Then we update selected appropriately based on the option they choose. I think this will be easier for us to implement, and a better user experience in the end.

B. Whenever you check a favorited pokemon we disable the transfer button so that transfers cannot be done. When the user highlights the transfer button we could show a tooltip explaining why, the easiest and quickest of the solutions, but we limit functionality.

mackhankins commented 8 years ago

Option A fits more into our long term goals. I would wait till after 1.2 though. Unless you have a implementation of a modal in mind for confirmations.

vinnymac commented 8 years ago

Option B actually has a big downside which is how we would deal with someone un-favoriting. We won't know to reenable the button in that situation, or if we got new data where favorites had been modified. I am not sure what the right way to handle this is, all the proposed solutions have difficulty and downsides.

mackhankins commented 8 years ago

Well a list of stuff involved in the action solves this problem. I think it's self explanatory if something doesn't show up in the modal list. The list of affected Pokemon by an action could be a dumbed down version of PokemonTable.

iPaulis commented 8 years ago

Option A is the most functional, and the one that seems more logical. But there is another option that you might be able to implement easier in the meantime. Just assume people don't want to transfer favourites and make "transfer without favorites" the default behaviour while you prepare the other functionalities. You would just need to add a warning in the confirmation window: Are you sure you want to transfer these Pokemon? (Favourited Pokemon wont be affected). And let people manually unfavourite them if they want to transfer. This behaviour makes a lot of sense to me, would this be possible?

vinnymac commented 8 years ago
screen shot 2016-08-17 at 10 18 43 pm

@mackhankins I pushed the confirmation dialog to develop. I tested it very little, I think it does what we want though. Take a look when you have time.

Perhaps we will have a settings screen one day and we will make it so you can choose one of the options by default and stop showing the confirmation, but for now something like this seems like it will do.

This could also be improved easily by detecting whether or not any favorites have been selected before showing the button.

mackhankins commented 8 years ago

@vinnymac now you're just showing off. We can definitely work with that. I'll do some testing tomorrow and we'll need one for evolve.