multiplayer: hold the authority's readings between updates, and hear the brakes

Two complaints, one cause underneath them.

The brake and squeal sounds came out in shreds as a train got going. Both are
shaped from UnitBrakeForce and Vel, and both of those are results of a physics
step. An update arrives ten times a second; in the twenty-odd frames between
them the local physics carries on computing its own speed and its own brake
force for a train it has no business simulating, they fall away, and the next
update snaps them back. rsBrake pitches on Vel and rsPisk gains on the brake
force ratio, so what the ear got was that sawtooth.

So the physics outputs of somebody else's train are now put back after every
local step, not only on the frame an update lands: speed, brake force, wheel
and engine revolutions, engine power, wheelslip, sand, and the running flags of
the converter and the compressors, which the local physics kept switching off
because the conditions it evaluates are not the ones the owner is evaluating.

The relays are the same problem wearing a different hat. MainCtrlActualPos - the
notch the resistors are actually on, as opposed to the one the handle is at -
was being stepped by the local automatic notching, off its own guesses, so the
clicks either fired at the wrong moment or never fired at all. It travels now,
together with ScndCtrlActualPos and StLinFlag, which stops the local notching
inventing anything, and every relay heard here is one that operated over there.

The brake unit keeps a sound bitfield of its own, and that is where the
accelerator and the cylinder venting live. Reading it clears it, so watching it
had to be done without consuming it: TBrake grows PeekSoundFlag(), and only the
flags that have just come up count as an event - the vehicle only clears them
when it has an accelerator sound to play, so a bit that stays lit is one event,
not one per frame. RaiseSoundFlag() hands them to the far end.

Snapshot version 5.

McZapkie/hamulce.cpp

@@ -588,6 +588,21 @@ int TBrake::GetSoundFlag()
 	return result;
 }
 
+/// Returns the accumulated SoundFlag bitfield and leaves it alone. Reading the flags is
+/// what clears them, so anything that only watches - the network layer, for one - has to
+/// look this way or it steals the sound from the vehicle it is watching.
+int TBrake::PeekSoundFlag() const
+{
+	return SoundFlag;
+}
+
+/// Adds events to the pending bitfield. Used to hand a vehicle the brake sounds its
+/// physics did not produce here because somebody else is running it.
+void TBrake::RaiseSoundFlag(int const Flags)
+{
+	SoundFlag |= Flags;
+}
+
 /// <summary>Sets the anti-slip brake target pressure.</summary>
 /// <param name="Press">Pressure [bar].</param>
 void TBrake::SetASBP(double const Press)

McZapkie/hamulce.h

@@ -481,6 +481,12 @@ class TBrake
 	virtual void ForceLeak(double const Amount);
 	/// <summary>Returns and clears the accumulated SoundFlag bitfield.</summary>
 	int GetSoundFlag();
