Quantcast
Thoughts on the Inspection API - WoWInterface
 
Thread Tools Display Modes
07-16-10, 10:56 AM   #1
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
Thoughts on the Inspection API

Hello all,

I'm the author of a class profiling tool. At the core, it pidgeon-holes party members into "roles", before deciding how to best play nicely with those roles (by giving them blessings, choosing totems, etc). Picking the right role is based on data from their class, spec, dungeon role, pet selection, etc.

A lot of that critical player data is read from the Inspection API. Which isn't perfect; it's cumbersome, ambiguous, shared, and vague. To be fair...it works... but Blizzard taking steps to limit it's impact on servers tells me two things: (A) It's been WAY more popular/used than they expected, and (B) it's therefore a prime candidate for revision in Cataclysm.

I hope this thread can fuel some constructive discussion on if/how the Inspection API could be changed, to make our lives easier. and by "our", i mean "our users". Lean, efficient, well designed add-ons make users happy. We just need the tools to get there
 
07-16-10, 11:14 AM   #2
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
A few of my concerns with the current implementation:

Ambiguity: Asynchronous Completion

Goal: let addons know when the inspect API is ready, busy, abandoned, or failed.

Current Challenge: Authors must resort to timeouts, when the API fails or is abandoned. documentation on the proper timeout values isn't readily available

Kudos to whoever made this call asynchronous. That lets the server merrily collect data on my inspect target, without locking my UI. Fabulous. That process isn't as transparent as I'd like, though.

The current workflow is something like (a) request data via NotifyInspect, (b) cache the name of the player locally, then (c) wait for an event from INSPECT_TALENT_READY. If INSPECT_TALENT_READY is received (d) do stuff, and call ClearInspectPlayer when finished.

As far as i can tell, INSPECT_TALENT_READY will fail to fire if: the inspect target is bogus; another NotifyInspect is sent while the old one is processing; the inspect target is (or moves) out of inspect range while processing; or the server gets too many requests within a ~10 second window. "Fail to return" really sucks, because it's an undetectable event. It's entirely possible the call would return after, say, 20 seconds, or 5...that information isn't publicly known. So an author has to code an arbitrary timeout

Note that other addons can mess with each other. Firing NotifyInspect while another addon is waiting for INSPECT_TALENT_READY will severely mess with them. You can detect this behavior by hooking NotifyInspect, INSPECT_TALENT_READY, and ClearInspectPlayer; it's generally bad etiquette to call NotifyInspect between when anyone else has called notify inspect, and when they call ClearInspectPlayer. There's nothing to enforce this though... an addon can very well call back-to-back inspect data for multiple players without clearing the inspect player, effectively hogging the API.

To Solve: A unique ID could be assigned/returned from NotifyInspect, which is attached to the relevant INSPECT_TALENT_READY event. Add notifications for INSPECT_FAILED and/or INSPECT_ABANDONED, either as new events, or as a return code in INSPECT_TALENT_READY. I'm sure there are other things that could work.
 
07-16-10, 02:02 PM   #3
IQgryn
A Cyclonian
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 46
It's possible to detect when someone else uses NotifyInspect during your waiting period and re-fire your event. Still a nuisance, though.

If Blizzard doesn't provide a better way to handle things, I may have to go write a LibQueueInspect that does this properly, once, so we don't have a bunch of separate implementations that step on each others' toes. Maybe I could get Ace or somebody to include it.

It would have to handle stuff not being returned, probably with a fairly short timeout, and re-send requests as necessary. I'd also have to add some sort of priority system so that an inspect-happy addon got pushed back in line if an addon that only requests a few now and again asks for an inspect.

Anyway, I'll give them some time first. Hopefully they'll get it right.
 
07-16-10, 02:15 PM   #4
IQgryn
A Cyclonian
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 46
Another problem is it is hard to match which player you inspected with the notification. It would be much nicer to be able to inspect by name, and have that returned as the inspect ID, or something similar. It would be even better if they did name+requestID.

Oh, and it would be nice if NotifyInspect returned an error when you did too many requests at once, instead of silently failing. They could even do that client-side to avoid taxing the servers for it. It would let you handle another addon pumping out requests a little better.
 
07-16-10, 02:26 PM   #5
ckaotik
A Fallenroot Satyr
 
ckaotik's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 29
Originally Posted by IQgryn View Post
I may have to go write a LibQueueInspect that does this properly, once, so we don't have a bunch of separate implementations that step on each others' toes.
Definetely a good idea. Currently, as far as I know, there is LibTalentQuery which can be combined with LibGroupTalents, both written by Zeksie, the author of ZOMG!Buffs.
This only covers talents, though, and addons like GearScore need item information as well. Having one library keeping care of both might be a little too much, but having a base LibInspect and on top of that something for gear and talents would go a long way, I suppose.

Anyhow, I guess we just have to wait and see what Blizzard makes out of this.
__________________
It all starts to make a creepy kind of sense. Avatar

