eu07/maszyna
16
0
Fork 0
mirror of https://github.com/MaSzyna-EU07/maszyna.git synced 2026-07-17 23:39:18 +02:00
Code Issues 121 Wiki Activity

fix: restore min>max tolerance lost in clamp->std::clamp migration

Browse Source 
The dumb3d->glm / utilities-simplification refactor replaced the old custom
clamp() (which tolerated Lo>Hi, returning the upper bound) with std::clamp,
where inverted bounds are undefined behaviour. Only a few sites were patched
afterwards, by hand, with std::minmax plumbing (incl. a stray `static` inside
a per-axle loop). Others silently produced wrong results in the AI braking /
acceleration path.

Add a safe_clamp() helper (normalizes inverted bounds) and use it ONLY where
Lo<=Hi cannot be proven at compile time (config min/max pairs, sign-dependent
expressions, container underflow). Sites with constant bounds or [0, x>=0]
keep std::clamp. Remove the manual std::minmax workarounds and rewrite the
damaged-track jolt code value-first.
This commit is contained in:
maj00r
2026-07-02 00:16:03 +02:00
parent 80c3feac03
commit 03bfbb1345
4 changed files with 33 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
McZapkie/Mover.cpp
View File

@@ -6012,7 +6012,7 @@ double TMoverParameters::TractionForce(double dt)
// charakterystyka pradnicy obcowzbudnej (elipsa) - twierdzenie Pitagorasa // charakterystyka pradnicy obcowzbudnej (elipsa) - twierdzenie Pitagorasa
EngineVoltage = std::sqrt(std::abs(square(tempUmax) - square(tempUmax * Im / tempImax))) * (tempMCP - 1) + (1.0 - Im / tempImax) * tempUmax * (tempMCPN - tempMCP); EngineVoltage = std::sqrt(std::abs(square(tempUmax) - square(tempUmax * Im / tempImax))) * (tempMCP - 1) + (1.0 - Im / tempImax) * tempUmax * (tempMCPN - tempMCP);
EngineVoltage /= tempMCPN - 1; EngineVoltage /= tempMCPN - 1;
EngineVoltage = std::clamp(EngineVoltage, Im * 0.05, 1000.0 * tmp / std::abs(Im)); EngineVoltage = safe_clamp(EngineVoltage, Im * 0.05, 1000.0 * tmp / std::abs(Im));
} }
} }
@@ -7442,11 +7442,11 @@ void TMoverParameters::CheckEIMIC(double dt)
if (MainCtrlActualPos != MainCtrlPos || LastRelayTime > InitialCtrlDelay) if (MainCtrlActualPos != MainCtrlPos || LastRelayTime > InitialCtrlDelay)
{ {
eimic -= std::clamp(-UniCtrlList[MainCtrlPos].SetCtrlVal + eimic, 0.0, eimic -= safe_clamp(-UniCtrlList[MainCtrlPos].SetCtrlVal + eimic, 0.0,
MainCtrlActualPos == MainCtrlPos ? dt * UniCtrlList[MainCtrlPos].SpeedDown : sign(UniCtrlList[MainCtrlPos].SpeedDown) * 0.01); // odejmuj do X MainCtrlActualPos == MainCtrlPos ? dt * UniCtrlList[MainCtrlPos].SpeedDown : sign(UniCtrlList[MainCtrlPos].SpeedDown) * 0.01); // odejmuj do X
eimic += std::clamp(UniCtrlList[MainCtrlPos].SetCtrlVal - eimic, 0.0, eimic += safe_clamp(UniCtrlList[MainCtrlPos].SetCtrlVal - eimic, 0.0,
MainCtrlActualPos == MainCtrlPos ? dt * UniCtrlList[MainCtrlPos].SpeedUp : sign(UniCtrlList[MainCtrlPos].SpeedUp) * 0.01); // dodawaj do X MainCtrlActualPos == MainCtrlPos ? dt * UniCtrlList[MainCtrlPos].SpeedUp : sign(UniCtrlList[MainCtrlPos].SpeedUp) * 0.01); // dodawaj do X
eimic = std::clamp(eimic, UniCtrlList[MainCtrlPos].MinCtrlVal, UniCtrlList[MainCtrlPos].MaxCtrlVal); eimic = safe_clamp(eimic, UniCtrlList[MainCtrlPos].MinCtrlVal, UniCtrlList[MainCtrlPos].MaxCtrlVal);
} }
if (MainCtrlActualPos == MainCtrlPos) if (MainCtrlActualPos == MainCtrlPos)
LastRelayTime += dt; LastRelayTime += dt;
@@ -7564,13 +7564,13 @@ void TMoverParameters::CheckSpeedCtrl(double dt)
// TODO: check how to disable integral part when braking in smart way // TODO: check how to disable integral part when braking in smart way
// double factorI = eimicSpeedCtrlIntegral >= 0 ? SpeedCtrlUnit.FactorIpos : SpeedCtrlUnit.FactorIneg; // double factorI = eimicSpeedCtrlIntegral >= 0 ? SpeedCtrlUnit.FactorIpos : SpeedCtrlUnit.FactorIneg;
double factorI = eimicSpeedCtrlIntegral >= 0 ? SpeedCtrlUnit.FactorIpos : SpeedCtrlUnit.FactorIneg; double factorI = eimicSpeedCtrlIntegral >= 0 ? SpeedCtrlUnit.FactorIpos : SpeedCtrlUnit.FactorIneg;
eimicSpeedCtrlIntegral = std::clamp(eimicSpeedCtrlIntegral + factorI * eSCP * dt, -1.0 + eSCP, 1.0 - eSCP); eimicSpeedCtrlIntegral = safe_clamp(eimicSpeedCtrlIntegral + factorI * eSCP * dt, -1.0 + eSCP, 1.0 - eSCP);
} }
else else
{ {
eimicSpeedCtrlIntegral = 0; eimicSpeedCtrlIntegral = 0;
} }
auto const DesiredeimicSpeedCtrl{std::clamp(eimicSpeedCtrlIntegral + eSCP, -SpeedCtrlUnit.DesiredPower, accfactor)}; auto const DesiredeimicSpeedCtrl{safe_clamp(eimicSpeedCtrlIntegral + eSCP, -SpeedCtrlUnit.DesiredPower, accfactor)};
eimicSpeedCtrl = std::clamp(DesiredeimicSpeedCtrl, eimicSpeedCtrl - SpeedCtrlUnit.PowerDownSpeed * dt, eimicSpeedCtrl + SpeedCtrlUnit.PowerUpSpeed * dt); eimicSpeedCtrl = std::clamp(DesiredeimicSpeedCtrl, eimicSpeedCtrl - SpeedCtrlUnit.PowerDownSpeed * dt, eimicSpeedCtrl + SpeedCtrlUnit.PowerUpSpeed * dt);
if (Vel < SpeedCtrlUnit.FullPowerVelocity) if (Vel < SpeedCtrlUnit.FullPowerVelocity)
{ {
@@ -8183,7 +8183,7 @@ double TMoverParameters::dizel_Momentum(double dizel_fill, double n, double dt)
enMoment = 0; enMoment = 0;
double enrot_min = enrot - (std::min(TorqueC, TorqueL + abs(hydro_TC_TorqueIn)) - Moment) / dizel_AIM * dt; double enrot_min = enrot - (std::min(TorqueC, TorqueL + abs(hydro_TC_TorqueIn)) - Moment) / dizel_AIM * dt;
double enrot_max = enrot + (std::min(TorqueC, TorqueL + abs(hydro_TC_TorqueIn)) + Moment) / dizel_AIM * dt; double enrot_max = enrot + (std::min(TorqueC, TorqueL + abs(hydro_TC_TorqueIn)) + Moment) / dizel_AIM * dt;
enrot = std::clamp(n, enrot_min, enrot_max); enrot = safe_clamp(n, enrot_min, enrot_max);
} }
if (hydro_R && hydro_R_Placement == 1) if (hydro_R && hydro_R_Placement == 1)
gearMoment -= dizel_MomentumRetarder(hydro_TC_nOut, dt); gearMoment -= dizel_MomentumRetarder(hydro_TC_nOut, dt);
@@ -8564,7 +8564,7 @@ bool TMoverParameters::ChangeDoorPermitPreset(int const Change, range_t const No
if (false == Doors.permit_presets.empty()) if (false == Doors.permit_presets.empty())
{ {
Doors.permit_preset = std::clamp(Doors.permit_preset + Change, 0, static_cast<int>(Doors.permit_presets.size() - 1)); Doors.permit_preset = safe_clamp(Doors.permit_preset + Change, 0, static_cast<int>(Doors.permit_presets.size() - 1));
auto const doors{Doors.permit_presets[Doors.permit_preset]}; auto const doors{Doors.permit_presets[Doors.permit_preset]};
auto const permitleft{((doors & 1) != 0)}; auto const permitleft{((doors & 1) != 0)};
auto const permitright{((doors & 2) != 0)}; auto const permitright{((doors & 2) != 0)};

13
utilities/utilities.h
View File

@@ -248,6 +248,19 @@ template <typename T> bool is_equal(T const &Left, T const &Right, T const Epsil
return Left == Right; return Left == Right;
} }
// Tolerant clamp. Unlike std::clamp, this does NOT invoke undefined behaviour when
// the bounds are inverted (Lo > Hi) - it normalizes them instead. This restores the
// pre-refactor behaviour of the old custom clamp() for code paths where the bounds are
// computed at runtime and can legitimately cross (e.g. AI acceleration/braking limits,
// physics jolt calculations), where std::clamp's UB produced wrong results.
template <typename Type_>
constexpr Type_ safe_clamp( Type_ const Value, Type_ const Lo, Type_ const Hi )
{
return ( Hi < Lo )
? std::clamp( Value, Hi, Lo )
: std::clamp( Value, Lo, Hi );
}
// keeps the provided value in specified range 0-Range, as if the range was circular buffer // keeps the provided value in specified range 0-Range, as if the range was circular buffer
template <typename T> T clamp_circular(T Value, T const Range = T(360)) template <typename T> T clamp_circular(T Value, T const Range = T(360))
{ {

7
vehicle/Driver.cpp
View File

@@ -3701,7 +3701,7 @@ bool TController::IncSpeedEIM() {
// TBD, TODO: set position based on desired acceleration? // TBD, TODO: set position based on desired acceleration?
OK = mvControlling->MainCtrlPos < mvControlling->MainCtrlPosNo; OK = mvControlling->MainCtrlPos < mvControlling->MainCtrlPosNo;
if( OK ) { if( OK ) {
mvControlling->MainCtrlPos = std::clamp( mvControlling->MainCtrlPos + 1, 6, mvControlling->MainCtrlPosNo ); mvControlling->MainCtrlPos = safe_clamp( mvControlling->MainCtrlPos + 1, 6, mvControlling->MainCtrlPosNo );
} }
*/ */
break; break;
@@ -7669,7 +7669,7 @@ TController::adjust_desired_speed_for_current_speed() {
if( iVehicles - ControlledEnginesCount > 0 ) { if( iVehicles - ControlledEnginesCount > 0 ) {
MaxAcc *= std::clamp( vel * 0.025, 0.2, 1.0 ); MaxAcc *= std::clamp( vel * 0.025, 0.2, 1.0 );
} }
AccDesired = std::min(AccDesired, std::clamp(MaxAcc, HeavyCargoTrainAcceleration, AccPreferred)); AccDesired = std::min(AccDesired, safe_clamp(MaxAcc, HeavyCargoTrainAcceleration, AccPreferred));
// TBD: expand this behaviour to all trains with car(s) exceeding certain weight? // TBD: expand this behaviour to all trains with car(s) exceeding certain weight?
/* /*
if( ( IsPassengerTrain ) && ( iVehicles - ControlledEnginesCount > 0 ) ) { if( ( IsPassengerTrain ) && ( iVehicles - ControlledEnginesCount > 0 ) ) {
@@ -7770,8 +7770,7 @@ TController::adjust_desired_speed_for_braking_test() {
break; break;
} }
case 3: { case 3: {
auto [minV, maxV] = std::minmax(fAccThreshold * 1.01f, fAccThreshold * 1.21f); AccDesired = safe_clamp( -AbsAccS, fAccThreshold * 1.01, fAccThreshold * 1.21 );
AccDesired = std::clamp(-AbsAccS, minV, maxV);
VelDesired = DBT_VelocityBrake; VelDesired = DBT_VelocityBrake;
if( vel <= DBT_VelocityRelease ) { if( vel <= DBT_VelocityRelease ) {
DynamicBrakeTest = 4; DynamicBrakeTest = 4;

20
vehicle/DynObj.cpp
View File

@@ -3014,7 +3014,7 @@ TDynamicObject::update_load_offset() {
0.0 : 0.0 :
100.0 * MoverParameters->LoadAmount / MoverParameters->MaxLoad ) }; 100.0 * MoverParameters->LoadAmount / MoverParameters->MaxLoad ) };
LoadOffset = std::lerp( MoverParameters->LoadType.offset_min, 0.f, std::clamp( 0.0, loadpercentage * 0.01, 1.0 ) ); LoadOffset = std::lerp( MoverParameters->LoadType.offset_min, 0.f, safe_clamp( 0.0, loadpercentage * 0.01, 1.0 ) );
} }
void void
@@ -3453,7 +3453,7 @@ bool TDynamicObject::Update(double dt, double dt1)
else else
p->MoverParameters->MED_EPVC_CurrentTime += dt1; p->MoverParameters->MED_EPVC_CurrentTime += dt1;
bool EPVC = p->MoverParameters->MED_EPVC && (p->MoverParameters->MED_EPVC_Time < 0 || p->MoverParameters->MED_EPVC_CurrentTime < p->MoverParameters->MED_EPVC_Time); bool EPVC = p->MoverParameters->MED_EPVC && (p->MoverParameters->MED_EPVC_Time < 0 || p->MoverParameters->MED_EPVC_CurrentTime < p->MoverParameters->MED_EPVC_Time);
float VelC = EPVC ? std::clamp(p->MoverParameters->Vel, p->MoverParameters->MED_Vmin, p->MoverParameters->MED_Vmax) : p->MoverParameters->MED_Vref;//korekcja EP po prędkości float VelC = EPVC ? safe_clamp(p->MoverParameters->Vel, p->MoverParameters->MED_Vmin, p->MoverParameters->MED_Vmax) : p->MoverParameters->MED_Vref;//korekcja EP po prędkości
float FmaxPoj = Nmax * float FmaxPoj = Nmax *
p->MoverParameters->Hamulec->GetFC( p->MoverParameters->Hamulec->GetFC(
Nmax / (p->MoverParameters->NAxles * p->MoverParameters->NBpA), VelC) * Nmax / (p->MoverParameters->NAxles * p->MoverParameters->NBpA), VelC) *
@@ -3647,22 +3647,20 @@ bool TDynamicObject::Update(double dt, double dt1)
// crude bump simulation, drop down on even axles, move back up on // crude bump simulation, drop down on even axles, move back up on
// the odd ones // the odd ones
// MoverParameters->AccVert += (MoverParameters->Vel*0.1f) * // MoverParameters->AccVert += (MoverParameters->Vel*0.1f) *
static double minV, maxV; double const headroom = std::clamp(
std::tie(minV, maxV) = std::minmax(MoverParameters->Vmax, MoverParameters->Vmax - (MoverParameters->Vel + MoverParameters->Vmax * 0.32f)); MoverParameters->Vmax - ( MoverParameters->Vel + MoverParameters->Vmax * 0.32f ),
double precalculatedValue = std::clamp(0.0, minV, maxV) * .05f * (MyTrack->iDamageFlag * 0.25f); 0.0, MoverParameters->Vmax );
double const jolt = headroom * .05f * ( MyTrack->iDamageFlag * 0.25f );
if (MyTrack->eType == tt_Normal) if (MyTrack->eType == tt_Normal)
{ {
std::tie(minV, maxV) = std::minmax(4.0, precalculatedValue); MoverParameters->AccVert += std::clamp( jolt, 0.0, 4.0 );
MoverParameters->AccVert += std::clamp(0.0, minV, maxV);
} }
else if (MyTrack->eType == tt_Switch) else if (MyTrack->eType == tt_Switch)
{ {
std::tie(minV, maxV) = std::minmax(1.0, precalculatedValue); double const accHorizontal = std::clamp( jolt, 0.0, 1.0 ) * ((axleindex % 2) != 0 ? 1 : -1);
double accHorizontal = std::clamp(0.0, minV, maxV) * (axleindex % 2 != 0 ? 1 : -1);
MoverParameters->AccS += accHorizontal; MoverParameters->AccS += accHorizontal;
MoverParameters->AccN += accHorizontal; MoverParameters->AccN += accHorizontal;
std::tie(minV, maxV) = std::minmax(2.0, precalculatedValue); MoverParameters->AccVert += std::clamp( jolt, 0.0, 2.0 );
MoverParameters->AccVert += std::clamp(0.0, minV, maxV);
} }
} }
} }