Skip to content

Conversation

@Architector4
Copy link
Contributor

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

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
@Causeless
Copy link
Contributor

Causeless commented Dec 26, 2025

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
Copy link
Contributor Author

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
Copy link
Contributor

Causeless commented Dec 27, 2025

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants