-
Notifications
You must be signed in to change notification settings - Fork 737
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
Advanced Ballistics - Drag model revamp & Spin drift correction #5566
Conversation
- Moved away from using the drag tables from the GNU exterior ballistics library - The drag functions are now based off this data from JBM Ballistics: http://www.jbmballistics.com/ballistics/downloads/text/ - The differences are minor, but some players might still appreciate the additional authenticity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I trust what you are doing.
@@ -29,7 +29,8 @@ private _distance = 0; | |||
while {_velocity > _thresholdVelocity} do { | |||
private _bc = GVAR(targetSolutionInput) select 14; | |||
private _dragModel = GVAR(targetSolutionInput) select 15; | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3", _dragModel, _bc, _velocity])); | |||
private _temperature = GVAR(targetSolutionInput) select 5; | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3:%4", _dragModel, _bc, _velocity, _temperature])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace after parseNumber
and format
.
@@ -118,7 +118,7 @@ while {_TOF < 15 && (_bulletPos select 1) < _targetRange} do { | |||
_trueSpeed = vectorMagnitude _trueVelocity; | |||
|
|||
if (missionNamespace getVariable [QEGVAR(advanced_ballistics,enabled), false]) then { | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3", _dragModel, _bc, _trueSpeed])); | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3:%4", _dragModel, _bc, _trueSpeed, _temperature])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
@@ -103,7 +103,7 @@ while {_TOF < 6 && (_bulletPos select 1) < _targetRange} do { | |||
_trueSpeed = vectorMagnitude _trueVelocity; | |||
|
|||
if (_useABConfig) then { | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3", _dragModel, _bc, _trueSpeed])); | |||
private _drag = parseNumber(("ace_advanced_ballistics" callExtension format["retard:%1:%2:%3:%4", _dragModel, _bc, _trueSpeed, _temperature])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same.
…ation - The error was introduced with this PR (#4708)
double A = -1; | ||
double M = -1; | ||
double calculateRetard(int DragFunction, double DragCoefficient, double Velocity, double Mach) { | ||
std::vector<double> machNumbers = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance optimization would be to have these tables be built at compile-time.
Or making them global variables and just selecting a pointer to them here.
Because this way you have a memory allocation and memcpy everytime a drag function is choosen.
for (int i = 0; i < machNumbers.size(); i++) { | ||
if (machNumbers[i] >= m) { | ||
int previousIdx = std::max(0, i - 1); | ||
double lcd = dragCoefficients[previousIdx]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
longer variable names would be neat here. I have no idea what lcd/lmn/cd mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Even though all we do here is a linear interpolation between the data points.
@@ -309,7 +249,7 @@ double calculateZeroAngle(double zeroRange, double muzzleVelocity, double boreHe | |||
|
|||
v = std::sqrt(vx*vx + vy*vy); | |||
|
|||
double retard = calculateRetard(dragModel, ballisticCoefficient, v); | |||
double retard = calculateRetard(dragModel, ballisticCoefficient, v, SPEED_OF_SOUND(15)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are calculating this constant.. at runtime. Everytime the function is called. Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RELEVANT VARIABLE NAME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dedmen, I expect it to be optimized away by the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't be here. because std::sqrt is not constexpr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC 7.2.0 is smart enough. I will test VS 2017 tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visual Studio 2017 produces the exact same assembly code in both cases:
#define SPEED_OF_SOUND(t) (331.3 + std::sqrt(1 + t / 273.15f))
...
double r = calcR(dragModel, ballisticCoefficient, v, SPEED_OF_SOUND(15));
#define SPEED_OF_SOUND(t) (331.3 + std::sqrt(1 + t / 273.15f))
...
const float c = SPEED_OF_SOUND(15);
double r = calcR(dragModel, ballisticCoefficient, v, c);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moving it into a const float doesn't change that std::sqrt is not constexpr before C++17.
Actually.. Not even C++17 has constexpr sqrt.
Does that assembly code that it produces calculate the Speed of sound. Or does it use a constant? If it calculates then it does exactly what I said
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did a release compile. And yes it is calling sqrt at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will just replace it with 340.275
.
const std::vector<double> machNumbersG8 = { 0.00, 0.05, 0.10, 0.15, 0.20, 0.25, 0.30, 0.35, 0.40, 0.45, 0.50, 0.55, 0.60, 0.65, 0.70, 0.75, 0.80, 0.825, 0.85, 0.875, 0.90, 0.925, 0.95, 0.975, 1.0, 1.025, 1.05, 1.075, 1.10, 1.125, 1.15, 1.20, 1.25, 1.30, 1.35, 1.40, 1.45, 1.50, 1.55, 1.60, 1.65, 1.70, 1.75, 1.80, 1.85, 1.90, 1.95, 2.00, 2.05, 2.10, 2.15, 2.20, 2.25, 2.30, 2.35, 2.40, 2.45, 2.50, 2.60, 2.70, 2.80, 2.90, 3.00, 3.10, 3.20, 3.30, 3.40, 3.50, 3.60, 3.70, 3.80, 3.90, 4.00, 4.20, 4.40, 4.60, 4.80, 5.00 }; | ||
|
||
const std::vector<std::vector<double>> dragCoefficients = { {}, dragCoefficientsG1, dragCoefficientsG2, {}, {}, dragCoefficientsG5, dragCoefficientsG6, dragCoefficientsG7, dragCoefficientsG8, {} }; | ||
const std::vector<std::vector<double>> machNumbers = { {}, machNumbersG1, machNumbersG2, {},{}, machNumbersG5, machNumbersG6, machNumbersG7, machNumbersG8,{} }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I completly forgot that. std::array is constexpr and would be a little better here.
You could then also use dragCoefficients.count() in line 137 instead of the hardcoded number. In case we ever get a G9 or smth.
(dragCoefficients.count()
with vector is a bad idea though. Because that would be evaluated at runtime.)
It wouldn't make a noticeable runtime difference I think. It would safe some time when the extension is called first time and a little memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Premature optimization is the root of all evil.
- Small performance improvement
Ready to use, if you ask me. |
@@ -20,7 +20,7 @@ | |||
TRACE_10("firedEH:",_unit, _weapon, _muzzle, _mode, _ammo, _magazine, _projectile, _vehicle, _gunner, _turret); | |||
|
|||
// Parameterization | |||
private ["_abort", "_AmmoCacheEntry", "_WeaponCacheEntry", "_opticsName", "_opticType", "_bulletTraceVisible", "_temperature", "_barometricPressure", "_bulletMass", "_bulletLength", "_muzzleVelocity", "_muzzleVelocityShift", "_bulletVelocity", "_bulletLength", "_barrelTwist", "_stabilityFactor", "_aceTimeSecond", "_barrelVelocityShift", "_ammoTemperatureVelocityShift"]; | |||
private ["_abort", "_AmmoCacheEntry", "_WeaponCacheEntry", "_opticsName", "_opticType", "_bulletTraceVisible", "_temperature", "_barometricPressure", "_bulletMass", "_bulletLength", "_muzzleVelocity", "_muzzleVelocityShift", "_bulletVelocity", "_bulletLength", "_barrelTwist", "_stabilityFactor", "_barrelVelocityShift", "_ammoTemperatureVelocityShift"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private
keyword would be faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private _abort = true;
is evaluated at compile-time.
private ARRAY
is runtime.
Also private ARRAY assigns nil to that variable. and later when you initialize it you assign again. so using private ARRAY you assign a value to the variable one extra time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can try to fix that in a separate PR. Feel free to make a GitHub issue for it.
But I think we're using private ARRAY
in many places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun-fact. using private keyword is also faster than not using private at all.
using private ARRAY however is slower than not privatizing variables. on average.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.. I'll make a PR that changes it everywhere where appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private ARRAY
is only in use because some components haven't gotten the love of cleanup yet since private VAR = VALUE
was introduced.
@@ -33,7 +31,7 @@ private _aceTimeSecond = floor CBA_missionTime; | |||
drop ["\A3\data_f\ParticleEffects\Universal\Refract","","Billboard",1,0.1,getPos _bullet,[0,0,0],0,1.275,1,0,[0.02*_caliber,0.01*_caliber],[[0,0,0,0.65],[0,0,0,0.2]],[1,0],0,0,"","",""]; | |||
}; | |||
|
|||
_bullet setVelocity (_bulletVelocity vectorAdd (parseSimpleArray ("ace_advanced_ballistics" callExtension format["simulate:%1:%2:%3:%4:%5:%6:%7", _index, _bulletVelocity, _bulletPosition, ACE_wind, ASLToATL(_bulletPosition) select 2, _aceTimeSecond, CBA_missionTime - _aceTimeSecond]))); | |||
_bullet setVelocity (_bulletVelocity vectorAdd (parseSimpleArray ("ace_advanced_ballistics" callExtension format["simulate:%1:%2:%3:%4:%5:%6:%7", _index, _bulletVelocity, _bulletPosition, ACE_wind, ASLToATL(_bulletPosition) select 2, CBA_missionTime toFixed 6]))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor error. format still expects 7 elements but you only give 6. Doesn't cause any runtime error and the extension doesn't care. So not a problem.
I predict bullet drops in Excel using Point-Mass formulas and my results are the same as Litz Advanced Ballistics software. Good thing is that I can turn it into a Monte Carlo simulation by adding randomness to muzzle velocity. If I also add controlled randomness to range estimation and consecutive drop error (simulating AI) then, I have a WEZ simulation, just like Litz. So, my suggestion is the inclusion a new ACE parameter: Anyway, My key parameter for trajectory simulation are:
This is an example t [s] | t lag [s] | vx [m/s] | vy [m/s] | vz [m/s] | v [m/s] | M | CD | FD [N] | ax [m/s2] | ay [m/s2] | az [m/s2] | x [m] | y [m] | z [m] 0.0000 | 0.0000 | 834.45 | 11.78 | 11.01 | 834.60 | 2.45 | 0.2725 | 0.000760 | -529.40 | -17.28 | -6.99 | 0.00 | 0.00 | 0.00 |
I mean, it is important to predict exact results in Excel, because it means we are in control here. Another important feature I think would be nice to share is the calculation of air friction. You can take all the values of my mod if you want. BTW, why don't you add all the ammo to ACE? I have even made the icons. Just wait one week and I will release an update. I have simulated all ammo, one-by-one in Litz Point-Mass Solver and then took note of TOF to reach 408 m/s, which is the start of the transonic region. Then I have used Excel's solver on my drop calculator to tell me which BI AirFriction value makes that bullet reach 408 m/s in the noted TOF. This way I know exactly the realistic AirFriction/Drag value required to decelerate the bullet. |
Right now our SQF code is a bigger performance problem than the ballistics calculation inside the extension. |
You can try to increase the simulation step and the gravity factor to
counter balance.
<http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
Virus-free.
www.avg.com
<http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
…On Tue, Oct 24, 2017 at 3:38 PM, Dedmen Miller ***@***.***> wrote:
Right now our SQF code is a bigger performance problem than the ballistics
calculation inside the extension.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5566 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AMAZnL9P3OqAIFQtlmb6-cpnLd0BV2CDks5svehhgaJpZM4PnCoU>
.
--
|
@QuickDagger, our
We would also needs guns that are able to shoot it. I think it makes more sense to deliver the ammo classes as part of 3rd party weapon packs. At least until magazine groups are working.
Your ammo will work perfectly fine with ACE3, if you add those config entries: https://ace3mod.com/wiki/framework/advanced-ballistics-framework.html If you have any questions or suggestions, please consider joining the discussion on: https://ace3public.slack.com/messages/C04CLGZCK/ |
The differences are minor, but some players might still appreciate the additional authenticity
Many thanks to @dedmen, @brainslush and @Laid3acK for the extensive help.