Towards a better Dex API

[Zarel]

Our current Dex API revolves around:

Dex.getMove(moveName)
Dex.getAbility(abilityName)
Dex.getItem(itemName)
Dex.getSpecies(speciesName)

Everything else is sort of just haphazardly squeezed in there.

The most noticeably missing feature is the ability to iterate through a list of all moves, or all abilities. We currently achieve this with for (const moveid in Dex.data.Moves), but this is not really intended to be an officially supported API, so much as a workaround. It also doesn't work in the client, which has a much older API for const moveid in BattleMovedex).

I've been meaning to standardize iteration APIs. Some time ago, I polled developers on the two options:

1. Dex.getMove(moveName) and Dex.allMoves()
2. Dex.moves.get(moveName) and Dex.moves.all()

Most people preferred the latter, and it certainly makes the code easier, but I figured I'd post one more issue declaring my intent, and looking for any final objections, before actually moving forward with it.

There's still one fear, which is that it won't be very consistent. We'll still have plenty of APIs which use getWhatever() functions. But that's probably pretty minor in the grand scheme of things.

Anyway. The plan is to rename Dex.getMove to Dex.moves.get (and so on for the others). Questions, comments?

[Zarel]

A related question is that we probably want four functions: one for a loose match, one for an exact ID match (for performance), one for iterating legal entries, and one for iterating all entries.

The one I'd default to:

Dex.moves.get(name), Dex.moves.getByID(id), Dex.moves.all(), Dex.moves.all(true)

Other options include:

Dex.moves.search(name), Dex.moves.get(id), Dex.moves.allLegal(), Dex.moves.all()

get(id) is a weird one: we use plenty of Maps where get needs to be an ID, but in most of the rest of our code, get usually takes a name. But there's nothing wrong with our API being more Map-like.

[Zarel]

You might ask: Can we implement it so for (const move of Dex.moves) works?

The answer is: yes, but for performance reasons, it wouldn't be usable on client. I would rather an API that works the same way on client and server than one that's slightly nicer but only works on the server.

One API that would work on the client is for (const move of Dex.moves()). This... would complicate the implementation code a lot, and also might be confusing. I definitely got pressured into renaming Users(name) into Users.get(name), so I assume that's the opposite direction of where people want to go.

[AnnikaCodes]

I prefer Dex.moves.search() and Dex.moves.get() to get() and getByID(). When I hear get, I don't assume that it'll get a loose match (I assume this is for the spellcheck on /ds and similar commands) and I would expect Dex.moves.get('Ari Slash') to not find a move. search conveys that we're looking for the best match, not necessarily an exact one. (get vs getByID makes sound like the former expects move names and the latter move IDs.)

[Zarel]

I prefer Dex.moves.search() and Dex.moves.get() to get() and getByID(). When I hear get, I don't assume that it'll get a loose match (I assume this is for the spellcheck on /ds and similar commands) and I would expect Dex.moves.get('Ari Slash') to not find a move. search conveys that we're looking for the best match, not necessarily an exact one. (get vs getByID makes sound like the former expects move names and the latter move IDs.)

It's not for spellcheck, it's for aliases and names. Dex.moves.search('Ari Slash') will not find a move.

[AnnikaCodes]

Oh, then I'm fine with get().

[Zarel]

Also perhaps worth considering: Dex.moves.moves() instead of Dex.moves.all(). Or maybe Dex.moves.list().

[Zarel]

I ended up just going with an unfiltered .all(). There's simply too many different filters needed by different use-cases; after discussion with other devs, I simply couldn't find a single filter with more than one use-case.

Possible desirable filters are documented in:

https://github.com/smogon/pokemon-showdown/blob/master/sim/NONSTANDARD.md