Interest in collaborating on import/export set parser?

ajhyndman commented 4 years ago

Hey! I realised the parser for importing and exporting sets in this package wasn't very portable, so I went through the exercise of writing some code myself.

https://github.com/ajhyndman/visual-pokemon-calc/blob/master/src/export.ts

It looks like there's some interest in improving the capabilities here as well (i.e. #335), so I just wanted to offer to submit something similar as a pull request. Are you interested?

scheibo commented 4 years ago

We don't really want to implement our own import/export logic ourselves and would like to be sharing it with PS so that we can automatically be up to date (and support legacy imports) when the format changes. I'd accept a PR which just rips out the current existing PS import/export logic from the client code (which is somewhat non trivial because it depends PS's data files), but I'm not sure about the solution youve linked (the client isn't using regexes... and because your solution doesnt have PS's data files its actually incorrectly parsing ability, for example).

ajhyndman commented 4 years ago

Yeah, one standard implementation of import/export sounds ideal!

Is the pokemon showdown code published somewhere? I looked briefly, but didn't find it.

scheibo commented 4 years ago

Is the pokemon showdown code published somewhere? I looked briefly, but didn't find it.

https://github.com/smogon/pokemon-showdown-client/blob/master/js/storage.js#L1083 is the code for importing (exporting is below that). https://github.com/pkmn-archive/dmg/blob/master/import.ts is where I extracted this in the past, but didn't port it over when I created the @smogon/calc package because I'm fairly certain sharing is a better long term approach than copying.

To import a PS set with 100% compatibility, among other things you must:

handle the same default Hidden Power IVs/DVs
handle special happiness logic;
support Trait: in addition to Ability:

I misremembered needing the PS data files to parse Ability, that's actually strictly only required for legacy unpackTeam (it used to always be required, but I changed it so that going forward it isn't needed)

https://pkmn.cc/modular-ps contains a high level overview of the modularization effort

:)

ajhyndman commented 4 years ago

I got some time to read through the roadmap doc today, and it looks amazing!

I do want to make a point of saying that working with this repository has been one of the most pleasant open source experiences I've had. I certainly wouldn't have started experimenting with my own project if hadn't been here already. So thanks for all the work!

I'm curious how far along the journey in that roadmap you are. I could probably poke around a few more repositories and issues, though, to see what I find.

I'll close this issue, it seems like the high level objective is already being tracked.

scheibo commented 4 years ago

I got some time to read through the roadmap doc today, and it looks amazing!

I do want to make a point of saying that working with this repository has been one of the most pleasant open source experiences I've had. I certainly wouldn't have started experimenting with my own project if hadn't been here already. So thanks for all the work!

Thanks!

I'm curious how far along the journey in that roadmap you are. I could probably poke around a few more repositories and issues, though, to see what I find.

Hmm... I havent really written down anywhere what the current state of all of this is, but because you asked:

calc is split out, though doesn't depend on data yet
data is technically working and is powering https://smogon.com/dex so its technically 'production ready' but still very much WIP to make ergonomic and generally useful etc and is very much the hardest component because it needs to meet a lot of requirements given how ubiquitous it is: https://github.com/smogon/data. there a design doc at https://pkmn.cc/ps-core-design
stats is finished but not published as the larger logs processing pipeline support is only 95% complete (https://github.com/pkmn/stats) - itll be the smogon org when its complete and being used in prod
client is actually something a spent i ton of time on recently and have it pretty much done in a branch i hope to get reviewed and published later this week. it actually subsumes view for the most part, so that package likely wont exist. there will likely be a ui package though where smogon projects can share common UI Preact components as we seek to redesign to a more shared aesthetic/brand
theres a new protocol package i've written (https://github.com/pkmn/protocol) which wasnt in the original doc but has proven desirable. it needs further testing but will also likely be out for review this week
lib is waiting on Zarel to publish
sim/random/team will probably be split together and were originally going to be blocked on data, but if we dont actually block on that and instead refactor all the packages to use data once it ships my approach with the forthcoming client package of generating the package instead of restructuring the code means these could be broken out soon as well

scheibo commented 4 years ago

@ajhyndman - https://github.com/pkmn/ps might interest you on this front. @pkmn/sim is working but not published, it will be published with @pkmn/protocol / @pkmn/client / @pkmn/sim hopefully by next week.

More relevant to your interests and the question posed in the bug: the set + team import/export/pack/unpack has just been extracted, modified to be data layer agnostic, tested and published: https://www.npmjs.com/package/@pkmn/sets.

ajhyndman commented 4 years ago

Very cool! Thank you for all the heads ups.

I feel like I hit my first milestone with my app too, this week. It's still missing a few key things, but I've been able to use it successfully in battles for myself!

smogon / damage-calc

Interest in collaborating on import/export set parser? #378