Fix inaccuracy of the DoTTime weapon stat

[lL1l1]

Issue

DoTTime represents how long it should take damage pulses to occur, with t = 0 being the first pulse and t = x being the last pulse.
In reality, there was error from WaitTicks flooring tick wait values and the DoT interval being calculated as DoTTime / DoTPulses instead of DoTTime / (DoTPulses - 1) (the first pulse occurs instantly, you don't need to wait for it).

This causes unit stats in the blueprints to be rather inaccurate depending on how much error accumulated from WaitTicks's flooring.
Some egregious cases are Aeon T3 mobile arty dealing damage over 4.2 seconds instead of 5 seconds, Aeon T3 static arty actually pulsing a 2nd time after 1 second instead of 2 seconds, and the UEF T1 bomber dealing damage over 3.6 seconds instead of 4.2 seconds. A lot of the 1 and 0.5 second times are actually 0.8 and 0.4 seconds respectively.

Description of the proposed changes

• 230c8a9 Rework the damage over time logic so that it is accurate to the DoTTime stat by correctly counting the first pulse as taking 0 time, and using an accumulator variable to correct the flooring errors. Also streamlines the Projectile DoDamage control flow which made it easier to modify (less branches where code needs to be duplicated).
• 37ef379 Adjust existing DoTTimes so that they match the old, inaccurate behavior, to avoid changing balance.
• 068d86f Fix variable localization typo in UnitDoTThread.
• The other commits refactor the code to be clearer or fix intellisense.

Testing done on the proposed changes

Logging when damage ticks occur before/after the changes on a UEF T1 bomber (UEA0103) modified to shoot 1 projectile.
Comparing the before/after stats using this console command on the unmodified unit stats:

UI_Lua 
for i, v in __blueprints do
	if type(i) == 'number' or not v.Weapon then continue end
	for _, w in v.Weapon do
		local t, p = w.DoTTime, w.DoTPulses
		if t > 0 and p > 0 then
			local oldDoTTime = (p-1) * math.floor(10*t/p)/10
			local newDoTTime = (p-1) * t/(p-1)
			LOG(string.format('Weapon: %-20s actual DoTTime before: %4.3g, after: %4.3g', w.BlueprintId, oldDoTTime, newDoTTime))
		end
	end
end

Output:

Weapon: dea0202-3-Bomb       actual DoTTime before:  5.4, after:    6
Weapon: xrb2308-1-Turret01   actual DoTTime before:  0.4, after:  0.5
Weapon: urb2205-1-Turret01   actual DoTTime before:  0.4, after:  0.5
Weapon: uab2303-1-MainGun    actual DoTTime before:  0.8, after:    1
Weapon: ura0204-1-Bomb       actual DoTTime before:  0.8, after:    1
Weapon: xrb2309-1-Turret01   actual DoTTime before:  0.4, after:  0.5
Weapon: uea0103-1-Bomb       actual DoTTime before:  3.6, after:  4.2
Weapon: urs0302-5-Torpedo01  actual DoTTime before:  0.4, after:  0.5
Weapon: urb2109-1-Turret01   actual DoTTime before:  0.8, after:    1
Weapon: urs0201-3-TorpedoL   actual DoTTime before:  0.4, after:  0.5
Weapon: ual0304-1-MainGun    actual DoTTime before:  4.2, after:    5
Weapon: xrl0305-2-Torpedo    actual DoTTime before:  0.4, after:  0.5
Weapon: urs0203-1-Torpedo01  actual DoTTime before:  0.8, after:    1
Weapon: uab2302-1-MainGun    actual DoTTime before:    1, after:    2
Weapon: daa0206-1-Suicide    actual DoTTime before:  9.5, after:   10
Weapon: xrl0403-3-Torpedo01  actual DoTTime before:  0.4, after:  0.5
Weapon: urs0304-2-Torpedo01  actual DoTTime before:  0.4, after:  0.5
Weapon: xrs0204-1-Torpedo01  actual DoTTime before:  0.4, after:  0.5

Checklist

• Changes are annotated, including comments where useful
• Changes are documented in the changelog for the next game version

[Garanas]

I understand the differences between EntityDoTThread2 and EntityDoTThread. At least I think they should be named better. But beyond that - from a maintainability perspective, is it desirable to have two instances of functions that are almost identical?

It may adjust the balance of mods (as you mention), but that weighs less than the reduced maintainability in my point of view.

[lL1l1]

I kept the second function around in case there are any non-projectile scripts utilizing the default damage's DoT by itself, and are expecting the first damage instance to happen immediately. Unfortunately since the total duration is calculated outside the function and we only get the interval it's not possible to cleanly avoid this.

Mod balance was not a concern, and I think they benefit more from visuals lining up with damage effects than potentially losing some precision tuned and tested stats.

[Garanas]

I kept the second function around in case there are any non-projectile scripts utilizing the default damage's DoT by itself, and are expecting the first damage instance to happen immediately. Unfortunately since the total duration is calculated outside the function and we only get the interval it's not possible to cleanly avoid this.

Do you have an example where this is done?

[lL1l1]

No.

[Garanas]

You respond fast 😃 , in that case my preference would weigh towards better maintainability and just go for one of the two definitions. But I'll leave that decision to you and @BlackYps

[clyfordv]

Tested, changes are good. Agree with Jip on minimizing number of functions if we can't identify external usage.

0th, 1st, and 11th tick of Emissary damage cycle:

[BlackYps]

I also agree that we should not keep functions around for unconfirmed hypotheticals

[lL1l1]

Just need a simple approval now, I removed the redundant functions and the rest of the code is reviewed.