Make emitter burst independent of delta time

[Architector4]

This code just replaces the value with 60Hz where it's used in relation to bursting. This is a bit of a hack, but it's the cleanest solution I can think of right now without going in and editing burst strength of everything lol

This makes total flight height consistent regardless of the delta time, at least as far as I can tell.

Old behavior:

tmp.sxfV3GMlO6.mp4

New behavior:

tmp.fX7IGltvfu.mp4

I can still get somewhat higher flight height with smaller delta time, but I think it's just because it allows for higher grained precision burst spamming lol

[Causeless]

I like this in principle as it's obviously a fix, but the manner in which the fix is written makes the code a bit more unclear. Furthermore the comment can be ambiguously interpreted as meaning the exact opposite of the intended message and implying that bursts should be delta-time dependant.

I'd prefer that we do some minor refactoring to make this obvious and clear:

1. Add a new GetBurstFuelUsage() function
2. Within this function, use the 1/60 trick, but in the comment state that the values were originally tuned for 60hz so that's why we're multiplying by that.
3. Use that in the external code with no multiplication by dt, to make it clear that bursts use a flat amount of extra fuel.

[Architector4]

I like this in principle as it's obviously a fix, but the manner in which the fix is written makes the code a bit more unclear. Furthermore the comment can be ambiguously interpreted as meaning the exact opposite of the intended message and implying that bursts should be delta-time dependant.

I'd prefer that we do some minor refactoring to make this obvious and clear:

1. Add a new GetBurstFuelUsage() function
2. Within this function, use the 1/60 trick, but in the comment state that the values were originally tuned for 60hz so that's why we're multiplying by that.
3. Use that in the external code with no multiplication by dt, to make it clear that bursts use a flat amount of extra fuel.

That sounds like a good plan, but after legitimate half an hour of staring at various jetpack/emitter code, I must admit I am completely and utterly lost.

Within AEJetpack::Burst, this whole formula for how much fuel should a burst consume looks bonkers magicky:

float fuelUsage = g_TimerMan.GetDeltaTimeMS() * static_cast<float>(std::max(GetTotalBurstSize(), 2)) * (CanTriggerBurst() ? 1.0F : 0.5F) * fuelUseMultiplier; // burst fuel

As far as I can tell, it was ultimately introduced in the commit 55414a5. While there's a changelog comment also added that mentions the CanTriggerBurst() ? 1.0F : 0.5F multiplier (though, still, why not zero if you can't trigger a burst??), the std::max(GetTotalBurstSize(), 2) feels completely inexplicable. Either way, with this formula in sight, fuel usage calculation from a burst seems very arbitrary.

The class AEmitter does not have any concept of fuel usage, but also needs this patching for the burst part in EstimateImpulse() method. Obviously, it cannot use such a GetBurstFuelUsage() method. This, to me, indicates that the correct place to put the 1/60 multiplier is in a new method, GetBurstStrength() or whatever, defined on AEmitter.

At this point I don't know if I should then put in the bonkers magic formula from above into such a method and just use it directly in AEJetpack::Burst and also use it in AEmitter::EstimateImpulse method too, changing its overall calculation, or if I should put the calculation currently used in AEmitter::EstimateImpulse into it and then find a way to shoehorn it into AEJetpack::Burst.

...Speaking of, the overall per-emission calculation in AEmitter::EstimateImpulse also feels inexplicable at the second... Get emission rate in particles per minute then divide it by 60 (to get particles per hour(????)) then multiply it by GetBurstSize() (which i guess makes sense after some thinking, because then we're measuring only the burst there, but still confused me for a second), and also add the particle count negative one (probably should be multiply)?? Huh.

.....Worse of all, I just discovered a parallel universe emitter class, PEmitter, which has nearly identical code to AEmitter in most cases except with slightly different variation and polish and that I didn't figure to apply my fix to because I didn't know it existed until now.

Infact, PEmitter/AEmitter could use some updating for code quality parity, or maybe just merging the two together because those look nearly identical for the most part already, except having a different superclass. I can't quite do that myself because I don't know exactly why we're rotating Moses which of these classes and subclasses is what lol

---

I wanted to tackle your feedback today but I feel like my brains are already done for today lmao

[Causeless]

Hah.

Yeah, I'd forgotten how terrible this stuff was. And you've found some genuinely frightening stuff that's even scarier than the already frightening stuff we knew about. I assume that particle-per-minute was meant to be multiplied to 60 for per-second... I'd need to look closer to really understand.

Most of this code is worth changing, backwards compatibility be damned, because frankly I've never felt like this stuff worked well regardless of code quality. I'd genuinely vote for deleting bursts entirely and instead doing something entirely programmatically and automatically (like, actors jump up with their feet when they start jetpacking on the ground).

You should've seen how even more terrible this stuff was before AEJetpack existed. Jetpack code was duplicated between different actors, and the jetpack itself was just an emitter with no extra logic. I always intended to eventually do the same sorta thing and unify emitters, but I never got around to it built up the mental strength to attempt it.

Given the complexity here, I think the best thing is to add some comments saying "fix this" and explaining all the problems you've found. And then maybe one day we can fix everything. But for now this fix is good.

[Architector4]

ough; scratch mental strength to fix it all, I'm unsure I have mental strength to even document properly all the stuff I found that should be fixed lmao

[Causeless]

Getting this in for now, I'll adjust the comments to be a little clearer once merged. We can keep a mental note (or just forget about this) one day in the future about the other various problems in the emitter code.

[Architector4]

oh yeah i kind of just mentally blocked this whole situation sorry lmao

[Causeless]

...Speaking of, the overall per-emission calculation in AEmitter::EstimateImpulse also feels inexplicable at the second... Get emission rate in particles per minute then divide it by 60 (to get particles per hour(????)) then multiply it by GetBurstSize() (which i guess makes sense after some thinking, because then we're measuring only the burst there, but still confused me for a second), and also add the particle count negative one (probably should be multiply)?? Huh.

@Architector4 I took another look at this and validated that the code is not as awfully bad as it looks.
We get particles-per-minute then divide by 60 to get particles-per-second, multiplied by the delta-time to get particles-per-frame. Dividing by 60 obviously makes the number smaller, whereas particles-per-hour would necessarily be a bigger number :)

The rest of the code is still inaccurate, but not extremely so- it still seems to consider bursts as being dependent on emissions-per-second and therefore deltatime, but it roughly approximates the true value. Definitely worth a rewrite though, I'll see if I can improve it.

[Architector4]

Dividing by 60 obviously makes the number smaller, whereas particles-per-hour would necessarily be a bigger number :)

...you're right

i think my brain just got fried from staring at this all too long lmao