[Suggestion] pokemon types data

[udnp]

Hi GoIV pj team.
I have a suggestion for adding pokemon types data to GoIV.

Currently this use is just improvement to remove hard coded pokemon types index number
in my previous merged PR #727 for eeveelution correction.
But I guess there might be other uses that need pokemon types data.

In this PR, a pokemon type value is indicated with 2 digits base 20 numerical value defined in the new resource file types.xml.
The 2nd digit means 1st type, and the 1st digit means 2nd type.
For example, "10" means NORMAL single type pokemon like Blissey,
or "BI" means PSYCHIC/FAIRY double type pokemon like Gardevoir.

What do you think about this suggestion?

P.S. Currently type data for all pokemons except "eeveelutions" are dummy values.
If this suggestion would be adopted these data should be filled with correct values.

[nahojjjen]

Hey, I currently dont have a clear understanding of what we can use type information for.

I know we can use it for clipboard tokens and correcting the eevee evolutions.

I cant review this properly without knowing where we''re planning on using it, but it looks to be implemented well. (except for the part where no pokemon currently has any type...)

[thearaks]

I like the idea of refactoring the Pokémon data model (and I mean all of it species, moves, types, scanresults...), I'd also like to move all the values (base stats, types, moves, ...) from android resources to static instances of the new data models (all except for names strings that need to be localized like species names and moves).
That said, having type information implemented with a base20 string/integer is a bit too much obscure to debug and, in general, to handle inside at code level: you can't do something like pokemon.types.contains(Type.FAIRY) or just a switch(pokemon.types.get(0)) { ... }.

For that reasons, I'd make something like this for types data model:

public enum Type {
  NORMAL(R.string.type_normal),
  GRASS(R.string.type_grass),
  ...;

  public String name;

  public Type(@StringRes int typeName) {
    name = Resources.getSystem().getString(typeName);
  }
}

(We could also implement type weakness and strenght in that model)
And, in Pokemon model:

  // All other pokemon attributes...
  public ImmutableList<Type> types;

And, to instantiate that:

pokemon.types = Collections.unmodifiableList(Arrays.asList(Type.NORMAL, Type, GRASS));

The first type should be added first so that the list will preserve the types order (types(0) -> first type while types(1) -> second type).

The above model would be initialized statically with all the species instances at startup.
This suggested model could be much more flexible.

Anyway, @udnp your implementation is very well done. Thank you!

[udnp]

Thanks for your comment and advice.

Hey, I currently dont have a clear understanding of what we can use type information for.

I've tried thinking about some cases where these data is needed.

• judging some pokemons evolved from same one pokemon like Eevee (if not already provided)
  Poliwrath or Politoed, from Poliwhirl
  Vileplume or Bellossom, from Gloom
• for battle info, providing pokemon type weakness and strength, like @thearaks says.

That said, having type information implemented with a base20 string/integer is a bit too much obscure to debug and,
in general, to handle inside at code level: you can't do something like pokemon.types.contains(Type.FAIRY) or just a switch(pokemon.types.get(0)) { ... }.

I'm glad for getting your advice like this thanks, and I agree your thoughts.

In this PR, actually I've thought that it is important for codes I changed to be not-large-scale and less bad-influence.
(because I'm cooperating from outside of GoIV project. 😃)

For that reasons, I'd make something like this for types data model:

OK, I have a question.
Do you think it would be better to accept this PR once?
Or I should be better to close this without merged and wait until @thearaks 's thoughts are implemented on future GoIV?
Both judgements are acceptable for me.

If this PR suggestion would be acceptable,
I'll do to fill all types values instead of dummy values and update this PR.
Do you have any other questions or comments?

[nahojjjen]

It'd probably be best to make the alterations thearaks suggested before merging.

[udnp]

It'd probably be best to make the alterations thearaks suggested before merging.

@nahojjjen, OK. I created a new PR #883 for minor improvement picked up from this PR.

Before the alterations @thearaks suggested made, I hope the minor improvement #883 would be merged.😃
Because I mistook the order of pokemon types in my previous PR...

Would you check PR #883?

[celandro]

I would suggest using compiled protos. See https://github.com/celandro/pogoprotos-java

Keeping your own enums is going to be painful and buggy.

[udnp]

@celandro, Thanks for your advice.
I agree that keeping your own enums is going to be painful and buggy.

I've not know POGOProtos, pogoprotos-java and game master data.
Now my understanding is following:

• POGOProtos is data model analized from PokemonGo app and network, including Pokemon name, type, and so on. For instance available from gamemaster.
• pogoprotos-java is one way to use POGOProtos with Java.
• Therefore, pogoprotos-java can be enough for implementing @thearaks 's suggestion above.

Actually, I implemented types.xml including own enums based on the way of using integers.xml.

So I have a question to GoIV pj team.
Is the integers.xml data currently generated automatically from other data like POGOProtos by tools or anything?
Or checked manualy?