nrkn / pvpoke-node

MIT License
6 stars 6 forks source link

Future, roadmap etc #2

Open nrkn opened 4 years ago

nrkn commented 4 years ago

@wasabigeek sorry I took two weeks to get back to you! Also cc'ing @alexameen here as they also indicated some interest

The idea is to build a collection of functions that allow you to do all the same things that pvpoke can do, and more, because if it's just a collection of functions you can combine them in new and interesting ways that eg pvpoke author hadn't thought of, as opposed to a handful of monolithic classes that hide most of the underlying functionality

So there is a sandbox folder, src/sandbox

This is for running scripts against the actual library to test what it can do so far, play with things etc

A good start would be to review what is there so far to get an idea for the style etc. and then pick something small and simple that pvpoke can do, add a script to the sandbox that does that thing, logging results to console, then port over some code from pvpoke is currently missing from this library (probably most of it!), just enough to do that thing

At some point when it's a little more complete, we need tests - some of the stuff in sandbox can form a starting point for these

Also, this library is now quite far behind pvpoke, so we would need to bring it up to date - there are some tools for converting the pvpoke json files to the format used by the library in sandbox

I'm currently swamped with work and haven't thought about this at all for some time, but I suspect that if I'm getting some help with this project, even if only a very small amount, that I'd be motivated to start working on it again!

aakropotkin commented 4 years ago

You're gonna hate me but i almost want to write this in Haxe or something so we can easily transpile to C, JS, TS, Java, Python, etc.

The simulation is simple enough that its not really a pain.

nrkn commented 4 years ago

You're gonna hate me but i almost want to write this in Haxe or something so we can easily transpile to C, JS, TS, Java, Python, etc.

The simulation is simple enough that its not really a pain.

If that's something you want to do, by all means! I'm not interested in haxe though personally, so wouldn't want to be involved.

aakropotkin commented 4 years ago

Ill just roll with whatever y'all want to do. Honestly I'm not picky as far as languages go

aakropotkin commented 4 years ago

@nrkn if you could add us as collaborators we could start filling out a checklist on the projects board. I have found that on these kind of projects it can help keep us from finishing tasks redundantly.

What was the toolkit you were talking about for converting the JSON files? I'd be happy to do that conversion, and learn about how the sandbox data differs from the upstream PvPoke data files.

From what I can tell you sliced the root JSON keys into separate files. Were there any other major differences?

aakropotkin commented 4 years ago

Having added slice-upstream-gm should I go ahead and remove src/data and update the paths from dist/data/pokedex.json to dist/data/pokemon.json? Alternatively I can manually rename the file.

nrkn commented 4 years ago

@alexameen yep good idea - have added you and @wasabigeek - also @simonh1000, who has also indicated interest.

The toolkit that translate from pvpoke to internal structure consists of:

I can't remember what the exact changes were but there are typings for the internal format under src/entities/ then pokedex and moves

I think how it currently works is, that I manually split the pvpoke data and put in in /src/sandbox/data

The transform-etc.ts files reformat them to fit internal and output to console, and I just piped the output to the relevant folder I think

I think the data needs to go into src/data rather than dist/data as I am using typescript json imports?

Sorry, I haven't touched any of this for a while!

nrkn commented 4 years ago

I think changes to the structure are around making some things be more explicit and less implicit?

Like instead of having some fields be optional, a default value is always provided so you don't constantly have to test if it exists while using the data downstream.

aakropotkin commented 4 years ago

Yeah the transforms you have now seem like they remove [ "defaultIVs", "level25CP", "eliteMoves" ] from pokemon, and leaves moves unchanged.

You can find this by:

