Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add set import logic #346

Merged
merged 4 commits into from Aug 25, 2019
Merged

Add set import logic #346

merged 4 commits into from Aug 25, 2019

Conversation

scheibo
Copy link
Contributor

@scheibo scheibo commented Aug 23, 2019

This is currently working modulo Gen 2 NU. Some notes:

  • in addition to pulling sets from Smogon I've also added logic for pulling sets from third party sources
  • 'legacy' mode results in pretty much the same output as @Honko's import application, with some noticeable differences:
    • Smogon's "No Item" gets intrepreted as ""
    • Level from Smogon sets gets preserved (eg. 1,2,5,99 etc)
    • Darmanitan-Z and Meloetta-P get written with the correct names
  • going forward, the sets format will be keyed first by format in order to facilitate the dropdown logic discussed on Update sets and commit set update script #343. The JSON sets will also be pretty printed with whitespace in the repo because Parcel will handle minimizing it for us.
  • 'weights' are intended to be used to modify the sort order of dropdowns, coupled with information already existing in sets (options that appear in sets will be sorted first by usage, then the remaining options will be sorted by global usage for the format). This logic will probably only exist in the new UI.
  • bugs in 'Heracrossite' and 'Crucibellite' were corrected, but missing.json contains all the IDs we're missing (needed for name lookup given usage stats currently report ids in most places in the chaos reports). Once we close Depend on Pokémon Showdown's data files #334, missing.json and the logic around it can be deleted.

All told, running this takes ~3m30s-3m45s on my machine. I'd imagine we'd probably want to run it at least once a month after @Marty-D does the usage stats updates (or if C&C staff do any large cleanups/uploads)

The most generic logic (mostly type definitions) for importing smogon analyses/stats are in a smogon package (deliberately zero dep) for use in other projects, though we may want to share set/weight import logic with the Pokemon-Showdown-Client repo's node build full process if we end up using these in the teambuilder/box system (specifically, creating a package that has a node-fetch dependency)

sets/src/index.ts Outdated Show resolved Hide resolved

function filter(format: Format, pokemon: string, set: PokemonSet) {
const hasMove = (m: string) => set.moves && set.moves.includes(m);
if (format === 'BH') return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If my understanding of BH bans is correct, I think this is technically incorrect because Groudon-Primal is banned but Groudon holding Red Orb to transform into Groudon-Primal is allowed. The same might be true for Rayquaza-Mega. You should probably check this with TI.

It doesn't affect anything at the moment because neither of them has an analysis for SM, but it's a potential problem if one gets added later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find anything about that on Smogon, but maybe I'm misreading or looking in the wrong places. @TheImmortal for the official ruling :)

@Honko - does your application handle BH M-Ray/P-Don? What does your condition here look like?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mine still has a check for Gen6 BH to handle P-Don and P-Kyogre, but that condition never applies anymore since I stopped importing Gen6 BH sets once it was no longer the current gen. I never added a new check for Gen7 BH, probably because the bans happened after I had stopped really paying attention.

    private static boolean isValidSetForAlt(
            final Moveset moveset,
            final String altName,
            final Metagame metagame) {
        // everything is legal in BH except Primal Kyogre/Groudon without appropriate Orb in XY
        if (metagame.getTier() == BH) {
            return !(metagame.getGeneration() == XY
                    && altName.contains("Primal")
                    && !(moveset.getItems().contains("Red Orb") || moveset.getItems().contains("Blue Orb")));
        }
        // ...
        // all the other forme checks you already have
        // ...
    }

Choose a reason for hiding this comment

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

I can't find anything about that on Smogon, but maybe I'm misreading or looking in the wrong places. @TheImmortal for the official ruling :)

Groudon-Primal and Rayquaza-Mega in their raw formes are banned. Base Groudon with Red Orb and Rayquaza with Dragon Ascent are legal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scheibo scheibo merged commit b6d1eb2 into smogon:master Aug 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Depend on Pokémon Showdown's data files
3 participants