[Enhancement] improve typing #2501
Conversation
src/data/pokemon-species.ts
Outdated
| @@ -528,7 +528,7 @@ export abstract class PokemonSpeciesForm { | |||
| } | |||
| } | |||
|
|
|||
| const pixelColors = []; | |||
| const pixelColors: integer[] = []; | |||
There was a problem hiding this comment.
Typescript doesn't have an integer type can you use number instead?
There was a problem hiding this comment.
I found there are many integer in the codebase:
pokerogue/src/data/pokemon-species.ts
Line 533 in edb4c7e
which is type alias of number from phaser package.
// nodue_modules/phaser/types/phaser.d.ts
declare type integer = number;So I thought it would be better for consistency if convention is to use integer over number when the value is integer case.
WDYT?
There was a problem hiding this comment.
Sure for the phaser stuff we can keep it consistent and use integers
src/field/pokemon.ts
Outdated
| for (let e = 0; e < evolutionChain.length; e++) { | ||
| // TODO: Might need to pass specific form index in simulated evolution chain | ||
| const speciesLevelMoves = getPokemonSpeciesForm(evolutionChain[e][0] as Species, this.formIndex).getLevelMoves(); | ||
| const speciesLevelMoves = getPokemonSpeciesForm(evolutionChain[e][0], this.formIndex).getLevelMoves(); | ||
| levelMoves.push(...speciesLevelMoves.filter(lm => (includeEvolutionMoves && !lm[0]) || ((!e || lm[0] > 1) && (e === evolutionChain.length - 1 || lm[0] <= evolutionChain[e + 1][1])))); |
There was a problem hiding this comment.
Can you destructure here so these loops are easier to read.
const [ species, level ] = evolutionChain[e]speciesLevelMoves.filter(([ level, move ]) => { ... }
There was a problem hiding this comment.
This might be worth doing inside of a separate PR instead of this one.
There was a problem hiding this comment.
will do in next PR !
src/data/pokemon-evolutions.ts
Outdated
| export type EvolutionLevel = [Species, integer]; | ||
|
|
There was a problem hiding this comment.
Can you add jsdoc comments that explain what this type means. For example
/**
* Pokemon evolution tuple type
* @property 0 - Pokemon Species
* @property 1 - The level at which it evolves
*/There was a problem hiding this comment.
I added jsdoc in the way you suggest and changed it to a named tuple:
I wonder what is the best way to document tuple elements with jsdoc though. It doesn't seem well supported...
There was a problem hiding this comment.
Yeah I tried looking it up but I couldn't find anything either. This is the best that I could come up with but if you find better by all means try it.
|
@2wheeh can you resolve the conflicts so this can be merged |
|
@OrangeRed done ! sorry for the late iterate. |
|
@2wheeh please fix merge conflicts (again) |
|
@torranx fixed. |
What are the changes?
It's minor type improvement.
Why am I doing these changes?
It's better for type safety.
What did change?
Screenshots/Videos
How to test the changes?
Checklist
npm run test)[ ] Are the changes visual?[ ] Have I provided screenshots/videos of the changes?