Decouple SI extraction from Animation::Loader into reusable SIReader

Extract generic SI file reading and audio extraction from
Multiplayer::Animation::Loader into a new Multiplayer::SIReader class.
This eliminates the coupling where NetworkManager reached into the
Animation namespace to extract horn WAV data, and removes the wasteful
intermediate horn cache (LegoCacheSound::Create copies PCM data, so the
cache served no purpose after template creation).

- New Multiplayer::SIReader owns SI file handle, header-only reading,
  lazy object loading, and audio track extraction
- New Multiplayer::AudioTrack struct (moved from SceneAnimData::AudioTrack)
- Animation::Loader delegates SI access via SetSIReader() pointer
- NetworkManager owns SIReader, passes it to Loader, uses it directly
  for horn sound extraction via ExtractFirstAudio()
- Consolidate duplicate horn vehicle arrays into single g_hornVehicles[]
- Move HornMsg next to EmoteMsg in protocol (both one-shot broadcasts)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

CMakeLists.txt

@@ -563,6 +563,7 @@ if (ISLE_EXTENSIONS)
     extensions/src/multiplayer/networkmanager.cpp
     extensions/src/multiplayer/protocol.cpp
     extensions/src/multiplayer/remoteplayer.cpp
+    extensions/src/multiplayer/sireader.cpp
     extensions/src/multiplayer/worldstatesync.cpp
   )
   if(EMSCRIPTEN)

extensions/include/extensions/multiplayer/animation/loader.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "extensions/multiplayer/sireader.h"
 #include "mxcriticalsection.h"
 #include "mxthread.h"
 #include "mxwavepresenter.h"
@@ -15,8 +16,6 @@ class LegoAnim;
 
 namespace si
 {
-class File;
-class Interleaf;
 class Object;
 } // namespace si
 