# Has to be run with ZSH, and requires JQ
rm -rf ./dist/data;
npm run build;
function get_keys() { cat ${1}|jq -r 'map( keys )|flatten|unique|.[]'; }
declare -a {our,their}_{move,pokemon}_keys;
our_move_keys=( `get_keys dist/data/moves.json` );
our_pokemon_keys=( `get_keys dist/data/pokedex.json` );
node ./dist/sandbox/slice-upstream-gm;
their_move_keys=( `get_keys dist/data/moves.json` );
their_pokemon_keys=( `get_keys dist/data/pokemon.json` );
print We remove \'${their_move_keys:|our_move_keys}\' from moves;
print We remove \'${their_pokemon_keys:|our_pokemon_keys}\' from pokemon;
wasabigeek commented 4 years ago

Would love to help contribute. I'm not super familiar with the PVPoke codebase though, assigning a small task would be helpful for me to focus :)

simonh1000 commented 4 years ago

No idea about timezones but know I would welcome chance to speak by zoom to learn properly what has been done and would could be added

nrkn commented 4 years ago

What I'd like to do next is write a checklist of all the functions and datatypes you need to do things that pvpoke can do, showing which have been implemented and where they are located within this project's source, and also annotate them with their location in the pvpoke source, so that it's easy to keep this in sync with changes to pvpoke.

This would give us a great roadmap and allow us to start choosing things to work on - I'm just a bit slammed with work at the moment since we came out of lockdown, so haven't had the opportunity yet.

aakropotkin commented 4 years ago

@nrkn Could you elaborate on the issue you were having with compare-default-ivs? I don't think I really understood the purpose of the test.

What is meant by "legacy" pokedex? Does "legacy" refer to the legacy pokedex data ( ex old Snorlax ), or is "legacy" referring to PvPoke's default IVs?

A good place to start might be properly calculating optimal IVs. What we have now appears to work as far as I can tell; but the compare-default-ivs example seems to illustrate that it's broken? ( Maybe I misunderstood ) On the surface this appears to be a simple maximization problem, but doing it with loops rather than matrix multiplication is an exponential performance difference.

I looked at how PvPoke calculates it, and we should NOT repeat their implementation. Fast Fourier Optimization can improve runtime from N^3 to N * log_3 N.

While caching the data makes the performance a sort of non-issue, the speed bump could allow us to iteratively improve "optimal" IVs from simulations ( using max stats as a base, and using breakpoints to identify which IVs win most ).

nrkn commented 4 years ago

@nrkn Could you elaborate on the issue you were having with compare-default-ivs? I don't think I really understood the purpose of the test.

I believe I was trying to get something up and running that I could run both here and on pvpoke and compare the results

What is meant by "legacy" pokedex? Does "legacy" refer to the legacy pokedex data ( ex old Snorlax ), or is "legacy" referring to PvPoke's default IVs?

Legacy pokedex data

A good place to start might be properly calculating optimal IVs. What we have now appears to work as far as I can tell; but the compare-default-ivs example seems to illustrate that it's broken? ( Maybe I misunderstood ) On the surface this appears to be a simple maximization problem, but doing it with loops rather than matrix multiplication is an exponential performance difference.

I looked at how PvPoke calculates it, and we should NOT repeat their implementation. Fast Fourier Optimization can improve runtime from N^3 to N * log_3 N.

While caching the data makes the performance a sort of non-issue, the speed bump could allow us to iteratively improve "optimal" IVs from simulations ( using max stats as a base, and using breakpoints to identify which IVs win most ).

That sounds great - I hadn't considered using matrices despite using them for a bunch of image processing code recently!

nrkn commented 4 years ago

I just pushed implementation.md - contains an overview of what is in the pvpoke source and where

Next, I'll be breaking down the contents of each of the files and what the various methods etc do, and which ones have been implemented already - have to get back to work now though

nrkn commented 4 years ago

I've updated implementation.md

It now contains information about the pvpoke raw data, and how it is mapped for our use internally, as well as information about some functions for working with it

Next - document methods!

nrkn commented 4 years ago

I used the work done by @alexameen to create a build step that pulls the upstream data and transforms it to the internal data types

