talwat / pokeget-rs

A better rust version of pokeget.
MIT License
91 stars 3 forks source link

check for 0 #2

Closed TheCrazyGM closed 11 months ago

TheCrazyGM commented 11 months ago

If pokeget 0 is run, it will panic, probably should add a check for that.

thread 'main' panicked at 'index out of bounds: the len is 905 but the index is 18446744073709551615', src/sprites.rs:23:33
talwat commented 11 months ago

Yeah that's a good idea. Should I redirect it to bulbasaur or a random pokemon or just put an error message?

TheCrazyGM commented 11 months ago

I personally would have it print a message to enter a valid Pokedex id.

On that same note, I'd check the upper end too, pokeget 10000000 also panicked.

On Sat, Aug 19, 2023, 3:20 PM Tal @.***> wrote:

Yeah that's a good idea. Should I redirect it to bulbasaur or a random pokemon or just put an error message?

— Reply to this email directly, view it on GitHub https://github.com/talwat/pokeget-rs/issues/2#issuecomment-1685088311, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4N7IDTZN356BEU7PUWHETXWEGYPANCNFSM6AAAAAA3WWAMW4 . You are receiving this because you authored the thread.Message ID: @.***>

talwat commented 11 months ago

I decided to just redirect it to a random pokemon, but for the high one I think it should say the same "pokemon not found" error as you get from pokeget doesnotexist.

TheCrazyGM commented 11 months ago

That makes sense. Would be better than having to adjust the upper check when new ones get added to the sprites in the future.

On Sun, Aug 20, 2023, 8:20 AM Tal @.***> wrote:

I decided to just redirect it to a random pokemon, but for the high one I think it should say the same "pokemon not found" error as you get from pokeget doesnotexist.

— Reply to this email directly, view it on GitHub https://github.com/talwat/pokeget-rs/issues/2#issuecomment-1685270536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4N7IDEP6CTOM55PMGBSRLXWH6IDANCNFSM6AAAAAA3WWAMW4 . You are receiving this because you authored the thread.Message ID: @.***>

talwat commented 11 months ago

Alright, it's done in the latest release.