Last edited by ckaotik : 07-16-10 at 02:55 PM. Reason: German ~= English. Tiny typo
 
07-16-10, 02:42 PM   #6
IQgryn
A Cyclonian
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 46
Originally Posted by ckaotik View Post
Currently, as far as I know, there ist LibTalentQuery which can be combined with LibGroupTalents, both written by Zeksie, the author of ZOMG!Buffs.
Oh, I didn't know about that one. I'd probably just (help) update it to handle the timeouts properly. The gear can be grabbed at the same time as the talent data the way it appears to work currently, or I could add a function that gathered the appropriate data (specified with flags) and passed that to your callback function instead.

Actually, I rather like that second option. Why should each addon have to handle the complexities of the talent tree API when you could just return a table with subtables for each tree (indexed by both name and tree number) that had rank numbers for each talent (indexed by both name and talent number). Gear could be done as a table indexed by slot name and/or number, too.

Code:
...
-- the last two args specify whether to return gear/talent info, respectively
TalentQuery.RegisterCallback(self, "hasTitansGrip", false, true)
...
TalentQuery:Query(unit)

function hasTitansGrip(event, pname, prealm, unitid, talents, gear)
    return talents["Fury"]["Titan's Grip"] == 1
end
 
07-16-10, 02:53 PM   #7
ckaotik
A Fallenroot Satyr
 
ckaotik's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 29
I just found that library last week myself. Also, besides talents and gear, there are also achievements and who knows what else. Maybe that's just me but I don't really like having one library that does too many things - e.g. I only care about talents, for buff suggestion, but it stores and manages gear as well. Depends on how much overhead this would create.
__________________
It all starts to make a creepy kind of sense. Avatar
 
07-16-10, 03:42 PM   #8
IQgryn
A Cyclonian
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 46
Ideally, it would just be the memory taken by of a couple if extra data-gathering functions. They'd only get called if you asked for them to, so there'd be no runtime overhead as long as you use all the results you ask for. Of course, if you ask for the gear just to look at their weapon(s), then there'd be some overhead there. My example above is a bad one in this regard, but it shows the simplicity of using it.

Note that I would be adding functions to the library's API, not changing the existing ones. You'd still have the option of doing it yourself.

Option 1:
Use the existing API, do all the data-gathering work yourself.

