@OnEntityPropertyChanged triggers too early (different from #134)

[austinmilt]

I have an issue similar to #134, but slightly different. It appears that some entities get handles as properties of other entities before those child entities are actually created. As a result, triggers of @OnEntityPropertyChanged can occur during the initial entity creation process. In particular, CDOTA_Unit_Hero_*.m_hAbilities.* handles can be assigned before the abilities themselves have been created. So I'm running into an issue where entities.getByHandle() returns null for abilities that havent been created even though they have an assigned handle. The real problem though is that @OnEntityPropertyChanged doesnt trigger again when the event is focused on the hero instead of the ability itself, so large portions of the replay are skipped where entities are actually being created.

Before suggesting solutions, to say exactly what I want to accomplish: any time a hero's ability slots (as they appear in-game) or item slots (as they appear in-game) change, I want to know the npc_dota... name of the ability/item in every slot including the first time these are defined. For instance, if Rubick steals Black Hole, I want to know that Black Hole appears in Rubick's 4'th slot. Or, if Magnus buys a Stout Shield, I want to know that the fourth item slot of Magnus now contains a Stout Shield. See issue #126 for more. Or, if Magnus moves the Stout Shield to his 3rd slot, I want to know that too.

I dont think this is the same issue as #134, which seems to be about the triggering of @OnEntityPropertyChanged before the "parent" entity itself has been created. Rather, I'd like @OnEntityPropertyChanged to fire the first time when the "child" entity (with the handle given in the property value) has been created. In practice, I dont know the best way to change clarity to make this happen.

One route would be to add an option and subroutine to the @OnEntityPropertyChanged listener that checks that the entity (if there is such an entity) corresponding to the property of interest actually exists. E.g. if CDOTA_Unit_Hero_Lina.m_hAbilities.0000 changes from null to 5, dont trigger unless entities.getByHandle(5) is non-null. I can see several issues stemming from this (What if the property isnt a handle? If the handle at 0000 doesnt change in the future, how will the listener trigger?).

Another option that would fix my particular problem would be a method of creating a listener, or activating a listener, once some condition has been satisfied. Something like

public boolean my_condition(...){
   return true;
}

@StartOnCondition(Command my_condition)
@OnEntityPropertyChanged(...)
public void do_something(...) {...}

Obviously this is syntactically bogus, and maybe clarity has a means of doing this already. I'm also not sure that some of the same issues for the previous solution wouldnt also apply here.

I believe there are workarounds existing in clarity that I could use, but it would be great if the convenience of @OnEntityPropertyChanged could actually work for me.

[spheenik]

Hmm from reading this and trying to make sense of it, it seems that this will happen only for multiple creates on the same tick?

Clarity does the following

• Gets a packet entities packet, which contains updates / creates for that tick, ordered by entity index.
• Clarity processes the changes and fires events immediately.
• Now if an entity early in this cycle gets a handle to an ability which is created later in the cycle, your lookup willl fail.

Would that describe the effects you see? If that's the case, something (like, the event emitter) should buffer all events until the packet entities packet has been processed completely.

To work around, you could buffer yourself and use @OnTickEnd to process the buffered data.

But I agree something should be done about this, but I'm unsure about the timeframe I can do this.

[austinmilt]

Yes, that could very well be the problem.

Fleshing out your suggestion a little more: the approach is to, e.g. store the handles for properties with each call of @OnEntityPropertyChanged and then also call @OnTickEnd to resolve the entities themselves at the completion of the entities packet, correct?

The major problem I see with this, and the reason I was so stoked to use @OnEntityPropertyChanged, is it cut out all the ticks where nothing of interest is happening (the vast majority of them). If I then employ @OnTickEnd (which occurs on every tick, correct?) then I cut out the major efficiency potential of @OnEntityPropertyChanged.

I guess another option would be to buffer handles until the game ends (What event would I look for using @OnGameEvent?) and then do the EntityNames StringTable lookup at the end, thus ensuring all created entities would be in the table... though is it possible that handles are removed from the StringTable if the entities no longer are needed?

[austinmilt]

You know, your current setup has an implicit logical OR association among multiple decorator listeners, e.g. if you use both @OnTickEnd and @OnEntityPropertyChanged above the same method, then the method is invoked when either listener is triggered. An alternative approach, and one I dont know if you can actually implement, is to wait until both are triggered, i.e. implement an AND. In other words, keep track of which listener has been triggered for a method. When all have been triggered, call the method, and reset the status of the listener. By default, using multiple decorators would use the OR relationship, but maybe you can add some syntax/options to enable customization, e.g.

@OnTickEnd
@OR
@OnEntityPropertyChanged

vs

@OnTickEnd
@AND
@OnEntityPropertyChanged

which again, I'm sure is nonsense, but maybe you get the idea. This would make the workaround you proposed much easier for me to implement.

EDIT: On the other hand, I guess I can create my own flags to do this, so maybe that's not worth the effort. With either approach @OnTickEnd is triggered all the time, even though the associate method wouldnt necessarily be.

[spheenik]

That's not what I meant: You have one function for the property changes, but instead of doing the lookups directly, you just store them somewhere.
Then you hava another function @OnTickEnd which then takes the buffered values and does the lookup, and eventually clears the buffers.

EDIT: Well, your first post describes this very well, so yes. It wont cost much, performance wise, to be called on tick end for every tick, as the function won't do much on most ticks.

[austinmilt]

Sure, but the other function invoked by @OnTickEnd would occur at every tick, correct? I can always check inside this function if the buffers are empty, but regardless dont I lose the efficiency of @OnEntityPropertyChanged by having an additional method invoked by @OnTickEnd?

[spheenik]

And regarding the @And idea. I can't do this, since the signatures vary for each event type :)

[spheenik]

Yes, it would be called every tick, but won't cost much if the list is empty.

Actually, clarity does this already under the hood (for combat log events), since those also have problems with string table lookups if I fire them instantly.

[spheenik]

Also, if you implement this, it's only a workaround, and if you come back here and tell me this solved your problem, I will implement something like this in clarity.

[austinmilt]

Implemented, and seems to have solved the problem and is still plenty fast! Given the code itself is surrounded by a bunch of other code (see #126), I wont reproduce here. But the method is essentially as you suggested:

1. Create a class variable entityHandles that holds entity handles (I used a nested HashMap with hero handles as keys and lists of items and abilities as values).
2. Use @OnEntityPropertyChanged to decorate a method that grabs hero IDs and ability/item IDs from hero attributes and stores them in entityHandles.
3. Use @OnTickEnd to decorate a method that (a) returns if entityHandles is empty, if not (b) resolves hero, ability, and item names (in EntityNames StringTable) by iterating over handles in entityHandles, then (c) clears entityHandles.

My method ends up creating a bunch of dynamic lists and hashmaps, which I think could be circumvented with some more intelligent programming, but it still runs plenty fast for my purposes.

[spheenik]

Good! So it seems we nailed the problem. I will put this on my todo list of things to handle directly in clarity.