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 encoding and decoding of item details (quantity/skin/upgrades) #2

Merged
merged 3 commits into from
Jun 25, 2017

Conversation

darthmaim
Copy link
Contributor

encode now takes an object ({id, quantity?, skin?, upgrade1?, upgrade2?}) as second argument to pass the additional item info. Just passing the id still works.

decode will now always return quantity for items, and optional skin, upgrade1 or upgrade2.

`encode` now takes an object (`{id, quantity?, skin?, upgrade1?, upgrade2?}`) as second argument to pass the additional item info. Just passing the id still works.

`decode` will now always return `quantity` for items, and optional `skin`, `upgrade1` or `upgrade2`.
@darthmaim
Copy link
Contributor Author

Now that I commited this I noticed it would be nice if the two upgrade slots were passed/returned as array ({upgrades: [1, 2]} instead of the 2 properties ({upgrade1: 1, upgrade2: 2}) to allow passing a ItemStack from the Api.

@codecov-io
Copy link

codecov-io commented Jun 25, 2017

Codecov Report

Merging #2 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #2   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          31     71   +40     
=====================================
+ Hits           31     71   +40
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7921c8...4ace663. Read the comment docs.

@queicherius
Copy link
Member

First off: Thank you! 💓

I agree that an array of upgrades would be more intuitive, then it mirrors the API. Gonna wait for that here.

Out if interest, is it possible to encode infusions too? I remember them not showing up in the game a while ago, but I have no idea how it is handled now.

If you have the motivation, you could also update the README for the new input format, if not that's cool too and I'll do it after the merge.

@darthmaim
Copy link
Contributor Author

You can't encode Infusions :(

Will update the README too.

@darthmaim
Copy link
Contributor Author

Should be good now 👍

@queicherius
Copy link
Member

Nice! 👍 LGTM.

@queicherius queicherius merged commit 3e41b46 into gw2efficiency:master Jun 25, 2017
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.

3 participants