Move vetDamage total inside instigator check

[clyfordv]

Previously, all damage taken from any source (including an allied unit or self) would be included in the veterancy dispersal calculations. This moves the addition operation to VetDamageTaken (the denominator in veterancyDispersal) inside an "IsEnemy" check, which prevents (for example) the Paragon from stealing 10,000 damage of veterancy experience for itself as it dies.

Downside is a particular unit may get more credit than it is "rightfully" owed for a unit that was hit by friendly fire--not a big downside, in the scheme of things.

[clyfordv]

... also, with the pitiless attitude of working with code I didn't write, is there a reason not to condense VetDamage (<EntityId, Unit>) and VetInstigators (<EntityId, damage>) into <Unit, damage>, and eliminate a whole table declaration on every unit in the game? (see most recent commit, tested it with the Mavor + Paragon and appears to work correctly)

[Garanas]

... also, with the pitiless attitude of working with code I didn't write, is there a reason not to condense VetDamage (<EntityId, Unit>) and VetInstigators (<EntityId, damage>) into <Unit, damage>, and eliminate a whole table declaration on every unit in the game? (see most recent commit, tested it with the Mavor + Paragon and appears to work correctly)

There certainly is!

When you use a table as a key the memory reference is used as the actual 'key' value. This memory reference is not deterministic and is therefore almost guaranteed to be different among players. As a result iterations on this collection are in a different order for each player. And the moment that happens all bets are off and the game can randomly desync, especially when mods or future contributors may suspect it is safe to use for other purposes.

All in all; no - we can't eliminate the table 🤠

[Garanas]

The other veterancy values start with Vet, I think this one should too.

[clyfordv]

let's do VetMassKillCredit

[lL1l1]

How about VetMassKilled?

[Garanas]

We could maybe however use this:

fa/engine/Core.lua

Lines 192 to 195 in ceb09ce

	---@param id EntityId
	---@return UserUnit | Unit
	function GetUnitById(id)
	end

I'm not sure how efficient that function is, we'd need to benchmark it. But that way we can entirely rely on the VetInstigators table and get rid of the VetDamage damage.

[lL1l1]

Would be nice to annotate what stats units usually have.

[Garanas]

Would be nice to annotate what stats units usually have.

How would we annotate that?

[Garanas]

The use of GetEntityById and GetUnitById appears to be fast - I can do 900 queries in 0.4ms. That's sufficient for this type of task!

[Garanas]

@clyfordv can you write a changelog snippet? Make sure to also describe the practical change for the players