search

ajhyndman / fire-emblem-working-title

A fledgling project wrangling Fire Emblem: Heroes stats
25 stars 6 forks source link

Consider adding the `poolDate` attribute #118

Closed carsomyr closed 6 years ago

carsomyr commented 6 years ago

First of all, thank you for this great project! I've been using it to generate an inventory tracking spreadsheet of heroes and skills. Have you considered including the poolDate attribute in stats.json? This is useful for summoning pool conditional probability calculations when a new banner releases.

P.S. - I don't know what's up with the date format changes is there some JS dependency I forgot to update?

ajhyndman commented 6 years ago

Hey, I'm glad to hear you've been finding the project useful!

I have no problem with adding the poolDate. Sounds like a good idea!

I'd guess the date format difference might be a result of different node versions? I'm running node v8.1.4 locally. Otherwise, it might be OS (I'm running a Mac) or else the locale setting on our machines? Either way, I'm happy to accept this PR, and rebuild stats.json afterward!

ajhyndman commented 6 years ago

Hmm, I'll have to look into why the Circle CI build isn't starting on public Pull Requests. I'll go ahead and override & merge here.

carsomyr commented 6 years ago

@ajhyndman Thanks! What happens when you run npm run scrape-stats? If it causes date format thrashing, I'll investigate further.

ajhyndman commented 6 years ago

Yeah, it has reverted all the date formatting.

https://github.com/ajhyndman/fire-emblem-working-title/commit/62beaeb529bbd71675ebb02f13c8ebe1d1912039

ajhyndman commented 6 years ago

I've published a new release to npm as well, in case you're using that!

carsomyr commented 6 years ago

@ajhyndman Node v8.9.1 here. I ran lerna clean and then lerna bootstrap. Does this guarantee that outdated package dependencies are replaced with the newest versions? Node is not my primary platform.

ajhyndman commented 6 years ago

I've set up the project to be compatible with "yarn workspaces", which should mean that just running yarn install in the root folder will update and rebuild everything.

I think lerna is workspaces aware now, so it's expected to have the same behaviour.

ajhyndman commented 6 years ago

Upgrading node doesn't change the date formats I get, so I guess that's not it after all.

I am running macOS, with my language set to English (Australia). Maybe one of those things is giving us trouble?

Actually, one of my preset date formats does look like the one that's added to my stats.json file. Maybe my OS settings are leaking into the output. I wonder if there's a way to guard against that.

image

I've opened another issue to investigate:

https://github.com/ajhyndman/fire-emblem-working-title/issues/121