pokemongo-dev-contrib / pokemongo-json-pokedex

Transform the data from the Pokemon GO master files to a better readable & processable JSON file.
Apache License 2.0
54 stars 14 forks source link

Id field on pokemon is not unique since Alolan forms were added #8

Closed whaevr closed 5 years ago

whaevr commented 6 years ago

For example there are 3 entries with the id of RATTATA, looking at the template_id for RATTATA in decompiled game_master I see the following

  template_id: "V0019_POKEMON_RATTATA"
  template_id: "V0019_POKEMON_RATTATA_ALOLA"
  template_id: "V0019_POKEMON_RATTATA_NORMAL"

same with exeggutor and any other 'mon that has an alolan form

Odd they decided to make a third "NORMAL" entry for these but, should template_id be used in place of pokemon_id since it is truly unique?

luissmg commented 6 years ago

I am having the same problem here. That third element is unnecessary, it does not bring any value. About the Alolan form... Ye, there should be at least a way of identifying the form of the pokémon. That would make my life easier too cause I can show different images for both types.

BrunnerLivio commented 6 years ago

Sorry for the late response. I was enjoying the sun during my holidays ;) Good catch!

See PR #9. Review if you see something, otherwise I'll merge it in 2 days. I also added test cases for this, which made me realize the id for Nidoran Male / Female was not unique too. Fix that one too..

Thanks for your contribution

feralheart commented 6 years ago

Also Dexys and Castform has duplicated normal and three alternativa forms with not unique id. Castform forms:

Deoxys forms:

For castform easy to distinguish them becouse of the type, but for Deoxys it's vare hard (all Deaxys has 1/1/1 stats in the GM.json and same type same moveset)

I made a function in my class what looks after the forms (for example if Castform's type is fire than it get's CASTFORM_FIRE id. Also if Marowak's type is dark/ghost than he got's MAROWAK_ALOLAN id).

BrunnerLivio commented 6 years ago

What are "*_NORMAL" forms actually for? They seem like the default version, but with less information? E.g. MAROWAK_DEFAULT does not store pastBranches compared to MAROWAK.

I can remove these "duplicates" if they do not bring any benefit..

luissmg commented 6 years ago

@BrunnerLivio First of all, hope your holidays were awesome :) Second, I think the Nidoran case is pretty much good covered. Forgot to mention that. I think that @feralheart is right. Those 2 Pokémon have 4 different types each.

I think that in those cases and with the Alolan there should be a new property in the json called forms or something like that for them. I saw you changed the ID to make then unique. That solves the problem partially for me. When you loop throught the array you will still find 2 Rattatas. I would prefer to have 1 Pokémon and a field form that would contain an array of Pokemon's (the data type). BUT, the solution you provide is fine too and I can get the json and adjust it my way. It should not be that much difficult.

About the _NORMAL forms that you talk... I don't know anything about the game master, but at least in this json the stats are the same (as far as I can tell), so I don't know what is the difference...

whaevr commented 6 years ago

I think the json shouldn't be changed too much from the existing game_master. Having 2 Rattata is fine as long as there is a way to distinguish with certainty that one is alolan.

Adding extra fields and shuffling the data around seems like something that shouldn't be done here if this project focus is translating the game_master to json format

luissmg commented 6 years ago

How do you detect the normal form of Deoxys and Castform? There should be a way to get that information. And how do you detect if it is the Alolan version? Detect if the ID as _ALOLAN in it?

whaevr commented 6 years ago

template_id gives us all that information, the different forms of deoxys have the form name in the id. Same with castform. If there is no additional form name in the template_id you can assume its the normal

luissmg commented 6 years ago

Hmmmm Ok, so maybe the trick is to check if the id is different from the name. If it is, it probably is because it's not the normal form. Ye, I can live with that. But only if no one as the idea to add a Pokémon where the name also contains the form. That would break all the verifications.

BrunnerLivio commented 6 years ago

As far as I understood @luissmg you suggested something like this:

Forms Map

{
   "name": "Deoxys",
   "id": "DEOXYS",
   "forms": [{
    "name": "Deoxys Attack",
    "id": "DEOXYS_DEFENSE"
   }, { ... }]
}

I think adding a "map" to the other forms is reasonable. But the other forms should stay a seperate Pokemon entry in my opinion, as I did implement it at the moment in #9.

I'll make a seperate issue for this map and merge #9 as soon as its ready beforehand. Thats because I currently have a lot of pending PRs and I do not want to have too many merge conflicts in the end :)

Normal

About the "normal"-Pokemons; I think I'll delete those entries, but add a test which checks if any "_NORMAL" form differs from the main form. So I won't forget about those and can take action in case they differ from each other.

Evolution

I just checked the Pokemons and realized there are Alolan evolutions? Like for example Geodude -> Graveler. I do not play PokemonGO anymore and also never played Sun & Moon. Am I right the Alolan Geodude can only transform into the Alolan Graveler (not normal one) etc.? If so I had to update the evolutions, because at the moment Alolan Geodude transform into normal Graveler according to the outputted JSON.

Can you guys approve my decision or is this the wrong way? @luissmg @whaevr

BrunnerLivio commented 6 years ago

Oops merged #9 by accident :) I think I won't revert it and apply the proposed changes in a new PR

whaevr commented 6 years ago

@BrunnerLivio you are correct that alolan forms only evolve into alolan evolutions! Its all new to me aswell, I didn't play Sun/Moon myself but am kinda interested in doing so after seeing them in Pogo. There are some interesting cases currently such as Alolan Marowak being a raid boss currently but alolan cubone is currently absent from Go. Same situation with Raichu/Pikachu, alolan raichu is a raid boss with no alolan pikachu (currently). Must be alolan to evolve into alolan

Im fine with it being handled how you have proposed 👍

luissmg commented 6 years ago

Hey guys, could not responde until now.

Yes @BrunnerLivio, that is what I was suggesting. What you propose is really the best approach into this! I agree with you.

Abiut the normal forms, I also agree on this. If they are indeed the same, they become useless in the json.

Yes, I played Sun and Moon and the Alolan forms only evolve into Alolan forms, sorry for not bringing it up before. I did miss that when I analized the json. It must be fixed in the versions.

luissmg commented 6 years ago

As I remember... Since you brought Pikachu into the conversation. In the Sun and Moon, there are only the Alolan versions of the Pokémon. So, a Pikachu will always evolve into the Alolan Raichu.

In Pokémon GO that is not the truth, as far as I know. You evolve Pikachu into a normal Raichu and you can catch an Alolan Raichu. Please, correct me if I am wrong!

So, I would just fix the Geodude evolutions and others like him that are wrong. But in the cases like the one I mentioned, the data is already ok. Once again correct me if I am wrong :)

BrunnerLivio commented 5 years ago

I've merged #16 so now *_NORMAL-Pokemons are deleted. I'll close this issue but created #18 to keep track of the Alolan Evolution Bug.

Thanks for your contributions!