+	/// <summary>Returns the accumulated SoundFlag bitfield without clearing it, for an
+	/// observer that must not consume what the vehicle itself is about to play.</summary>
+	int PeekSoundFlag() const;
+	/// <summary>Adds events to the pending SoundFlag bitfield, so that what happened on
+	/// another peer is heard here as well.</summary>
+	void RaiseSoundFlag(int const Flags);
 	/// <summary>Returns the current brake status flags.</summary>
 	int GetBrakeStatus() const
 	{

application/drivermode.cpp

@@ -30,6 +30,7 @@ http://mozilla.org/MPL/2.0/.
 #include "rendering/renderer.h"
 #include "utilities/Logs.h"
 #include "network/entities.h"
+#include "network/snapshot.h"
 /*
 namespace input {
 
@@ -449,6 +450,11 @@ bool driver_mode::update()
 
 	Timer::subsystem.sim_total.stop();
 
+	// what the other peers' trains are doing is theirs to decide, and the local physics
+	// has spent this frame disagreeing about it. put their readings back before anything
+	// is heard or drawn from them
+	network::hold_remote_state();
+
 	simulation::Region->update_sounds();
 	audio::renderer.update(Global.iPause ? 0.0 : deltarealtime);
 

network/snapshot.cpp

@@ -79,6 +79,11 @@ TTrack *track_by_id(uint32_t const Id)
 
 // one-shot sound events seen since the last update went out, per vehicle
 std::unordered_map<std::string, int> g_soundevents;
+// the same for the brake unit's own events, plus what was on its bitfield the last time
+// we looked - the flags are only cleared when the vehicle has an accelerator sound to
+// play, so a bit that stays lit is one event, not one per frame
+std::unordered_map<std::string, int> g_brakesoundevents;
+std::unordered_map<std::string, int> g_brakesoundseen;
 
 std::unordered_map<std::string, uint64_t> g_lastvehicles;
 std::unordered_map<std::string, uint64_t> g_lastmemcells;
@@ -221,6 +226,19 @@ struct vehicle_state
 	bool compressorlock{false};
 	bool releaser{false};
 	int32_t soundevents{0};
+	// the brake unit keeps its own event bitfield, and it drives the accelerator and the
+	// cylinder venting
+	int32_t brakesoundevents{0};
+	// outputs of the physics, not settings. a peer that does not run this train computes
+	// its own and gets them wrong, so the authority's values are what it is told to use:
+	// these are what the braking, squealing and relay sounds are built from
+	double unitbrakeforce{0.0};
+	int32_t mainctrlactual{0};
+	int32_t scndctrlactual{0};
+	bool linecontactors{false};
+	bool slippingwheels{false};
+	bool sanddose{false};
+	double enginepower{0.0};
 	int32_t emergencywarningsignal{0};
 	bool alarmchain{false};
 	double wheelrevolutions{0.0};
@@ -231,6 +249,10 @@ struct vehicle_state
 	bool aiactive{false};
 };
 
+// the authoritative reading for every train somebody else runs, held so that it can be
+// put back after each local physics step
+std::unordered_map<std::string, vehicle_state> g_remotestate;
+
 void serialize_vehicle(std::ostream &Stream, vehicle_state const &State)
 {
 	sn_utils::s_str(Stream, State.name);
@@ -273,6 +295,14 @@ void serialize_vehicle(std::ostream &Stream, vehicle_state const &State)
 	sn_utils::s_bool(Stream, State.compressorlock);
 	sn_utils::s_bool(Stream, State.releaser);
 	sn_utils::ls_int32(Stream, State.soundevents);
+	sn_utils::ls_int32(Stream, State.brakesoundevents);
+	sn_utils::ls_float64(Stream, State.unitbrakeforce);
+	sn_utils::ls_int32(Stream, State.mainctrlactual);
+	sn_utils::ls_int32(Stream, State.scndctrlactual);
+	sn_utils::s_bool(Stream, State.linecontactors);
+	sn_utils::s_bool(Stream, State.slippingwheels);
+	sn_utils::s_bool(Stream, State.sanddose);
+	sn_utils::ls_float64(Stream, State.enginepower);
 	sn_utils::ls_int32(Stream, State.warningsignal);
 	sn_utils::ls_int32(Stream, State.emergencywarningsignal);
 	sn_utils::s_bool(Stream, State.alarmchain);
@@ -330,6 +360,14 @@ vehicle_state deserialize_vehicle(std::istream &Stream)
 	state.compressorlock = sn_utils::d_bool(Stream);
 	state.releaser = sn_utils::d_bool(Stream);
 	state.soundevents = sn_utils::ld_int32(Stream);
+	state.brakesoundevents = sn_utils::ld_int32(Stream);
+	state.unitbrakeforce = sn_utils::ld_float64(Stream);
+	state.mainctrlactual = sn_utils::ld_int32(Stream);
+	state.scndctrlactual = sn_utils::ld_int32(Stream);
+	state.linecontactors = sn_utils::d_bool(Stream);
+	state.slippingwheels = sn_utils::d_bool(Stream);
+	state.sanddose = sn_utils::d_bool(Stream);
+	state.enginepower = sn_utils::ld_float64(Stream);
 	state.warningsignal = sn_utils::ld_int32(Stream);
 	state.emergencywarningsignal = sn_utils::ld_int32(Stream);
 	state.alarmchain = sn_utils::d_bool(Stream);
@@ -393,6 +431,16 @@ vehicle_state read_from(TDynamicObject const &Vehicle)
 
 	auto const events = g_soundevents.find(Vehicle.name());
 	state.soundevents = (events != g_soundevents.end() ? events->second : 0);
+	auto const brakeevents = g_brakesoundevents.find(Vehicle.name());
+	state.brakesoundevents = (brakeevents != g_brakesoundevents.end() ? brakeevents->second : 0);
+
+	state.unitbrakeforce = mover.UnitBrakeForce;
+	state.mainctrlactual = mover.MainCtrlActualPos;
+	state.scndctrlactual = mover.ScndCtrlActualPos;
+	state.linecontactors = mover.StLinFlag;
+	state.slippingwheels = mover.SlippingWheels;
+	state.sanddose = mover.SandDose;
+	state.enginepower = mover.EnginePower;
 
 	state.warningsignal = mover.WarningSignal;
 	state.emergencywarningsignal = mover.EmergencyBrakeWarningSignal;
@@ -409,6 +457,41 @@ vehicle_state read_from(TDynamicObject const &Vehicle)
 	return state;
 }
 
+// the readings the sound is built from. these are results of a physics step, and a peer
+// that is not running this train produces its own, wrong, version of them every frame -
+// which is exactly what made the brakes and the squeal come out in shreds, the pitch
+// chasing a speed that fell away between updates and snapped back on the next one.
+// so they are taken from the authority and put back after every local step
+void apply_outputs(TDynamicObject &Vehicle, vehicle_state const &State)
+{
+	auto &mover = *Vehicle.MoverParameters;
+
+	mover.V = State.velocity;
+	mover.Vel = State.velocitykmh;
+	mover.UnitBrakeForce = State.unitbrakeforce;
+	mover.nrot = State.wheelrevolutions;
+	mover.enrot = State.enginerevolutions;
+	mover.EnginePower = State.enginepower;
+	mover.SlippingWheels = State.slippingwheels;
+	mover.SandDose = State.sanddose;
+
+	// the notch the resistors are actually on, as opposed to the one the handle is at.
+	// with this coming from the authority the local automatic notching stays out of it,
+	// and every relay that clicks here is one that clicked over there
+	mover.MainCtrlActualPos = State.mainctrlactual;
+	mover.ScndCtrlActualPos = State.scndctrlactual;
+	mover.StLinFlag = State.linecontactors;
+
+	// the machinery whose sound runs for as long as it does. left to itself the local
+	// physics keeps switching these off, because the conditions it evaluates are not the
+	// ones the owner of the train is evaluating
+	mover.ConverterFlag = State.converterrunning;
+	mover.CompressorFlag = State.compressorrunning;
+	mover.PantCompFlag = State.pantcompressorrunning;
+	mover.CompressorGovernorLock = State.compressorlock;
+	mover.WarningSignal = State.warningsignal;
+}
+
 // applies everything except where the vehicle is; position is dealt with per consist,
 // because moving one vehicle of a coupled set on its own tears the couplers apart.
 // only ever called for vehicles somebody else is running
@@ -457,25 +540,20 @@ void apply_controls(TDynamicObject &Vehicle, vehicle_state const &State)
 	mover.iLights[0] = State.lights[0];
 	mover.iLights[1] = State.lights[1];
 
-	// the horn, the wheels and the engine note. these are set from a cab this peer does
-	// not have, so nothing but the result of them ever reaches it
-	mover.ConverterFlag = State.converterrunning;
-	mover.CompressorFlag = State.compressorrunning;
-	mover.PantCompFlag = State.pantcompressorrunning;
-	mover.CompressorGovernorLock = State.compressorlock;
-
 	if (mover.Hamulec != nullptr && mover.Hamulec->Releaser() != State.releaser)
 		mover.Hamulec->Releaser(State.releaser ? 1 : 0);
 
 	// the one-shot events are added to whatever this peer's own physics produced; the
 	// vehicle's sound handling plays them once and wipes the lot, as it always does
 	mover.SoundFlag |= State.soundevents;
+	if (mover.Hamulec != nullptr)
+		mover.Hamulec->RaiseSoundFlag(State.brakesoundevents);
+
+	// and the physics outputs the sounds are shaped from
+	apply_outputs(Vehicle, State);
 
-	mover.WarningSignal = State.warningsignal;
 	mover.EmergencyBrakeWarningSignal = State.emergencywarningsignal;
 	mover.AlarmChainFlag = State.alarmchain;
-	mover.nrot = State.wheelrevolutions;
-	mover.enrot = State.enginerevolutions;
 
 	for (int i = 0; i < 2; ++i)
 	{
@@ -579,6 +657,7 @@ std::string write_vehicles(bool const Full, bool const OwnedOnly)
 
 		// the one-shot events have been handed over; they must not go out twice
 		g_soundevents.erase(state.name);
+		g_brakesoundevents.erase(state.name);
 	}
 
 	if (count == 0)
@@ -622,6 +701,9 @@ void read_vehicles(std::istream &Stream, network::snapshot_mode const Mode, netw
 
 		apply_controls(*vehicle, state);
 		states.emplace(vehicle, state);
+		// kept so that the local physics can be overruled again on every frame until the
+		// next update arrives, not only on the frame it lands
+		g_remotestate[state.name] = state;
 
 		Result.worst_position_error = std::max(Result.worst_position_error, glm::length(vehicle->GetPosition() - state.position));
 	}
@@ -882,9 +964,46 @@ void network::note_sound_events(std::string const &Vehicle, int const Events)
 	g_soundevents[Vehicle] |= Events;
 }
 
+void network::note_brake_sound_events(std::string const &Vehicle, int const Events)
+{
+	// what comes in here was peeked, not consumed, so a flag the vehicle never gets round
+	// to clearing would otherwise be reported over and over. only what has just come up
+	// counts as an event
+	auto &seen = g_brakesoundseen[Vehicle];
+	int const fresh = Events & ~seen;
+	seen = Events;
+
+	if (fresh == 0)
+		return;
+
+	g_brakesoundevents[Vehicle] |= fresh;
+}
+
+void network::hold_remote_state()
+{
+	if (g_remotestate.empty())
+		return;
+
+	for (auto &entry : g_remotestate)
+	{
+		TDynamicObject *vehicle = simulation::Vehicles.find(entry.first);
+		if (vehicle == nullptr || vehicle->MoverParameters == nullptr)
+			continue;
+
+		// a train that has come under this peer's own control runs its own physics again
+		if (network::is_locally_simulated(network::Entities.id_of(entry.first)))
+			continue;
+
+		apply_outputs(*vehicle, entry.second);
+	}
+}
+
 void network::reset_snapshot_history()
 {
 	g_soundevents.clear();
+	g_brakesoundevents.clear();
+	g_brakesoundseen.clear();
+	g_remotestate.clear();
 	g_lastvehicles.clear();
 	g_lastmemcells.clear();
 	g_lastswitches.clear();

network/snapshot.h

@@ -18,7 +18,7 @@ namespace network
 {
 
 // layout of the snapshot container. a reader refuses a blob it does not understand
-constexpr uint32_t SNAPSHOT_VERSION = 4;
+constexpr uint32_t SNAPSHOT_VERSION = 5;
 
 // the sections a snapshot is built from. a reader skips over any id it does not know,
 // which is what makes the format extensible: a peer built before a section existed can
@@ -77,4 +77,15 @@ void reset_snapshot_history();
 // is called from the vehicle's own sound handling, just before it wipes them
 void note_sound_events(std::string const &Vehicle, int Events);
 
+// same, for the events the brake unit raises. those are only cleared when the vehicle
+// happens to have an accelerator sound to play, so what is passed here is peeked rather
+// than consumed and only the flags that have just appeared are taken
+void note_brake_sound_events(std::string const &Vehicle, int Events);
+
+// re-asserts, on every frame, the physics outputs of the vehicles somebody else is
+// running. an update arrives ten times a second; between them the local physics carries
+// on computing its own speed and brake force for a train it has no business simulating,
+// and the sawtooth that produces is what the brake and squeal sounds were chasing
+void hold_remote_state();
+
 } // namespace network

vehicle/DynObj.cpp

@@ -4626,6 +4626,12 @@ void TDynamicObject::RenderSounds() {
     }
 
     // yB: przyspieszacz (moze zadziala, ale dzwiek juz jest)
+    if( ( true == network::is_multiplayer() )
+     && ( MoverParameters->Hamulec != nullptr ) ) {
+        // peeked rather than read: reading the brake unit's flags is what clears them,
+        // and they belong to the vehicle, not to us
+        network::note_brake_sound_events( name(), MoverParameters->Hamulec->PeekSoundFlag() );
+    }
     if( true == bBrakeAcc ) {
         if( true == TestFlag( MoverParameters->Hamulec->GetSoundFlag(), sf_Acc ) ) {
             sBrakeAcc.play( sound_flags::exclusive );