Option 2:
Use the API I proposed, have an easier time of it at the possible expense of a little more gathering overhead (if you don't use the full talent tree, for example).

Option 3:
Use the proposed API, but only request the bits you need all of and do the rest yourself.
 
07-19-10, 10:52 AM   #9
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
Originally Posted by IQgryn View Post
Another problem is it is hard to match which player you inspected with the notification. It would be much nicer to be able to inspect by name, and have that returned as the inspect ID, or something similar. It would be even better if they did name+requestID.
Shared resource makes kittens cry

Goal: let addons scan whatever inspect data is currently available.

Current Challenge: No facility to identify which unit the inspect data belongs to.

It's really awkward having to deal with the Inspect API, knowing that any addon at any time can interfere with your calls. Making a library would only fix half the problem; any other addon could still interfere with the Inspect API, and therefore mess with the entire Inspection library.

I think managing concurrent inspection requests is best accomplished in the root API itself. Not much would have to change: add either serialized requests, and pair "ready" notifications with unique identifiers (for the request or the unit)... or "not ready" events if the inspect API is busy. There just isn't enough transparency in the current implementation to allow either of those things. or, I'm a bad programmer.

It's also a real shame that the data returned from the Inspect API can't be shared between addons. "INSPECT_TALENT_READY" doesn't really help a concurrently running addon. It would know that somebody else is at some state in the inspect workflow...but it can't piggy back and/or use the currently available inspect data out of context; it needs a player GUID or a unitID. That data could be made available in a 3rd party library, or a revamped Inspect API implementation. without the identity data, though, sharing of inspect data results isn't really feasible :3

To Solve: include a unit GUID in the INSPECT_TALENT_READY notification; or, provide a function to query the talent group's owner: guid = TalentGroupOwner()
 
07-19-10, 11:07 AM   #10
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
Originally Posted by ckaotik View Post
I just found that library last week myself. Also, besides talents and gear, there are also achievements and who knows what else. Maybe that's just me but I don't really like having one library that does too many things - e.g. I only care about talents, for buff suggestion, but it stores and manages gear as well. Depends on how much overhead this would create.
Feature Creep & Bloat

It's really weird how much data is returned to the client about a player. Are achievements pulled back in the same call? Statistics? Gear? Talent Groups? Reputations? Yesterday's Lunch?

Sometimes, all I want to know is the player's active talent group. That shouldn't take 5 seconds! It would be absolutely rad if the Inspect data could be segregated and/or sped up. Or at least, if i could just get some quick data that could potentially change every 10 seconds.

Taking a specific example, I can cache a player's gear and talents. I can persist that across sessions; awesome. What I can't cache, is the currently active talent spec. I can't even persist that within a session.

If a party member gets summoned with a summon stone, addons can scan their talents & gear when they pop up. if the player then zones in...addons have to wipe the slate. While the player is sitting waiting inside the dungeon, the party member at the stone could be futzing with their talent spec, of flipping gear around. Any time the party member leaves the client zone of awareness, things get out of sync...when the party member comes back into sight, addons need to ask for ALL the inspect data again, JUST to see if the party members changed spec or gear while they were AWOL. That's retarded!

To Solve: Decouple inspect data that is highly variable: gear & active spec

Last edited by toddimer : 07-19-10 at 01:07 PM. Reason: typo (talent ~= spec)
 
07-19-10, 11:31 AM   #11
Shadowed
...
Premium Member
Featured
Join Date: Feb 2006
Posts: 387
Originally Posted by toddimer View Post
Feature Creep & Bloat

It's really weird how much data is returned to the client about a player. Are achievements pulled back in the same call? Statistics? Gear? Talent Groups? Reputations? Yesterday's Lunch?

Sometimes, all I want to know is the player's active talent group. That shouldn't take 5 seconds! It would be absolutely rad if the Inspect data could be segregated and/or sped up. Or at least, if i could just get some quick data that could potentially change every 10 seconds.

Taking a specific example, I can cache a player's gear and talents. I can persist that across sessions; awesome. What I can't cache, is the currently active talent spec. I can't even persist that within a session.

If a party member gets summoned with a summon stone, addons can scan their talents & gear when they pop up. if the player then zones in...addons have to wipe the slate. While the player is sitting waiting inside the dungeon, the party member at the stone could be futzing with their talent spec, of flipping gear around. Any time the party member leaves the client zone of awareness, things get out of sync...when the party member comes back into sight, addons need to ask for ALL the inspect data again, JUST to see if the party members changed spec or gear while they were AWOL. That's retarded!

To Solve: Decouple inspect data that is highly variable: gear & talent choice.
It doesn't take 5 seconds, it's closer to 1-3 depending on latency. I actually doubt you would see that much of an increase by only sending talent data, at least nothing to make it noticeable. And achievements/statistics are a separate call, unrelated to NotifyInspect.
 
07-19-10, 01:05 PM   #12
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
Originally Posted by Shadowed View Post
It doesn't take 5 seconds, it's closer to 1-3 depending on latency. I actually doubt you would see that much of an increase by only sending talent data, at least nothing to make it noticeable.
Ignoring timeouts, I agreed on both counts. My annoyance is probably just an artifact of what my addon is doing. For all intent and purposes, querying the active talent spec of a target takes 1-3 seconds (depending on latency). That's pretty annoying/expensive just for a 1/2 response.

Originally Posted by Shadowed View Post
I actually doubt you would see that much of an increase by only sending talent data, at least nothing to make it noticeable.
Conversely, only sending the active talent spec could result in significant savings for that specific call. Right now that's coupled to the inspect data via GetActiveTalentGroup. Allowing free-for-all querying of anyone's active talent group would be rad, if possible.
 
07-19-10, 10:38 PM   #13
toddimer
A Deviate Faerie Dragon
 
toddimer's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 18
I hope my walls of text don't squelch the discussions... the posts in here so far have been constructive and well thought out.

LibTalentQuery sounds neat; I suspect someone pointed me to it at some point already. the idea of a buncha addons sharing data through a library sounds exciting. Blizzard providing that capacity in the native API would be even more exciting, but, that's not exactly within our control

Note to self; look into LibTalentQuery!
 
07-20-10, 06:34 AM   #14
ckaotik
A Fallenroot Satyr
 
ckaotik's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 29
Originally Posted by toddimer View Post
I hope my walls of text don't squelch the discussions... the posts in here so far have been constructive and well thought out.
Nah, at least for me there just isn't that much to write anymore.

Originally Posted by toddimer View Post
LibTalentQuery sounds neat;[...]the idea of a buncha addons sharing data through a library sounds exciting. Blizzard providing that capacity in the native API would be even more exciting, but, that's not exactly within our control
You summed it up pretty well - now, if only Blizzard would do something about it, that would be awesome. I guess, as they obviously are aware of inspect request issues (why else would they throttle them?), they might "fix" it. If they find the time, that is - Cataclysm isn't exactly a small project.
__________________
It all starts to make a creepy kind of sense. Avatar
 
07-23-10, 05:10 AM   #15
ckaotik
A Fallenroot Satyr
 
ckaotik's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 29
Oh! I just did a mini test in the beta, being /eventtrace and inspecting a player. Guess what happened!

INSPECT_READY, time: ..., arg1: 0x0100000000916C3

It seems we'll get a GUID with the event :3 Also, it seems gear and talents are included in this. Achievements still trigger a different event.
__________________
It all starts to make a creepy kind of sense. Avatar

Last edited by ckaotik : 07-23-10 at 05:45 AM.
 
 

WoWInterface » AddOns, Compilations, Macros » Cataclysm Beta » Thoughts on the Inspection API

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off