The condition added for holding the speed uphill near a stop pushed the
cognitive complexity of control_tractive_force() to 26, over the 25
SonarCloud allows. It now lives in is_too_fast_uphill(); the behaviour
is unchanged.
adjust_desired_speed_for_braking_test runs after the target speed was
worked out and overwrites AccDesired. In its third phase it holds the
deceleration near fAccThreshold until the speed has dropped by 8 km/h.
On a 20 per mille descent that application barely holds the speed, so
the phase can last minutes, and a stop signal coming up meanwhile is
ignored. In one real-time run an E186 with 19 container wagons started
the test 1290 m before a stop signal, lost 7 km/h by the signal and
overran it by 274 m after the emergency application.
While the test runs the AI now uses whichever of the two asks for the
harder braking: the test may brake harder than the route needs, never
softer.
Measured in real time, four runs of 32 consists each on a 20 per mille
descent, on the flat and on the descent with half the rail adhesion:
the test no longer runs within 600 m of a stop signal (before: down to
414 m on this branch, and 14 m with the overrun above in a build with
other brake changes), no overruns, stops 8-15 m short of the signal 86,
117 and 83 of 128 (before 90, 117 and 78).
Within fMaxProximityDist of a stop, going uphill, the AI holds its speed
with AccDesired = min(0, AccPreferred). control_tractive_force only cut
the power uphill when AccDesired < 0, so an AccDesired of exactly 0 kept
the controller where it was. A consist that came into that range under
power kept full power and crept up to the signal.
Holding the speed uphill on the way to a stop now asks for no speeding
up: power comes off once the train is faster than AccDesired allows.
With the current brake valves this rarely shows: on a 10 per mille climb,
four real-time runs of 32 consists each, the E186 with 25 empty 408W drew
power in 0-14 samples of the last 50 m before and 0-8 after, no overruns
either way. It surfaced once the AI predicted the ESt3 brake closer to
what the valve delivers, where the same train crept to 12 km/h from 40 m
out and overran the signal; with this change it stopped short in all runs.
* fix(ai): release what the AI was holding when the player takes over
* fix(ai): keep the light selector lookup within its table, Sonar findings of the handover
- the light selector size comes from the vehicle definition (LightsList Size)
and isn't limited, while the table holds 18 positions; a bigger selector
read past the table, and so did a table index other than the front or
rear one. Only the positions the table holds are looked at now, and the
selector stays where it is for a bad table index (cpp:S2259),
- the lambdas of the handover capture what they use explicitly (cpp:S3608),
- the chained assignments resetting the headlight state on the AI taking
over are split (cpp:S1121).
* refactor(sonar): use override instead of redundant virtual (cpp:S3471)
* refactor(sonar): remove empty statements, one declaration per statement (cpp:S1116, cpp:S1659)
* refactor(sonar): drop redundant lambda return types (cpp:S3574)
* refactor(sonar): prefer using over typedef (cpp:S5416)
* refactor(sonar): use =default for trivial special members (cpp:S3490)
* refactor(sonar): use nullptr literal (cpp:S4962)
* refactor(sonar): remove redundant access specifiers (cpp:S3539)
* refactor(sonar): remove redundant casts (cpp:S1905)
* refactor(sonar): use std::scoped_lock instead of std::lock_guard (cpp:S5997)
* refactor(sonar): construct elements in place with emplace_back (cpp:S6003)
* refactor(sonar): make narrowing conversions explicit (cpp:S5276)
* refactor(sonar): mark unused parameters (cpp:S1172)
Unused names are commented out; parameters used only on some platforms
are marked [[maybe_unused]].
* refactor(sonar): declare non-mutating member functions const (cpp:S5817)
Virtual functions are left untouched so overrides keep matching.
* refactor(sonar): merge collapsible nested if statements (cpp:S1066)
Comments that preceded the inner if are kept above the merged statement.
* refactor(sonar): scope single-use variables in if init-statements (cpp:S6004)
* fix(sonar): give polymorphic base classes virtual destructors (cpp:S1235)
* refactor(sonar): use try_emplace for map insertion (cpp:S6030)
* refactor(sonar): use auto where the type is spelled out on the right (cpp:S5827)
* refactor(sonar): remove redundant parentheses (cpp:S1110)
* refactor(sonar): document intentionally empty functions (cpp:S1186)
* refactor(sonar): remove dead stores (cpp:S1854)
Side effects (e.g. LISTLINE++) are kept.
* refactor(sonar): remove unused variables, captures and private fields (cpp:S1481, cpp:S1068)
* refactor(sonar): use structured bindings for pair access (cpp:S6005)
* refactor(sonar): use range-based loops and ranges algorithms (cpp:S5566)
* refactor(sonar): split chained assignments (cpp:S1121)
* refactor(sonar): add default cases to switch statements (cpp:S3562)
* refactor(sonar): use std::size for array lengths (cpp:S7127)
* refactor(sonar): use std::ranges algorithms and std::erase(_if) (cpp:S6197)
* refactor(sonar): access static members through the class (cpp:S2209)
* refactor(sonar): add braces around nested if-else (cpp:S5261)
* refactor(sonar): avoid side effects inside typeid operands (cpp:S5279)
* refactor(sonar): drop top-level const from by-value return types (cpp:S5951)
* refactor(sonar): declare variables separately from their type definitions (cpp:S3646)
* refactor(sonar): declare loop counters in the for statement (cpp:S5955)
* refactor(sonar): rely on class template argument deduction (cpp:S6012)
* refactor(sonar): inherit base constructors instead of forwarding them (cpp:S5952)
* refactor(sonar): rename locals that shadow outer declarations (cpp:S1117)
Only local variables and definition parameters are renamed; shadowing
struct fields and enumerators in headers are left for a separate change.
* refactor(sonar): reduce enum qualification with using enum (cpp:S6177)
Applied only where no unqualified name in the block could be captured by
the introduced enumerators.
* refactor(sonar): mark converting constructors explicit (cpp:S1709)
sound_source and the gl::object conversion operators stay implicit; they
are relied upon by copy-list-initialisation and GL calls.
* refactor(sonar): initialise members in initialiser lists or declarations (cpp:S3230)
* refactor(sonar): make read-only local references and pointers const (cpp:S5350)
* refactor(sonar): take read-only pointer and reference parameters as const (cpp:S995)
TTrain command handlers keep their signature, it is fixed by the handler table.
* refactor(sonar): pass expensive-to-copy parameters by const reference (cpp:S1238)
Signatures declared in files touched by PR 206 (Driver.h, EU07.cpp) and
virtual overrides are left unchanged.
* refactor(sonar): use transparent comparators for string-keyed maps (cpp:S6045)
std::unordered_map instances and map types exposed in headers used by
other modules are left as they are.
* fix(sonar): do not use UI text as a printf format string (cpp:S5281)
Text lines containing '%' would otherwise be interpreted as format
specifiers by ImGui::TextColored.
* refactor(sonar): use std::bind_front for callbacks (cpp:S5995)
* refactor(sonar): drop const_cast on string literals passed to the Python C API (cpp:S859)
* refactor(sonar): take read-only string parameters as std::string_view (cpp:S6009)
The Windows messaging helpers keep std::string, they need c_str().
* refactor(sonar): make read-only global tables const (cpp:S5421)
Mutable global state (flags, timers, singletons) is left as is.
* refactor(sonar): replace single-case switch statements with if (cpp:S1301)
* refactor(sonar): format UUIDs and log timestamps with std::format (cpp:S6494, cpp:S5945)
* refactor(sonar): use std::array for local fixed-size arrays (cpp:S5945)
Arrays that are class members or part of headers are left unchanged.
* fix(sonar): test the bearing damage bit, not bit 0 (cpp:S872)
'DamageFlag & dtrain_bearing > 0' parsed as 'DamageFlag & (dtrain_bearing > 0)',
i.e. DamageFlag & 1, so bearing friction reacted to the wrong damage flag.
* refactor(sonar): assorted small fixes
- empty() instead of size() comparisons (cpp:S1155)
- member initialisers in declaration order (cpp:S3229)
- no redundant c_str() when a std::string is accepted (cpp:S7121)
- contains() on associative containers (cpp:S6171)
- const reference instead of a copying range-for (cpp:S6032)
- starts_with() for prefix checks (cpp:S6178)
- no std::move on temporaries (cpp:S5274)
- has_value() on std::optional (cpp:S7172)
- bool literals for bool members (cpp:S5820)
- inline only on the definition (cpp:S3548)
- explicit base access specifier (cpp:S5965)
- explicit element access instead of dereferencing an array (cpp:S7116)
- rely on the C++20 rewritten operator!= (cpp:S6186)
* refactor(sonar): more small fixes
- std::format instead of iostream manipulators in to_string/to_hex_str (cpp:S6495);
output verified identical, negative hex values keep the unsigned form
- remove an empty compound statement (cpp:S108)
- drop an always-false unsigned < 0 branch (cpp:S1768)
- explicit lambda capture (cpp:S3608)
- std::make_unique (cpp:S5950)
* chore: strip stray carriage returns left in TTrain constructor comments
* fix: keep quantize() in its own value type
The S5276 change cast the result of the generic quantize<Type_>() to
unsigned int, which truncated float/double callers (shadow angle slider,
editor rotation snapping, AI speed control steps) and broke negative values.
* fix(sonar): use integer loop counters in segment sampling (cpp:S2193)
* refactor(sonar): clean up issues reported on PR 209
- remove locals left unused after the dead-store cleanup (cpp:S1481, cpp:S1854)
- unnamed unused parameter (cpp:S1172)
- single-use declarations moved into if init-statements (cpp:S6004)
- auto for casted initialisers (cpp:S5827)
- const references and pointers where nothing is modified (cpp:S5350)
- rename structured bindings shadowed by inner declarations (cpp:S1117)
Inside the proximity range only a stop had a final approach of its own;
for a lower speed ahead that branch set nothing, so AccDesired stayed at
the preferred acceleration. With the range at 50 m, which this PR uses so
that trains stop 8-15 m short of a signal, the AI stopped braking within
the last 50 m before a speed restriction and even took power: EMUs reached
a 20 km/h restriction at 25 km/h. On master the range is 15-40 m, which
kept this mostly out of sight.
A lower speed ahead now keeps the braking curve right up to where it
begins. Stops are unchanged.
Measured in real time on current master (with #199), with a 20 km/h
restriction one coach long, two runs of 32 consists each: over 22 km/h in
the restriction on the flat 9 on master, 15 before this change, 1 after
(a 36WEa at 22.1 km/h); on the 20 per mille descent 15, 0 and 0. Stops
8-15 m short of the signal stay as they were (flat 57 and 55 of 64,
descent 44 and 42), no overruns.
- end the charging position when the control reservoir of any vehicle, the
locomotive included, rises 0.05 bar above the nominal pipe pressure
- standing and unable to depart for 10 s: allow the charging position
regardless of the equivalent pipe pressure and the train type
Resolve the conflicts with the merged nested ifs (#194) in
control_tractive_force() and control_braking_force(): the conditions keep
the new flattened form, with this branch's changes carried into them.
The function would flag the consist as not ready (`iEngineActive` set to `false`) when the brake handle was at a cutoff position. Since this is what the brake lever is exactly starting at in the EW58 train, the train when asked to move the brake lever to operating position would not listen since the `control_tractive_and_braking_force()` function which is supposed to do that is gated behind the readiness flag in the `TController::Update()` function.
This makes sure EW58 trains can be driven by AI once again instead of being stuck.
I don't really know why the brake position check was there, since it would fail only if:
- There is a brake handle
- The brake handle is in the cutoff position
- The brake handle is NOT in the operating position
The third check seems redundant but curiously there is one case where this matters and it's... MHZ_EN57. Its operating and cutoff positions are both defined as `0`, which might be a hack, since the actual cutoff position in the MHZ_EN57 handle is meant to be `6`.
When the AI takes power after a brake release, determine_consist_state()
marks the consist not Ready if any vehicle's distributor reports the lap
or application state (b_hld | b_on), whatever the speed, and the sanity
check in control_handles() then puts the master controller back to zero.
A distributor stays in the lap position until the brake pipe has fully
recharged, so a few seconds after a release, with 0.2-0.3 bar left in a
cylinder, one wagon of a train is enough to throw an EU07 or EP09 that
has just reached the 2nd-5th position back to zero; it starts again
moments later.
The check in motion was meant to be lenient (brake force over 10 kN),
but the distributor state was applied at any speed. In motion it now
counts only when the cylinder holds more than 0.4 bar, the UIC figure
already used a few lines below for a released brake. At a stand the
check is unchanged.
Measured in real time against master, two runs of each scenery (a 20 per
mille descent, flat and a 10 per mille climb with 32 consists each, and a
descent with 15 freight trains of other wagon types), with a 20 km/h
restriction one coach long: throttle-downs to zero in motion caused by
readiness drop from 13 to 1, 7 to 2, 0 to 0 and 13 to 6. Each of those
left has a wagon over 0.4 bar that is applying or lapped, or one still
releasing with more than 10 kN at walking pace. No coupler broke; signal
overruns, speeds in the restriction and rolling back on the climb
are the same as on master within run-to-run spread.
The new braking curve below plans the stop at the signal itself unless
the stopping point moves with it (in testing, trains stopped 0-3 m past
the signal), so both go in one change.
1. The braking curve.
The braking curve towards a lower speed or a stop assumed a constant
deceleration from the present speed over the whole distance. A pneumatic
brake does not work like that:
- it acts on the state the train is in once it has applied; while the
train is still speeding up (downhill), plan from the speed it will have
reached by then, over the distance left by then;
- cast iron blocks grip harder as the train slows, so a constant
deceleration plan asks for too much at the start and then releases;
follow the shape of the consist brake table (fBrake_a0/a1) over speed
instead (shaped_braking_demand), over the distance left once the brake
has had its fill time;
- no power while the train is still above the speed it is braking for.
IncBrake() only adds braking when the brake table says the present
position is not enough. When the table overestimates the consist (see
the previous rapid step fix, or a descent), the train keeps slowing too
little with the handle held. After the full fill time with the handle
unchanged, believe the train rather than the table (fBrakeHeldTime).
When the AI asks for no braking and the train is still slowing down more
than that, release. With the gentler plan above the demand eases off
early, and without this a heavy freight train in G came to a stand a few
hundred metres before a W4 with its brake still on (seen in testing).
2. The stopping window.
In train running the final approach to a stop signal began at
fMaxProximityDist (15-40 m, cargo 25-50 m), aimed at half of
fMinProximityDist (10-15 m, cargo 20-25 m), and set VelDesired = 0,
which asks for hard braking whatever the distance left. Where a train
came to a stand depended on how it entered that phase: on the test
sceneries anywhere from 1 to 45 m short of the signal on the flat and
25-42 m uphill for heavy freight.
For train running only (a W4 stop point, shunting and coupling keep the
old margins and the old final approach):
- stop 8-15 m short of the signal whatever the length of the train, aim
at the middle of that window (stopping_margin) both with the braking
curve and in the final approach, and ask there only for what the
distance left takes, following the consist brake table;
- start the final approach 50 m out (cargo 60 m) whether the train is
moving or standing; it used to be 15-40 m while moving and 50 m once
stopped, and with the gentler final approach above a train that came
to a stand between the two released to creep closer and, uphill,
rolled back and tried again;
- uphill, while the gradient alone would bring the train to a stand short
of the window, keep it rolling at no more than the speed the gradient
can take off by then, with power if needed;
- uphill, put the train brake on as the train comes to a stand, keep it
on while it stands, and apply it for real if the train rolls back. This
is done at the very end of pick_optimal_speed(): the adjustments made
before it assume a train moving forward and can raise the demand back
to "no braking" (with the rule placed earlier, EMUs rolled back up to
90 m in testing). The EMU branch of
control_braking_force() gets the "slipping back" condition the wagon
branch has always had.
3. Known limitation.
Heavy trains of wagons with the ESt3AL2 distributor (413S, 601W and
alike) respond more slowly than the AI expects: with the present brake
model the distributor fills the AL2 relay chamber, whose capacity is set
to 2 in TNESt3::SetSize (Przekladniki[i]->CreateCap(i)), through
orifices sized for about 1 l, so these wagons need 34 s to reach 95 %
cylinder pressure in G against 21 s for a plain ESt3. On a 20 per mille
descent an ET22 with 20 loaded 413S wagons overran the stop signal by up
to 6.5 m in 6 of 24 runs at x20 with this change. With the relay chamber
at 1 l and the release orifices at about 80 % of their diameter, as the
author of the brake model suggested, none of them did (0 of 16).
Two changes that only work together: the first on its own lets a heavy
freight train brake less on a descent than it needs (in testing an
ET42 with 30 loaded wagons overran the stop signal by 16-48 m at 20 per
mille), the second makes the hold on a descent what it should be.
The comparison that decides whether the AI may skip its reaction time
between two brake steps stays on the brake-only demand, as before: it
is about how much brake is asked for, and a heavy train in G approaching
a stop on a descent needs its brake built up at once. Compared in the
resultant frame, that train built up its brake one step per reaction
time and overran the signal (16 copies of the ET42 train, x20:
22 of 32 overruns with the comparison moved, 0 of 48 with it kept).
1. Compare the measured deceleration in the same frame.
Going downhill, adjust_desired_speed_for_current_speed() subtracts the
gravity component from AccDesired, so that IncBrake() sizes the handle
position for what the brake alone has to deliver. control_braking_force()
then compares that brake-only value with AbsAccS, which is the resultant
acceleration and already contains the gravity. On a 20 per mille descent
the two differ by about 0.2 m/s2, so the AI asked for more braking than
the target needed and then released, and repeated that.
Keep the subtracted part in fAccDesiredGravity and add it back wherever
AccDesired is compared with AbsAccS. It is zero everywhere else (flat,
uphill, braking test), so the behaviour there does not change.
2. Hold the speed with one continuous law.
On a descent the AI looks ahead to decide whether it will exceed the
target speed. The look-ahead was (1 - a0) * 30 / 3.6 s, a few seconds,
while a pneumatic brake in G takes tens of seconds to apply or to
release; the AI therefore acted on an estimate the train had already
left behind. The demand then jumped between the braking threshold above
the target and a lerp towards full acceleration below it, and with the
brake that slow this turned into long alternating applications and
releases.
- look ahead for half the brake application time while the train is
speeding up and half the release time while it is slowing down
(brake_response_horizon), never less than before;
- use one law on both sides of the target, proportional to the
predicted difference, and hold a band just below the limit that widens
with the gradient (pneumatic up to 3 km/h, EP/electric up to 1.5 km/h);
- once below a lower limit ahead, hold that limit instead of speeding up
towards the line speed and braking again.
* Fix the speed table index remap in TablePurger()
TablePurger() rebuilds the speed table and has to move SemNextIndex and
SemNextStopIndex along with it. Two cases came out wrong.
A trimmed point was never invalidated: the index was first overwritten with
the position the next kept point would take, and the guard that should reset
it to -1 compared the loop counter against that new value, so it only matched
when nothing had been trimmed before it. The index then pointed at an
unrelated record, and if that record happens to sit at zero velocity the
placement can no longer be updated either, because the update requires the
recorded one to be non-zero.
An index pointing at the last entry was left untouched, because the loop stops
one short of it and that entry is appended afterwards. The stale value ended
up past the end of the trimmed table, which is what the provisory check in
TableTraceRoute() catches.
The placements are now recorded separately and assigned once the new table is
complete, with the always-kept last entry handled too. The trim condition is
shared by the counting and the copying pass instead of being spelled out
twice.
TableCheck() drops entries beyond a switch that changed under the scan without
adjusting the placements either, so they are invalidated there as well. The
check in TableTraceRoute() stays as a safety net, without the TODO.
* Sort the whole speed table in TableSort()
TableSort() made a single bubble pass over the table. That only moves an entry
one place towards the front, but TableTraceRoute() adds the scanned events of a
track before the track itself, so a track carrying k scanned events lands k
places too far and needs k passes. With two or more events in one slot - a cab
signal resonator next to a signal, a signal and a W4 - the track entry stays
behind entries that are farther away, and TableUpdate() walks the table out of
distance order until enough later scans have shifted it back.
The range before the last slot is now sorted with std::stable_sort, and the
semaphor placements are moved along by looking their events up again.
---------
Co-authored-by: pwarszawski <pwarszawski@gmail.com>
When the Autorewident function is enabled, the EP fuse switch will no longer automatically engage or disengage if the player is controlling the train. This would happen for example when approaching a station.
Simplified lamp logic by reducing unnecessary brackets, `if` statements, ternary operators and boolean comparisons.
Added new lamps:
- `i-gear1` - 1st gear in the gearbox is active
- `i-gear2` - 2nd gear in the gearbox is active
- `i-hydrolockup` - The gearbox is locking the engine and wheels together
Additionally, fixed motor overload and differential relay lights not turning off when the battery is turned off.
std::uniform_real/int_distribution has UB when min>max. The old "fancy Random"
used interpolate(a,b,t) which tolerated a>b; the migration to
std::uniform_real_distribution did not, and only one inverted call site was
fixed by hand. Swap the bounds inside Random()/Random(int)/LocalRandom() so
every call site is safe, and reorder the two inverted literal calls in
Driver.cpp (fActionTime buzzer / departure delay) for readability.
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.
Shift isn't used as that would affect joystick controls and we
probably should discourage the user from roaming around in external
cam mode too much, so they won't forget that they're not in regular
free fly out of the cab.
Update Init(...) parameters to glm::dvec3, change LookAt to glm::dvec3 and adjust Camera.cpp to remove redundant glm::dvec3 casts (lookAt call and vector subtraction). These changes improve precision for camera lookAt vector in world coordinates above 4 digits
