fa icon indicating copy to clipboard operation
fa copied to clipboard

Fix inaccuracy of the `DoTTime` weapon stat

Open lL1l1 opened this issue 1 year ago • 5 comments

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

  • 230c8a9e7a3908209b8dc8504cbe541022c3927b 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).
  • 37ef379c8bd8f63e04c2a460e5c8a10362d7b2cb Adjust existing DoTTimes so that they match the old, inaccurate behavior, to avoid changing balance.
  • 068d86f5869a65cadd88df595e64819ce04a62a5 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

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

lL1l1 avatar Sep 21 '24 23:09 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.

lL1l1 avatar Sep 23 '24 10:09 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.

Do you have an example where this is done?

Garanas avatar Sep 23 '24 12:09 Garanas

No.

lL1l1 avatar Sep 23 '24 12:09 lL1l1

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

Garanas avatar Sep 23 '24 13:09 Garanas

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

BlackYps avatar Oct 21 '24 11:10 BlackYps

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

lL1l1 avatar Nov 06 '24 09:11 lL1l1