Pull request is https://github.com/nrkn/pvpoke-node/pull/6

nrkn commented 4 years ago

Just some observations for consideration and discussion:

Rankings is blocked by battle not being implemented Battle is blocked by AI not being implemented

The AI implementation is here and has a dataset associated with it:

https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/js/training/TrainingAI.js

This is the dataset:

https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/data/training/aiArchetypes.json

aakropotkin commented 4 years ago

Just some observations for consideration and discussion:

Rankings is blocked by battle not being implemented Battle is blocked by AI not being implemented

The AI implementation is here and has a dataset associated with it:

https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/js/training/TrainingAI.js

This is the dataset:

https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/data/training/aiArchetypes.json

I have this partially outlined. So far I have added a bunch of types, and skeletons for all of the functions. I can send what I have or wait until it's fully implemented. Whichever you prefer.

nrkn commented 4 years ago

Just some observations for consideration and discussion: Rankings is blocked by battle not being implemented Battle is blocked by AI not being implemented The AI implementation is here and has a dataset associated with it: https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/js/training/TrainingAI.js This is the dataset: https://github.com/pvpoke/pvpoke/blob/f6d1dfd3a2b93a624a69f573a3dda7bdedc87f22/src/data/training/aiArchetypes.json

I have this partially outlined. So far I have added a bunch of types, and skeletons for all of the functions. I can send what I have or wait until it's fully implemented. Whichever you prefer.

I had a look at your branch - it looks really good!

We can do a pull now if you like

But we should talk about TypeScript numeric enums - we stopped using them at work for a number of reasons:

  1. type safety - typescript will let you pass a function that has a numeric enum arg any number, not just those in range for the enum
  2. transpiler output is clunky (doesn't bother me but bothers my colleagues!)
  3. verbosity

Consider:

const dispatchAction = ( action: TimelineAction ) => console.log( action )

dispatchAction( 10 ) // tsc is fine with this!

If you do a quick search you can all sorts of other complaints too - and there are workarounds, but we found using string literal unions to be more concise and in many ways safer

They're very similar to using string enums (which we could consider if you really want enums - solves 1. and 2. and partly 3.):

enum TimelineAction { FAST = 'fast', CHARGED = 'charged', WAIT = 'wait' }

There is an even better pattern that we use at work that I'm not currently using in this project which we could consider:

// typescript will mark as readonly and tsc won't let you alter
const actions = ['fast', 'charged', 'wait'] as const 

// same as 'fast' | 'charged' | 'wait' except `actions` is the sole source of truth
type TimelineAction = typeof actions[number] 
aakropotkin commented 4 years ago

Hey so a few months back I was running a fork of PvPoke that I was hacking around on. I realized that on full rankings the limiting factor on the system was JavaScript's memory manager. I cleaned up things here and there in the codebase to eliminate waste; but I realized that ultimately JavaScript was just not meant for crunching datasets of that size. That was around the time that I stopped pushing to this, because I assumed that by extension, TypeScript was going to hit the exact same wall.

So. I re-implemented PvPoke from scratch in C. It's still a work in progress, but I just got the simulator with a naive AI running a few days ago. The vast majority of the work was in parsing the GM file and data processing. With that out of the way there is now a fairly simple interface for swapping in different AIs, and data sources.

I thought that some of you might be interested in checking it out.

I know that C isn't everyone's favorite language, so you might be happy to hear that I spent the last few days creating Python Bindings for the API. This will essentially let you hack around in Python and run battles, that are wrapped around the optimized C core.

https://github.com/alexameen/cpoke

nrkn commented 4 years ago

So. I re-implemented PvPoke from scratch in C.

Wow, that's really cool! I just got back into C again this year, but only briefly before life and work overtook me

I know that C isn't everyone's favorite language, so you might be happy to hear that I spent the last few days creating Python Bindings for the API.

How about a node binding? :)

aakropotkin commented 4 years ago

Can Node use shared objects?