@@ -27,14 +26,7 @@ struct SceneAnimData {
 	LegoAnim* anim;
 	float duration;
 
-	struct AudioTrack {
-		MxU8* pcmData;
-		MxU32 pcmDataSize;
-		MxWavePresenter::WaveFormat format;
-		std::string mediaSrcPath;
-		int32_t volume;
-		uint32_t timeOffset;
-	};
+	using AudioTrack = Multiplayer::AudioTrack;
 	std::vector<AudioTrack> audioTracks;
 
 	struct PhonemeTrack {
@@ -69,22 +61,18 @@ struct SceneAnimData {
 	void ReleaseTracks();
 };
 
-// Loads animation data from ISLE.SI on demand, bypassing the streaming pipeline.
-// Reads only the RIFF header + offset table on first open, then seeks to
-// individual objects as requested.
+// Loads animation data from ISLE.SI on demand.
+// Delegates SI file access to a SIReader instance.
 class Loader {
 public:
 	Loader();
 	~Loader();
 
-	bool OpenSI();
+	void SetSIReader(SIReader* p_reader) { m_reader = p_reader; }
+
 	SceneAnimData* EnsureCached(uint32_t p_objectId);
 	void PreloadAsync(uint32_t p_objectId);
 
-	// Extract just the first WAV audio track from a composite SI object.
-	// Used for horn sounds from dashboard composites (which have no animation).
-	SceneAnimData::AudioTrack* EnsureHornCached(uint32_t p_objectId);
-
 private:
 	class PreloadThread : public MxThread {
 	public:
@@ -96,19 +84,13 @@ class Loader {
 		uint32_t m_objectId;
 	};
 
-	static bool OpenSIHeaderOnly(const char* p_siPath, si::File*& p_file, si::Interleaf*& p_interleaf);
-	bool ReadObject(uint32_t p_objectId);
 	static bool ParseAnimationChild(si::Object* p_child, SceneAnimData& p_data);
-	static bool ParseSoundChild(si::Object* p_child, SceneAnimData& p_data);
 	static bool ParsePhonemeChild(si::Object* p_child, SceneAnimData& p_data);
 	static bool ParseComposite(si::Object* p_composite, SceneAnimData& p_data);
 	void CleanupPreloadThread();
 
-	si::File* m_siFile;
-	si::Interleaf* m_interleaf;
-	bool m_siReady;
+	SIReader* m_reader;
 	std::map<uint32_t, SceneAnimData> m_cache;
-	std::map<uint32_t, SceneAnimData::AudioTrack> m_hornCache;
 	MxCriticalSection m_cacheCS;
 
 	PreloadThread* m_preloadThread;

extensions/include/extensions/multiplayer/networkmanager.h

@@ -9,6 +9,7 @@
 #include "extensions/multiplayer/platformcallbacks.h"
 #include "extensions/multiplayer/protocol.h"
 #include "extensions/multiplayer/remoteplayer.h"
+#include "extensions/multiplayer/sireader.h"
 #include "extensions/multiplayer/worldstatesync.h"
 #include "mxcore.h"
 #include "mxtypes.h"
@@ -201,6 +202,9 @@ class NetworkManager : public MxCore {
 	uint8_t m_lastVehicleState;
 	bool m_wasInRestrictedArea;
 
+	// SI file reader (shared with animation loader)
+	SIReader m_siReader;
+
 	// NPC animation playback
 	Multiplayer::Animation::Catalog m_animCatalog;
 	Multiplayer::Animation::Loader m_animLoader;

extensions/include/extensions/multiplayer/protocol.h

@@ -149,6 +149,12 @@ struct EmoteMsg {
 	uint8_t emoteId; // Index into emote table
 };
 
+// One-shot horn sound trigger, broadcast to all peers
+struct HornMsg {
+	MessageHeader header;
+	uint8_t vehicleType; // VehicleType enum value
+};
+
 // Immediate customization change, broadcast to all peers
 struct CustomizeMsg {
 	MessageHeader header;
@@ -197,12 +203,6 @@ struct AnimCompletionParticipant {
 	char displayName[8]; // 7 chars + null
 };
 
-// One-shot horn sound trigger, broadcast to all peers
-struct HornMsg {
-	MessageHeader header;
-	uint8_t vehicleType; // VehicleType enum value
-};
-
 // Host -> All: animation completed successfully (natural completion only, not cancellation)
 struct AnimCompleteMsg {
 	MessageHeader header;

extensions/include/extensions/multiplayer/sireader.h

@@ -0,0 +1,66 @@
+#pragma once
+
+#include "mxcriticalsection.h"
+#include "mxwavepresenter.h"
+
+#include <cstdint>
+#include <string>
+
+namespace si
+{
+class File;
+class Interleaf;
+class Object;
+} // namespace si
+
+namespace Multiplayer
+{
+
+struct AudioTrack {
+	MxU8* pcmData;
+	MxU32 pcmDataSize;
+	MxWavePresenter::WaveFormat format;
+	std::string mediaSrcPath;
+	int32_t volume;
+	uint32_t timeOffset;
+};
+
+// Reads objects from an SI archive on demand, bypassing the streaming pipeline.
+// Reads only the RIFF header + offset table on first open, then seeks to
+// individual objects as requested.
+class SIReader {
+public:
+	SIReader();
+	~SIReader();
+
+	// Open isle.si with header-only read (lazy object loading)
+	bool Open();
+
+	// Open any SI file with header-only read (for background threads with independent handles)
+	static bool OpenHeaderOnly(const char* p_siPath, si::File*& p_file, si::Interleaf*& p_interleaf);
+
+	// Lazy-load a single SI object by index
+	bool ReadObject(uint32_t p_objectId);
+
+	// Get a previously loaded object (must call ReadObject first)
+	si::Object* GetObject(uint32_t p_objectId);
+
+	// Extract WAV audio from a single SI child object
+	static bool ExtractAudioTrack(si::Object* p_child, AudioTrack& p_out);
+
+	// Extract the first WAV child from a composite SI object.
+	// Opens SI if needed, reads the object, finds first WAV child, extracts audio.
+	// Caller owns the returned pointer and its pcmData buffer.
+	AudioTrack* ExtractFirstAudio(uint32_t p_objectId);
+
+	bool IsReady() const { return m_siReady; }
+	si::Interleaf* GetInterleaf() { return m_interleaf; }
+
+private:
+	si::File* m_siFile;
+	si::Interleaf* m_interleaf;
+	bool m_siReady;
+	MxCriticalSection m_cs;
+};
+
+} // namespace Multiplayer

extensions/src/multiplayer/animation/loader.cpp

@@ -1,14 +1,11 @@
 #include "extensions/multiplayer/animation/loader.h"
 
 #include "anim/legoanim.h"
-#include "extensions/common/pathutils.h"
 #include "flic.h"
 #include "misc/legostorage.h"
 #include "mxautolock.h"
-#include "mxwavepresenter.h"
 
 #include <SDL3/SDL_stdinc.h>
-#include <file.h>
 #include <interleaf.h>
 
 using namespace Multiplayer::Animation;
@@ -95,77 +92,13 @@ SceneAnimData& SceneAnimData::operator=(SceneAnimData&& p_other) noexcept
 	return *this;
 }
 
-Loader::Loader()
-	: m_siFile(nullptr), m_interleaf(nullptr), m_siReady(false), m_preloadThread(nullptr), m_preloadObjectId(0),
-	  m_preloadDone(false)
+Loader::Loader() : m_reader(nullptr), m_preloadThread(nullptr), m_preloadObjectId(0), m_preloadDone(false)
 {
 }
 
 Loader::~Loader()
 {
 	CleanupPreloadThread();
-	for (auto& [id, track] : m_hornCache) {
-		delete[] track.pcmData;
-	}
-	delete m_interleaf;
-	delete m_siFile;
-}
-
-bool Loader::OpenSIHeaderOnly(const char* p_siPath, si::File*& p_file, si::Interleaf*& p_interleaf)
-{
-	p_file = new si::File();
-
-	MxString path;
-	if (!Extensions::Common::ResolveGamePath(p_siPath, path) || !p_file->Open(path.GetData(), si::File::Read)) {
-		delete p_file;
-		p_file = nullptr;
-		return false;
-	}
-
-	p_interleaf = new si::Interleaf();
-	if (p_interleaf->Read(p_file, si::Interleaf::HeaderOnly) != si::Interleaf::ERROR_SUCCESS) {
-		delete p_interleaf;
-		p_interleaf = nullptr;
-		p_file->Close();
-		delete p_file;
-		p_file = nullptr;
-		return false;
-	}
-
-	return true;
-}
-
-bool Loader::OpenSI()
-{
-	if (m_siReady) {
-		return true;
-	}
-
-	if (!OpenSIHeaderOnly("\\lego\\scripts\\isle\\isle.si", m_siFile, m_interleaf)) {
-		return false;
-	}
-
-	m_siReady = true;
-	return true;
-}
-
-bool Loader::ReadObject(uint32_t p_objectId)
-{
-	if (!m_siReady) {
-		return false;
-	}
-
-	size_t childCount = m_interleaf->GetChildCount();
-	if (p_objectId >= childCount) {
-		return false;
-	}
-
-	si::Object* obj = static_cast<si::Object*>(m_interleaf->GetChildAt(p_objectId));
-	if (obj->type() != si::MxOb::Null) {
-		return true;
-	}
-
-	return m_interleaf->ReadObject(m_siFile, p_objectId) == si::Interleaf::ERROR_SUCCESS;
 }
 
 bool Loader::ParseAnimationChild(si::Object* p_child, SceneAnimData& p_data)
@@ -212,49 +145,6 @@ bool Loader::ParseAnimationChild(si::Object* p_child, SceneAnimData& p_data)
 	return true;
 }
 
-bool Loader::ParseSoundChild(si::Object* p_child, SceneAnimData& p_data)
-{
-	auto& chunks = p_child->data_;
-	if (chunks.size() < 2) {
-		return false;
-	}
-
-	// data_[0] = WaveFormat header, data_[1..N] = raw PCM blocks
-	const auto& header = chunks[0];
-	if (header.size() < sizeof(MxWavePresenter::WaveFormat)) {
-		return false;
-	}
-
-	SceneAnimData::AudioTrack track;
-	SDL_memcpy(&track.format, header.data(), sizeof(MxWavePresenter::WaveFormat));
-	track.pcmData = nullptr;
-	track.pcmDataSize = 0;
-	track.volume = (int32_t) p_child->volume_;
-	track.timeOffset = p_child->time_offset_;
-	track.mediaSrcPath = p_child->filename_;
-
-	MxU32 totalPcm = 0;
-	for (size_t i = 1; i < chunks.size(); i++) {
-		totalPcm += (MxU32) chunks[i].size();
-	}
-
-	if (totalPcm == 0) {
-		return false;
-	}
-
-	track.pcmData = new MxU8[totalPcm];
-	track.pcmDataSize = totalPcm;
-	track.format.m_dataSize = totalPcm;
-	MxU32 offset = 0;
-	for (size_t i = 1; i < chunks.size(); i++) {
-		SDL_memcpy(track.pcmData + offset, chunks[i].data(), chunks[i].size());
-		offset += (MxU32) chunks[i].size();
-	}
-
-	p_data.audioTracks.push_back(std::move(track));
-	return true;
-}
-
 bool Loader::ParsePhonemeChild(si::Object* p_child, SceneAnimData& p_data)
 {
 	auto& chunks = p_child->data_;
@@ -333,7 +223,10 @@ bool Loader::ParseComposite(si::Object* p_composite, SceneAnimData& p_data)
 			}
 		}
 		else if (child->filetype() == si::MxOb::WAV) {
-			ParseSoundChild(child, p_data);
+			Multiplayer::AudioTrack track;
+			if (SIReader::ExtractAudioTrack(child, track)) {
+				p_data.audioTracks.push_back(std::move(track));
+			}
 		}
 	}
 
@@ -362,15 +255,18 @@ SceneAnimData* Loader::EnsureCached(uint32_t p_objectId)
 		// Preload failed — fall through to synchronous load
 	}
 
-	if (!OpenSI()) {
+	if (!m_reader || !m_reader->Open()) {
 		return nullptr;
 	}
 
-	if (!ReadObject(p_objectId)) {
+	if (!m_reader->ReadObject(p_objectId)) {
 		return nullptr;
 	}
 
-	si::Object* composite = static_cast<si::Object*>(m_interleaf->GetChildAt(p_objectId));
+	si::Object* composite = m_reader->GetObject(p_objectId);
+	if (!composite) {
+		return nullptr;
+	}
 
 	SceneAnimData data;
 	if (!ParseComposite(composite, data)) {
@@ -420,7 +316,7 @@ MxResult Loader::PreloadThread::Run()
 	si::File* siFile = nullptr;
 	si::Interleaf* interleaf = nullptr;
 
-	if (!OpenSIHeaderOnly("\\lego\\scripts\\isle\\isle.si", siFile, interleaf)) {
+	if (!SIReader::OpenHeaderOnly("\\lego\\scripts\\isle\\isle.si", siFile, interleaf)) {
 		m_loader->m_preloadDone = true;
 		return MxThread::Run();
 	}
@@ -443,46 +339,3 @@ MxResult Loader::PreloadThread::Run()
 
 	return MxThread::Run();
 }
-
-SceneAnimData::AudioTrack* Loader::EnsureHornCached(uint32_t p_objectId)
-{
-	{
-		AUTOLOCK(m_cacheCS);
-		auto it = m_hornCache.find(p_objectId);
-		if (it != m_hornCache.end()) {
-			return &it->second;
-		}
-	}
-
-	if (!OpenSI()) {
-		return nullptr;
-	}
-
-	if (!ReadObject(p_objectId)) {
-		return nullptr;
-	}
-
-	si::Object* composite = static_cast<si::Object*>(m_interleaf->GetChildAt(p_objectId));
-
-	// Find the first WAV child in the composite (the horn sound)
-	for (size_t i = 0; i < composite->GetChildCount(); i++) {
-		si::Object* child = static_cast<si::Object*>(composite->GetChildAt(i));
-
-		if (child->filetype() == si::MxOb::WAV) {
-			SceneAnimData data;
-			if (ParseSoundChild(child, data)) {
-				// Take ownership of the PCM buffer before data's destructor frees it.
-				// AudioTrack has a raw pointer, so std::move alone doesn't transfer ownership.
-				SceneAnimData::AudioTrack track = data.audioTracks[0];
-				data.audioTracks[0].pcmData = nullptr;
-
-				AUTOLOCK(m_cacheCS);
-				auto result = m_hornCache.emplace(p_objectId, track);
-				return &result.first->second;
-			}
-			break;
-		}
-	}
-
-	return nullptr;
-}

extensions/src/multiplayer/networkmanager.cpp

@@ -73,6 +73,7 @@ NetworkManager::NetworkManager()
 	  m_animInterestDirty(false), m_lastAnimPushTime(0), m_connectionState(STATE_DISCONNECTED), m_wasRejected(false),
 	  m_reconnectAttempt(0), m_reconnectDelay(0), m_nextReconnectTime(0), m_hornTemplates{}, m_activeHorns()
 {
+	m_animLoader.SetSIReader(&m_siReader);
 }
 
 NetworkManager::~NetworkManager()
@@ -1106,6 +1107,19 @@ void NetworkManager::SendHorn(int8_t p_vehicleType)
 	SendMessage(msg);
 }
 
+// Vehicle type and dashboard composite ID for each horn-capable vehicle
+struct HornVehicleInfo {
+	int8_t vehicleType;
+	uint32_t dashboardObjectId;
+};
+
+static const HornVehicleInfo g_hornVehicles[] = {
+	{VEHICLE_BIKE, IsleScript::c_BikeDashboard},
+	{VEHICLE_AMBULANCE, IsleScript::c_AmbulanceDashboard},
+	{VEHICLE_TOWTRACK, IsleScript::c_TowTrackDashboard},
+	{VEHICLE_DUNEBUGGY, IsleScript::c_DuneCarDashboard},
+};
+
 void NetworkManager::HandleHorn(const HornMsg& p_msg)
 {
 	// Sweep finished horn sounds
@@ -1126,11 +1140,10 @@ void NetworkManager::HandleHorn(const HornMsg& p_msg)
 		return;
 	}
 
-	// Map vehicle type to horn template index
-	static const int8_t hornVehicles[] = {VEHICLE_BIKE, VEHICLE_AMBULANCE, VEHICLE_TOWTRACK, VEHICLE_DUNEBUGGY};
+	// Find horn template for this vehicle type
 	int templateIdx = -1;
 	for (int i = 0; i < HORN_VEHICLE_COUNT; i++) {
-		if (hornVehicles[i] == static_cast<int8_t>(p_msg.vehicleType)) {
+		if (g_hornVehicles[i].vehicleType == static_cast<int8_t>(p_msg.vehicleType)) {
 			templateIdx = i;
 			break;
 		}
@@ -1148,20 +1161,12 @@ void NetworkManager::HandleHorn(const HornMsg& p_msg)
 	}
 }
 
-// Dashboard composite IDs that contain horn WAV children
-static const uint32_t g_hornDashboardIds[4] = {
-	IsleScript::c_BikeDashboard,
-	IsleScript::c_AmbulanceDashboard,
-	IsleScript::c_TowTrackDashboard,
-	IsleScript::c_DuneCarDashboard,
-};
-
 void NetworkManager::PreloadHornSounds()
 {
 	for (int i = 0; i < HORN_VEHICLE_COUNT; i++) {
 		m_hornTemplates[i] = nullptr;
 
-		Animation::SceneAnimData::AudioTrack* track = m_animLoader.EnsureHornCached(g_hornDashboardIds[i]);
+		AudioTrack* track = m_siReader.ExtractFirstAudio(g_hornVehicles[i].dashboardObjectId);
 		if (!track) {
 			continue;
 		}
@@ -1176,6 +1181,9 @@ void NetworkManager::PreloadHornSounds()
 		else {
 			delete sound;
 		}
+
+		delete[] track->pcmData;
+		delete track;
 	}
 }
 

extensions/src/multiplayer/sireader.cpp

@@ -0,0 +1,163 @@
+#include "extensions/multiplayer/sireader.h"
+
+#include "extensions/common/pathutils.h"
+#include "mxautolock.h"
+
+#include <SDL3/SDL_stdinc.h>
+#include <file.h>
+#include <interleaf.h>
+
+using namespace Multiplayer;
+
+SIReader::SIReader() : m_siFile(nullptr), m_interleaf(nullptr), m_siReady(false)
+{
+}
+
+SIReader::~SIReader()
+{
+	delete m_interleaf;
+	delete m_siFile;
+}
+
+bool SIReader::OpenHeaderOnly(const char* p_siPath, si::File*& p_file, si::Interleaf*& p_interleaf)
+{
+	p_file = new si::File();
+
+	MxString path;
+	if (!Extensions::Common::ResolveGamePath(p_siPath, path) || !p_file->Open(path.GetData(), si::File::Read)) {
+		delete p_file;
+		p_file = nullptr;
+		return false;
+	}
+
+	p_interleaf = new si::Interleaf();
+	if (p_interleaf->Read(p_file, si::Interleaf::HeaderOnly) != si::Interleaf::ERROR_SUCCESS) {
+		delete p_interleaf;
+		p_interleaf = nullptr;
+		p_file->Close();
+		delete p_file;
+		p_file = nullptr;
+		return false;
+	}
+
+	return true;
+}
+
+bool SIReader::Open()
+{
+	if (m_siReady) {
+		return true;
+	}
+
+	if (!OpenHeaderOnly("\\lego\\scripts\\isle\\isle.si", m_siFile, m_interleaf)) {
+		return false;
+	}
+
+	m_siReady = true;
+	return true;
+}
+
+bool SIReader::ReadObject(uint32_t p_objectId)
+{
+	if (!m_siReady) {
+		return false;
+	}
+
+	size_t childCount = m_interleaf->GetChildCount();
+	if (p_objectId >= childCount) {
+		return false;
+	}
+
+	si::Object* obj = static_cast<si::Object*>(m_interleaf->GetChildAt(p_objectId));
+	if (obj->type() != si::MxOb::Null) {
+		return true;
+	}
+
+	return m_interleaf->ReadObject(m_siFile, p_objectId) == si::Interleaf::ERROR_SUCCESS;
+}
+
+si::Object* SIReader::GetObject(uint32_t p_objectId)
+{
+	if (!m_siReady) {
+		return nullptr;
+	}
+
+	size_t childCount = m_interleaf->GetChildCount();
+	if (p_objectId >= childCount) {
+		return nullptr;
+	}
+
+	return static_cast<si::Object*>(m_interleaf->GetChildAt(p_objectId));
+}
+
+bool SIReader::ExtractAudioTrack(si::Object* p_child, AudioTrack& p_out)
+{
+	auto& chunks = p_child->data_;
+	if (chunks.size() < 2) {
+		return false;
+	}
+
+	// data_[0] = WaveFormat header, data_[1..N] = raw PCM blocks
+	const auto& header = chunks[0];
+	if (header.size() < sizeof(MxWavePresenter::WaveFormat)) {
+		return false;
+	}
+
+	SDL_memcpy(&p_out.format, header.data(), sizeof(MxWavePresenter::WaveFormat));
+	p_out.pcmData = nullptr;
+	p_out.pcmDataSize = 0;
+	p_out.volume = (int32_t) p_child->volume_;
+	p_out.timeOffset = p_child->time_offset_;
+	p_out.mediaSrcPath = p_child->filename_;
+
+	MxU32 totalPcm = 0;
+	for (size_t i = 1; i < chunks.size(); i++) {
+		totalPcm += (MxU32) chunks[i].size();
+	}
+
+	if (totalPcm == 0) {
+		return false;
+	}
+
+	p_out.pcmData = new MxU8[totalPcm];
+	p_out.pcmDataSize = totalPcm;
+	p_out.format.m_dataSize = totalPcm;
+	MxU32 offset = 0;
+	for (size_t i = 1; i < chunks.size(); i++) {
+		SDL_memcpy(p_out.pcmData + offset, chunks[i].data(), chunks[i].size());
+		offset += (MxU32) chunks[i].size();
+	}
+
+	return true;
+}
+
+AudioTrack* SIReader::ExtractFirstAudio(uint32_t p_objectId)
+{
+	if (!Open()) {
+		return nullptr;
+	}
+
+	if (!ReadObject(p_objectId)) {
+		return nullptr;
+	}
+
+	si::Object* composite = GetObject(p_objectId);
+	if (!composite) {
+		return nullptr;
+	}
+
+	for (size_t i = 0; i < composite->GetChildCount(); i++) {
+		si::Object* child = static_cast<si::Object*>(composite->GetChildAt(i));
+
+		if (child->filetype() == si::MxOb::WAV) {
+			AudioTrack* track = new AudioTrack();
+			if (ExtractAudioTrack(child, *track)) {
+				return track;
+			}
+			delete track;
+			break;
+		}
+	}
+
+	return nullptr;
+}