-
Notifications
You must be signed in to change notification settings - Fork 25
Make emitter burst independent of delta time #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Make emitter burst independent of delta time #252
Conversation
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
|
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:
|
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 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 The class 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 ...Speaking of, the overall per-emission calculation in .....Worse of all, I just discovered a parallel universe emitter class, Infact, I wanted to tackle your feedback today but I feel like my brains are already done for today lmao |
|
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 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. |
|
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 |
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