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 /v2/achievements and /v2/achievements/daily. #96

Merged
merged 2 commits into from
Nov 9, 2015
Merged

Conversation

lye
Copy link
Contributor

@lye lye commented Sep 21, 2015

This is a first-pass -- for the initial release /v2/achievements will only be populated with the current daily achievements. I need to go through the entire list and make sure there's nothing that shouldn't be exposed before we can enable it for all the data.

We'll be extending the achievement endpoint a bit more when we release /v2/account/achievements so that for achievements that use a bitset internally for progress (e.g., Dungeon Master) you can reference what the individual bits mean (since /v2/account/achievements will just give you the completed bit indexes). That's kind of a longer-term change since it requires changes to the actual underlying content -- we have new support to display strings for the individual achievement steps in-game now.

Anyway, let me know if anything looks terribly off with the structure.

@ghost
Copy link

ghost commented Oct 8, 2015

It runs but for PvE there r no icon and no flags.
look here
http://test.teufelspack.de/achievements

@Coffee4cr
Copy link

#99 as I posted here, they don't seem to show the proper dailies either, when you compare them to
https://wiki.guildwars2.com/wiki/Daily/Table which also seems 1 day off.

@ghost
Copy link

ghost commented Oct 8, 2015

Ingame checked, does not provide the current, at least not in Europe

@Coffee4cr
Copy link

Yeah I don't think it's different on NA/EU

I know last night when i did my dailies, that Ruins was one of them, and ascalon lumberer

@ghost
Copy link

ghost commented Oct 8, 2015

Maybe not different, but the time lag could be a problem

@lye
Copy link
Contributor Author

lye commented Oct 8, 2015

I'm guessing there's time lag between NA/EU, since their daily resets are at different times. Totally forgot about that; unfortunately won't be able to fix that until some time after HoT ;_;

They have different reset times, so they should be treated as two separate
sets.
@lye
Copy link
Contributor Author

lye commented Oct 8, 2015

Updated the PR with my proposed fix for the NA/EU split. Note that this is a breaking change (but I can't figure out a cleaner way to do it).

@atomicptr
Copy link

Any ETA on when the breaking change comes live?

@lye
Copy link
Contributor Author

lye commented Oct 27, 2015

Not for another couple of weeks -- putting out a couple dozen small fires right now and the release window for making changes to that component is looking kind of gross. Sorry for the delay :(

One change that might be reasonable is to have /v2/achievements/daily continue returning the bad data for awhile (rather than ["na", "eu"]) and add in /v2/achievements/daily/{na,eu}. I'm not sure how useful that is, but it's an option so that you can write working applications now, then have a window (2-3+ weeks) to update them without breakage.

@atomicptr
Copy link

That would be really useful ;).

@RedGlow
Copy link
Contributor

RedGlow commented Nov 5, 2015

I'm wondering about the /v2/account/achievements. It's clear you already have it planned, so I won't make a pull request, but do we know when this feature may land? Given the increasing amount of achievement-based collections, it's getting more and more useful.

@lye
Copy link
Contributor Author

lye commented Nov 5, 2015

Hrm, actually, lemmie see if I can turn it on this week.

EDIT: Gonna have to take that back; I thought I had an implementation + pull request lying around, but I didn't write it yet. The backend support is all in but I'd like to have a PR up for a week or so so that I can get some eyes on it first. I'll have it up later today.

@themacguffinman
Copy link

Are fractal dailies going to be in /v2/achievements/daily?

Also, is the final endpoint going to include reward details?

@lye
Copy link
Contributor Author

lye commented Nov 6, 2015

Uhh, those are really good points. I should add both of those.

lye pushed a commit that referenced this pull request Nov 9, 2015
Add /v2/achievements and /v2/achievements/daily.
@lye lye merged commit f6c7c9b into master Nov 9, 2015
@lye
Copy link
Contributor Author

lye commented Nov 9, 2015

The /v2/achievements is now no longer dailies-only, and /v2/account/achievements is online. The backend support for the NA/EU fixes aren't going to land until the Dec. 1st patch -- and hopefully I'll have the rest of the missing achievement details in there -- so I'll open a separate pull request for those.

@Eearslya
Copy link
Contributor

Trying to document it on the wiki, noticed a couple things.
Could we potentially get some information in /v2/achievements as to exactly what the bitmask means?
Also noticed a flag you hadn't mentioned in your PR, RepairOnLogin. (Saw it on achievement 2314) Is that something we're supposed to see?

EDIT: Whoops, that's probably what you meant by "missing achievement details".

@lye
Copy link
Contributor Author

lye commented Nov 10, 2015

Yeah, the bits will make more sense when the changes to /v2/achievements goes out -- they basically correspond to indexes into a list of strings indicating which steps for the achievement there are. I left 'em in for the initial release because they're indexes into a string array that you can see in-game.

Uhh, I'll probably remove RepairOnLogin later this week. It's not really useful for application developers; left it in by accident.

@Eearslya
Copy link
Contributor

Is bits only a 0-7 thing? If so, how will that work with achievements that have more than 8?

And are the achievement categories part of the missing information planned to be added?

@lye
Copy link
Contributor Author

lye commented Nov 10, 2015

The values in bits are all uint32_ts, so they can go up quite far. They're stored internally as a bit array, hense "bits".

EDIT: also I couldn't come up with a better name.

@ghost
Copy link

ghost commented Nov 10, 2015

Hello there,

I regret to inform you that the API provides erroneous data.

I currently only randomly Armorer tested, but here are the most error:

Example Greatsword:
{"id": 57, "current": 1, "max": 5000, "Done": false}

I have the success, however, for a long time.

If I find the time today, I'll make a quick side and then the evening to have today a comparison with the information in the game.

@lye
Copy link
Contributor Author

lye commented Nov 10, 2015

Yep, found the issue for that. Will try to get a fix out with the next patch ;____;

Basically all completed counters are broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants