Skip to content

Support Champions - #19

Closed
thejetou wants to merge 2 commits into
pkmn:mainfrom
thejetou:champions
Closed

thejetou wants to merge 2 commits into
pkmn:mainfrom
thejetou:champions

Conversation

@thejetou

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread pkmn/index.ts Outdated
let data = this.cache.gen[type][gen.num];
if (!data) {
if (this.minimal) {
if (isChampions) format = 'champions' + format;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgive me, but why can't a user just pass champions as part of the format?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had this kind of backwards. I didn't internalize that we pass in gen8ou and not just ou so I switched it to championsou now meaning we can use that to figure out isChampions instead of the user having to pass it in always (they will still need to pass it in if there's no format).

I also fixed a bug that had champions and the regulations for the VGC formats in the master champions.json file and format file names, made baseFormat handle champions VGC / BSS formats, added more tests, fixed a caching bug (champions formats now cache to 0).

For the stats function it expects champions<format> (like the analyses & sets functions) but on the Smogon disk they are gen9champions<format> (equiv to the PS format name) if you want me to switch to match that format lmk.

- Users passes in `champions` now and can elide in the arguments
  when passing in a format
- Fix caching bugs (cache champions to `0`)
- Base format handles champions VGC / BSS formats
- Add more tests
Comment thread pkmn/index.ts
@@ -227,7 +227,8 @@ export class Smogon {
* Returns Analysis objects for the given Pokémon species and gen, optionally scoped to a
* particular format.
*/
async analyses(gen: Generation, species: string | Specie, format?: ID) {
async analyses(gen: Generation, species: string | Specie, format?: ID, isChampions = false) {
isChampions = isChampions || !!format?.startsWith('champions');

@scheibo scheibo Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is isChampions still needed as a param? answered on discord also, this param should be mentioned in the doc comment if it is necessary (ie/ "scoped to a particular format or champions"). as a final nit, i prefer just champions even if its a boolean

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, im wondering if we should just make the format param format | 'champions'? avoids the extra arg, can still just check startsWith('champions') to detect champions. need to do a literal string check on the format arg and it also is effectively using an inband sentinel instead of another arg, but might be cleaner?

Comment thread update/sets.mjs
@@ -102,12 +104,13 @@ function eligible(gen, species) {

(async () => {
const index = {analyses: {}, sets: {}};
for (let gen = 1; gen <= 9; gen++) {
const dex = Dex.forGen(gen);
for (let gen = 0; gen <= 9; gen++) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this should just be i or whatever and psGen should be renamed gen

Comment thread smogon/index.ts
@@ -208,7 +208,7 @@ export const Analyses = new (class {
* Returns Smogon's display representation of the given gen.
*/
gen(gen: GenerationNum) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wont work, it needs to be 0 | GenerationNum, no? in which case it maybe should just be 'champions' | GenerationNum probably?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually just pretty sure we shouldnt be calling this function with champions (or 0) at all and should just handle champions at a higher level

Comment thread update/sets.mjs
await serialize(data.gen.analyses, `analyses/gen${gen}.json`);
index.sets[`gen${gen}.json`] =
await serialize(data.gen.sets, `sets/gen${gen}.json`);
const genFname = gen === 0 ? `champions.json` : `gen${gen}.json`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genFname -> file? I like terser names :)

scheibo added a commit that referenced this pull request Aug 28, 2026
closes #19

Co-authored-by: Waleed Hassan <waleed.hassan.aidid@gmail.com>
@scheibo scheibo closed this in 8abc70b Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants