Authors CurseForge

if (not playerGUID) playerGUID = UnitGUID("player")

Quote from Xinhuan

UnitGUID("player") is actually tricky to cache (at least not as straightforward as you think), because it can return nil on PLAYER_ENTERING_WORLD and PLAYER_LOGIN.

Is this fact used to infer some useful information sometimes? Or is it just a fluke and there'd be no harm in returning the real value (if known yet) even at times when the native UnitGUID wouldn't?

Quote from Xinhuan

Seems like a fluke really. If you assumed that it is available at PLAYER_ENTERING_WORLD, and end up caching a nil value, it comes back and bites you later, and you end up putting code like

if not playerGUID then playerGUID = UnitGUID("player")

everywhere to check for a non-nil value, and it defeats the purpose of the optimization, and you might as well just call UnitGUID("player") directly.

But we don't have to blindly try to cache at P_E_W, and we don't have to put that test everywhere; we only have to do it until we get the real GUID once, cache that, and then stop checking.

And the beauty of dynamic languages is that this kind of run-time rewiring ought to be pretty easy to do efficiently. For example:

function LibUnitID:GetUnitID(unitid)
  tagID.player = API_UnitGUID("player")
  if (tagID.player) then
    LibUnitID.GetUnitID = function(unitid)
      return tagID[unitid]
    end
  end
  return tagID[unitid]
end

Now anybody who queries UnitIDs before "player" is available has this extra overhead. But there probably shouldn't be very many queries during loading, and if there are, it's not a big deal to slow them down. The first query that populates "player" resets the method, so all further queries are fast.

But it does make me wonder if there are other times when UnitGUID returns the wrong thing, or when we don't get the events we're expecting to update the cache. What if someone leaves the group while you're zoning to another continent, for example?

Do you know if any testing has been done along those lines?

Quote from Xinhuan

That's a big catch22 there. You have to put the check everywhere in order to get the GUID, and add additional "if playerGUID == nil then return end" statements to stop processing if it is currently nil or code that uses it will error.

I am not talking about code to check for the nil-ness of the variable so that you will only ever call UnitGUID() minimally. I'm talking about the code needed to check for the nil-ness of the variable so that the rest of your code doesn't error because its nil.

Ah, I think I see what you're saying. The library could do it efficiently enough, but the caller would still have to check for nil in case it actually does query before it's ever been available (as opposed to querying later when it's missing again, but we could have it cached).

In theory I guess callers could do the same rewiring trick, to at some point replace their method with a version that assumes a non-nil response. But at that point you're probably right that it's not worth it, since "player" accounted for <1% of the calls in my tests anyway.

I'm still curious about that phenomenon in general though. Do UnitIDs other than "player" return nil during zoning? I wonder if its possible for the lib to get an event at just the wrong time, try to re-cache the GUID, and get nil by mistake. Then I suppose I'd have to have it turn off cache updates between zoning events, and queue all the callbacks until we're sure we can get valid new data, and then issue them all and re-enable regular updating. That'll be tedious. I hope this fluke only happens for "player". :)

for the pew event and nil playerGUID, my guess is that addons would be hooking pew to initialize their system. with your callback scheme in place, couldn't you make a callback for PlayerGUIDChanged or something that would take the place of the pew event in terms of initialization? so instead of registering a pew event, they'd register a PlayerGUIDChanged callback to do their init. assuming that UnitGUID("player") is important to them.

I think it's kinda funny that UnitGUID("player") should ever return nil. It's the only GUID you can (theoretically) always get and which doesn't change (unless you transfer server or delete/rerolled).

A PlayerGUIDChanged() seems a weird thing to do, because literally once you called UnitGUID() after startup and got a valid return, you should never ever call it again, even after zoning etc. You can be sure the valid GUID you once got will be valid.

I think a fairly sensible strategy is to try to get playerGUID on the first (and only the first!) P_E_W, and if it fails launch a 1-sec timer to try again. Keep doing that until you have your GUID. If it fails after 5 seconds, just yell bloody murder and quit. Really that shouldn't happen.

So even if UnitGUID("player") is bogus during zoning, you should never ever have to care.


Then fire the callback for the player unit, so everybody who needs it gets the update. After that you should done.

actually, i guess instead of a playerGUIDChanged callback, you could just use "player" as the unit to track with the LibUnitID:RegisterUnit() function. "player" would be a special case scenario, tho and would require a bit of inside knowledge regarding what that callback really signifies -- namely that the player has entered the world and sync'd up with the server a little better than with the normal pew event.

the only point to the function is really to avoid ever having the UnitGUID("player") function from ever returning nil.

Quote from lilsparky

actually, i guess instead of a playerGUIDChanged callback, you could just use "player" as the unit to track with the LibUnitID:RegisterUnit() function. "player" would be a special case scenario, tho and would require a bit of inside knowledge regarding what that callback really signifies -- namely that the player has entered the world and sync'd up with the server a little better than with the normal pew event.

the only point to the function is really to avoid ever having the UnitGUID("player") function from ever returning nil.

I'm considering that. The idea would be, addons would stick their init-after-player-has-GUID code in an "init()" function, and at load they'd "RegisterUnit(player,init)".

If the player's GUID isn't available yet, it'd register just like any other callback, and eventually get called (and then probably auto-deleted, since it won't fire again). If the player's GUID is already available, then the RegisterUnit() would just immediately call back, running the init code immediately.

I'm not sure yet what events to register to try testing player GUID, though. Clearly P_E_W won't cut it, so we'd have to figure out what the first event is when we can actually be sure the player's GUID is ready. Or failing that, I guess stick it in OnUpdate or somesuch, and just unregister as soon as we get it.

That probably explains it though.

A few requests for comments:

- I've started thinking that the UnitID=>GUID portion of the delta table isn't terribly useful, since the library can be queried for GUIDs pretty fast anyway. What seems more useful is a quicker way to determine if a particular UnitID was involved in a change, without looping over the whole list of changed UnitIDs. If UnitID=>GUID, you'd have to keep your own local copy of the GUID and compare it to see if it changed, which is a little silly. So I'm thinking it would be more useful to have UnitGUID=>true, so on any callback you could check "delta.party3" to see immediately if party3 changed.

- This introduces a back-compatibility break, however, and then there's the question of versioning. Since the library is not yet released, is it kosher to make this change and still leave it at "LibUnitID-1.0-alpha"? Or should I really go to "LibUnitID-1.1-alpha" just in case someone is already trying to use an earlier 1.0-alpha build?

- What's the convention for the ## Version: line in standalone library .toc files? Should that be only the major version ("1.0") or the full version of the library ("1.0.13-alpha")?