no combat log events in build 1101+

[howardchung]

@spheenik, it looks like the old-style combat log events have been removed completely.

While I couldn't find any references to CMsgSource1LegacyGameEvent in clarity code, I ran a quick test and confirmed that there aren't any more combat log events emitted.

I've done some early investigation, and it looks like combat log events are now a little different:
odota/core#711

Basically we need to add code to handle the DOTA_UM_CombatLogDataHLTV packet, decode it with CMsgDOTACombatLogEntry, and then send that on to the CombatLog class, which will need to be changed somewhat.

The proto message contains the field names, which are a little different from what we have now:
https://github.com/SteamDatabase/GameTracking/blob/9e40ab5297e7dbce7eb3ec221eee0361fcd92356/Protobufs/dota/dota_gcmessages_common.proto#L1536

• damage_source_name seems to replace sourcename
• in general the old GameEventDescriptors had no underscores, while the proto definition does (targetname->target_name)

Depending on how busy you are this weekend I can try to help write some of this.

The good news is that the new combat log entry seems to have more interesting data :)

[spheenik]

Ok, I implemented it, but it'll break old code (a bit):
The combat log entries you will get now will implement this interface (for legacy and current):

https://github.com/skadistats/clarity/blob/master/src/main/java/skadistats/clarity/model/CombatLogEntry.java

This should be compatible with the old one, bar this:

• getType() does not return an ordinal, but the enum directly. If your code relies on the ordinal, you can get to it by getType().ordinal()
• getValue() now returns the integer.
• there is an additional function getValueName() that will translate this to a name for cases where you want it
• there's no more getGameEvent()
• there's hasXXXX() functions for all properties, which can query whether a certain property is set for the given entry.
• getSourceName() now is getDamageSourceName()

[howardchung]

Sounds good.

• I can adapt to the getType() change.
• getValue() was an integer before, so no change?
• Similarly no change to getValueName() interface?
• Never used getGameEvent(), except for toString() for debug, but the CombatLogEntry should now be toString()able itself?
• hasXXXX(), probably don't need, but nice to have

[spheenik]

I think the getType() change makes it more readable:

https://github.com/skadistats/clarity-examples/blob/master/src/main/java/skadistats/clarity/examples/combatlog/Main.java

• Looked it up again, you're right, no change to getValue() and getValueName()... :)
• toString() methods are already implemented.
• hasXXX() can get rid of this code
  

  clarity/src/main/java/skadistats/clarity/processor/gameevents/CombatLog.java

  Lines 198 to 200 in ecc697f

  	if (goldReasonIdx == null) {
  	return 0;
  	}

[howardchung]

So is the application now responsible for checking hasXXX before trying to getXXX? What happens if we try to getXXX on a field that doesn't exist (legacy replay?)

[spheenik]

yea, you're right, I'll prevent NPE happening... gimme a sec :)

[spheenik]

isAttackerHero() default is true?

[howardchung]

I think so, because before that field existed the combat log only displayed hero-related events. This is way back in the S1 era though, so it probably doesn't matter

[spheenik]

also, no null check here:

clarity/src/main/java/skadistats/clarity/processor/gameevents/CombatLog.java

Line 166 in ecc697f

return event.getProperty(abilityToggleOnIdx);

default true or false?

[howardchung]

I'm guessing false for both since if those fields didn't exist the combat log wouldn't be tracking toggle abilities? Not 100% sure

[howardchung]

I'm not sure if it's completely safe to have clarity put defaults for nonexistent fields transparently. If we're wrong about these defaults it could potentially lead to incorrect data for users.

The best thing to do probably is to return a null/undefined value that is distinct and alerts the user that the field doesn't exist. I'm not particularly worried this since I don't plan to be parsing legacy replays but it's something to think about.

[spheenik]

Made false the default.
New snapshot pushed.
Now, one thing I'll probably do now is refactor getAssistPlayerX() to a single function returning a List (the protobuf has that as well). There is no need in having to collect the assist players yourself, I think...

[spheenik]

I don't wanna return null in these cases. If the client needs to distinguish it has to use the hasXXX functions. I will leave the defaults like they were (no one ever complained) :)

[howardchung]

Okay, sounds good.

[howardchung]

changes for reference:
odota/core@ef022ba

[spheenik]

Ok, implemented that change with the assist players - all done for now.
Have fun with the new properties :)

[Decoud]

Thanks for all your work on this you two. Has anyone come across DOTA_COMBATLOG_NEUTRAL_CAMP_STACK? Looks like an interesting entry but I haven't seen it in the 25 or so pro games I've looked at (which should probably have a lot of stacking). I also haven't come across DOTA_COMBATLOG_LOCATION and RUNE_PICKUP but am not so interested in those.

Is there any documentation that tells all the possible fields a specific type of combat log entry may have? Some of the entries of the same type can have more or less fields depending on the entry. Like stun_duration and slow_duration seem kind off on DOTA_COMBATLOG_PLAYERSTATS (seems like all the fields are a bit redundant or useless in this one) so I overlooked the 2 fields but the two of them appear to pop of in the MODIFIER_ADD where they may be a bit more useful.

Another thing that may be specific to how I've done it (which has a lot of archaic code) and may have ceased to be relevant a while ago is a possible change in the inflictor_name field. Back with Valve's original C++ parser the inflictor_name field used to have a 0 for "dota_unknown" for a lot of entries as all the auto attacks and a lot of other things were given that. Current getInflictorName() gives an empty field instead of a 0 while I think the old game events version gave a "inflictorname=0" probably not an issue unless you were doing things inefficiently like I was (simple fix on my end to just add the 0's in but others may be interested in this).

[howardchung]

camp stack is new and I haven't seen it yet. Maybe it'll show up in future replays.

No documentation unfortunately, you'll just have to try to match up the combat log with watching the replay.

I think with the protobuf combat log you'll get something for each field, but the data may be null or otherwise invalid depending on the type of combat log entry.