Reliability retrofit: thread-safe WebSockets, getPlayer hardening, request validation, dedicated-server packaging

[coredmp95]

Summary

A targeted reliability retrofit of the HTTP/WebSocket monitoring API so it returns accurate,
real-time game state without crashing or corrupting the dedicated server — reliability of the
monitoring channel over feature breadth.

No new external dependencies. SML ^3.12.0 compatibility preserved. All public API response
shapes are unchanged or additive-only (see Compatibility).

Because the project has no automated test framework, every runtime change was verified
live against a deployed Satisfactory Linux dedicated server, not just by compiling.

Area	What changed	Why it matters
WebSocket threading	All inbound callbacks and outbound sends are marshaled through the game thread.	Removes a whole class of race-condition crashes / shared-state corruption.
getPlayer	Crash-safe reads, new location.lookPitch, persistent player names across disconnect.	Endpoint no longer crashes the server; richer, more reliable data.
WS request validation	Malformed/malicious subscribe envelopes are rejected with a clear error before dispatch.	Bad input can no longer reach endpoint logic on the now-guaranteed game thread.
Dedicated-server packaging	Branded 503 fallback when the web UI is missing; icon staging verified against a real packaged build.	Servers without the web UI bundle degrade gracefully instead of breaking silently.
Dev tooling	Reproducible build/package pipeline, WS stress + validation harnesses, deploy/verify runbook.	Makes the plugin buildable and verifiable on Linux.

Changes

1. Thread-safe WebSocket request handling

FicsitRemoteMonitoring.cpp/.h — inbound open/close/message callbacks and the push loop no
longer touch shared subsystem state (ConnectedClients, EndpointSubscribers, subscription maps)
from the uWS background thread. Introduces generation-tagged client identity
(ClientGenerations + IsClientCurrent), marshals all mutation to the game thread via
AsyncTask, and defers outbound send()/close() onto the captured uWS loop with liveness +
generation re-checks. Retires the old OnClientDisconnected path; fixes a connect/disconnect log
bug. No client-visible behavior change — purely where the work runs.

2. getPlayer endpoint reliability

PlayerLibrary.cpp, FicsitRemoteMonitoring.h, FicsitRemoteMonitoring.build.cs — IsValid()
guards on the two known crash sites (GetInventory(), GetHealthComponent()) with safe fallbacks;
new location.lookPitch (camera/aim pitch, clamped [-90,90]); a session-persistent PlayerNameCache
(keyed by the crash-safe APlayerState::GetUniqueId(), not AFGPlayerState::GetUserID() which
SIGSEGVs on dedicated servers) so offline players are emitted without dereferencing despawned
actors. Links the CoreOnline module.

3. WebSocket request validation (VALD-01)

FicsitRemoteMonitoring.cpp/.h — new ValidateWSSubscriptionEnvelope performs strict envelope
validation before any endpoint logic runs (action present/known; endpoints must be a string or
array of strings; GET-only registry match; capped problem reporting). Wired into
ProcessClientRequest and OnMessageReceived; rejected input returns an {"error": …} frame and
never reaches dispatch. HTTP CallEndpoint path untouched (no REST regression).

4. Dedicated-server asset packaging & fallback

FicsitRemoteMonitoring.cpp, FRM_Request.cpp/.h, Config/FilterPlugin.ini — a branded 503
fallback page (SendFallbackPage) when the www/ web UI bundle isn't deployed, with carve-outs so
sub-resources / /Icons/* / /api/* keep their normal 404/JSON content-type contract, and a
guard so extensionless bare-form API calls (e.g. /getWorldInv) keep returning JSON. Confirmed
that Config/PluginSettings.ini (unchanged) is what stages the Icons/ directory into a packaged
build — verified against a real LinuxServer zip; FilterPlugin.ini marked inert.

Developer tooling (non-shipping)

build/package.sh (reproducible compile/package), build/RUNBOOK.md (deploy/verify runbook),
build/tools/ws-stress-harness.mjs + ws-validation-probe.mjs (zero-dependency Node test
instruments), .gitignore, and this CHANGELOG.md.

Compatibility

Unchanged: Config/PluginSettings.ini (already correct); HTTP CallEndpoint dispatch; SML
^3.12.0; no new external C++/JS libraries (uses already-linked Unreal Json + vendored uWS).

Additive-only: getPlayer gains location.lookPitch and may now include offline players; WS
subscribe/unsubscribe now returns an {"error": …} frame for malformed envelopes (previously
undefined behavior). Well-formed requests are unaffected.

Note on upstream porisius#297 (feat: Actor Pitch)

This PR intentionally exposes the player pitch as location.lookPitch, not pitch, so it
coexists cleanly with the generic actor-rotation pitch field added in
porisius#297 (which lives in the shared getActorJSON helper). The two are
complementary and use distinct keys:

• pitch (from feat: Actor Pitch porisius/FicsitRemoteMonitoring#297) = actor body rotation — meaningful for vehicles/trains tilting on
  terrain; ~0 for an upright player.
• location.lookPitch (this PR) = the player's camera/aim pitch — the only pitch that
  carries information for a player.

Different files, so there is no merge conflict, and no key shadowing when both land.

Verification

No automated test framework exists (by design). Verified live on a deployed Linux dedicated
server:

• Threading: WS stress harness under connection churn — clean closes, Remaining connections: 0
  after each cycle, no crash/leak.
• Validation: ws-validation-probe.mjs — 8/8 malformed-payload cases rejected; 25-client churn,
  no regression for valid clients.
• Packaging: real LinuxServer.zip inspected (Icons/ staged); PKG-02 six-case curl battery
  (503 fallback + carve-outs + bare-form JSON guard) all green.

See CHANGELOG.md for the full per-area detail and reviewer file map.

---

🤖 Generated with Claude Code

[sourcery-ai]

Reviewer's Guide

Retrofits the monitoring plugin for reliability by making WebSocket handling thread-safe, hardening getPlayer against crashes and adding pitch/offline players, validating WebSocket subscription envelopes before dispatch, and improving dedicated-server packaging with a 503 fallback page, plus build/run tooling and WS probes.

Sequence diagram for thread-safe WebSocket subscribe and push flow

sequenceDiagram
  actor WSClient
  participant uWSLoop as uWSLoopThread
  participant FRM as AFicsitRemoteMonitoring
  participant GameThread

  WSClient->>uWSLoop: send("{action:'subscribe', endpoints:['getWorldInv']}")
  uWSLoop->>FRM: OnMessageReceived(ws, message, opCode)
  FRM->>FRM: ValidateWSSubscriptionEnvelope(JsonRequest, Action, ValidNames, ErrorMessage)
  alt envelope invalid
    FRM-->>WSClient: ws.send({"error": ErrorMessage})
  else envelope valid
    FRM->>GameThread: AsyncTask(GameThread, [ws, RequestClientID, Action, ValidNames])
    GameThread->>FRM: ProcessClientRequest(ws, RequestClientID, Action, ValidNames)
    GameThread->>FRM: EndpointSubscribers[Endpoint].Add(ws)
    GameThread->>FRM: StartWebSocketPushDataLoop()
  end

  loop PushUpdatedData loop
    participant PushThread
    PushThread->>GameThread: AsyncTask(GameThread, build Snapshot from EndpointSubscribers, ClientGenerations)
    GameThread-->>PushThread: Snapshot({Payload, {ws, ClientID}})
    PushThread->>uWSLoop: CapturedLoop->defer([ws, ClientID, PayloadUtf8])
    uWSLoop->>FRM: check LoopLiveSockets[ws] == ClientID
    alt client still live
      uWSLoop-->>WSClient: ws.send(PayloadUtf8)
    else client disconnected
      uWSLoop-->>WSClient: [no-op]
    end
  end

Loading

File-Level Changes

Change	Details	Files
WebSocket lifecycle, subscription management, and push broadcasting are made thread-safe with generation-tagged clients and loop-thread-deferred send/close operations.	Capture uWS loop pointer on the loop thread and use Loop::defer for all outbound send/close operations from game/push threads Introduce ClientGenerations and LoopLiveSockets maps with ClientID generation tags and an IsClientCurrent helper to validate ws pointers before mutating shared state Marshal open/close and subscribe/unsubscribe mutations (ConnectedClients, EndpointSubscribers, ClientGenerations) onto the game thread via AsyncTask with shutdown guards Rework PushUpdatedData to snapshot endpoint payloads and recipients on the game thread, then defer per-client sends onto the loop thread with liveness checks Ensure push loop flag bHasRunningPushDataLoop is only read/written on the game thread	Source/FicsitRemoteMonitoring/Private/FicsitRemoteMonitoring.cpp Source/FicsitRemoteMonitoring/Public/FicsitRemoteMonitoring.h
The getPlayer endpoint is hardened to avoid crashes, adds camera pitch to location, and emits offline players from a session-persistent name cache keyed by a safe stable player ID.	Guard inventory and health component access with IsValid checks and provide neutral defaults when components are missing Compute location.pitch from PlayerController control rotation with clamping to [-90,90] and inject into getActorJSON result Track visited IDs and names during live actor enumeration to avoid duplicates when adding offline players Introduce PlayerNameCache keyed by APlayerState::GetUniqueId().ToString() and use it to build offline player JSON entries when actors are gone Link CoreOnline module to safely use FUniqueNetIdRepl::IsValid and ToString instead of dedicated-server-crashing AFGPlayerState::GetUserID	Source/FicsitRemoteMonitoring/Private/Endpoints/World/PlayerLibrary.cpp Source/FicsitRemoteMonitoring/Public/FicsitRemoteMonitoring.h Source/FicsitRemoteMonitoring/FicsitRemoteMonitoring.build.cs
WebSocket subscribe/unsubscribe requests are validated synchronously on the loop thread with strict JSON and endpoint checks, returning structured error frames without closing the connection.	Add ValidateWSSubscriptionEnvelope to enforce valid action values, endpoints as string or array of strings, and GET-only endpoint registration Replace permissive GetStringField/AsString usage with TryGetStringField/TryGetArrayField plus explicit EJson::String checks Wire validation into ProcessClientRequest to short-circuit invalid envelopes while still allowing partial success for mixed valid/invalid names Update OnMessageReceived to respond with an error JSON frame when payload is not valid JSON instead of silently logging and dropping Cap the number of reported problems in error messages to prevent oversized frames from extremely invalid inputs	Source/FicsitRemoteMonitoring/Private/FicsitRemoteMonitoring.cpp Source/FicsitRemoteMonitoring/Public/FicsitRemoteMonitoring.h
HTTP routing for the dedicated server gains a branded 503 HTML fallback page when the web UI bundle is missing, while preserving JSON contracts for API routes and icons.	Introduce DocsURL constant and SendFallbackPage helper to serve a 503 HTML page pointing to docs and /api/ when web UI files are absent Modify the catch-all HTTP handler to distinguish page navigations from API calls using extension plus IsRegisteredEndpointName, sending fallback only for page-like requests Ensure bare-form API endpoints (extensionless paths that match registered endpoints) continue to return JSON and are not diverted to the fallback Update HandleGetRequest to use the fallback page for missing html/htm files while keeping JSON error responses for other extensions Document FilterPlugin.ini as inert for packaging and rely on PluginSettings.ini stage settings for Icons/www bundling	Source/FicsitRemoteMonitoring/Private/FicsitRemoteMonitoring.cpp Source/FicsitRemoteMonitoring/Private/FRM_Request.cpp Source/FicsitRemoteMonitoring/Public/FRM_Request.h Config/FilterPlugin.ini
Developer tooling is added for reproducible build/package, deployment/runbook, and WebSocket stress/validation harnesses, plus minor repo scaffolding.	Add build/package.sh wrapper around RunUBT and RunUAT PackagePlugin for Linux editor compile and Win64/LinuxServer packaging Document end-to-end build/deploy/verify flow in build/RUNBOOK.md including packaging pitfalls and fallback-page test battery Add ws-stress-harness.mjs to churn multiple WebSocket clients and assert server liveness and clean closes Add ws-validation-probe.mjs to exercise malformed and well-formed WS subscription payloads and check for expected error or success behavior Update .gitignore for planning artifacts and add CHANGELOG.md describing reliability retrofit scope and file map	build/package.sh build/RUNBOOK.md build/tools/ws-stress-harness.mjs build/tools/ws-validation-probe.mjs CHANGELOG.md .gitignore

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• PushUpdatedData uses a busy-wait loop with FPlatformProcess::Sleep(0.0001f) to wait for the game-thread snapshot, which can be unnecessarily CPU-intensive; consider using an event or more explicit synchronization primitive to avoid tight polling.
• PlayerNameCache is only ever appended to and cleared on server restart, so long-lived servers with many unique players may accumulate stale entries; consider adding an eviction strategy or periodic cleanup keyed off player activity.
• build/package.sh’s header comments still describe the package stage as “not yet implemented,” which no longer matches the implemented stage_package function; updating this comment will make the script’s behavior clearer to future maintainers.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- PushUpdatedData uses a busy-wait loop with FPlatformProcess::Sleep(0.0001f) to wait for the game-thread snapshot, which can be unnecessarily CPU-intensive; consider using an event or more explicit synchronization primitive to avoid tight polling.
- PlayerNameCache is only ever appended to and cleared on server restart, so long-lived servers with many unique players may accumulate stale entries; consider adding an eviction strategy or periodic cleanup keyed off player activity.
- build/package.sh’s header comments still describe the `package` stage as “not yet implemented,” which no longer matches the implemented stage_package function; updating this comment will make the script’s behavior clearer to future maintainers.

## Individual Comments

### Comment 1
<location path="Source/FicsitRemoteMonitoring/Private/FicsitRemoteMonitoring.cpp" line_range="706" />
<code_context>
-            if (!EndpointSubscribers.Contains(Endpoint)) {
-                EndpointSubscribers.Add(Endpoint, TSet<uWS::WebSocket<false, true, FWebSocketUserData>*>());
-            }
+void AFicsitRemoteMonitoring::PushUpdatedData() {

-			if (!bHasRunningPushDataLoop) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the spin-wait in PushUpdatedData with an FEvent-based wait and splitting ValidateWSSubscriptionEnvelope into smaller helper functions to make the threading and validation logic clearer and easier to reason about.

The three‑thread choreography and generation maps are deliberate for safety, so I wouldn’t suggest reverting that. There are, however, a couple of focused changes that can reduce complexity (and some subtle risk) without changing behavior:

---

### 1. Avoid spin‑wait in `PushUpdatedData` snapshot

`PushUpdatedData` currently spins on `bSnapshotComplete`:

```cpp
FThreadSafeBool bSnapshotComplete = false;

AsyncTask(ENamedThreads::GameThread, [WeakThis, &Snapshot, &bSnapshotComplete]()
{
    ...
    bSnapshotComplete = true;
});

// ...
while (!bSnapshotComplete)
{
    FPlatformProcess::Sleep(0.0001f);
}
```

This adds cognitive load and is easy to misuse (capturing by reference, manual sleep tuning). You can keep the same “fire‑and‑wait” shape but use a `FEvent` to make the synchronization explicit and cheaper:

```cpp
void AFicsitRemoteMonitoring::PushUpdatedData()
{
    struct FPushRecipient { uWS::WebSocket<false, true, FWebSocketUserData>* Ws; int32 ClientID; };
    struct FPushSnapshotEntry { FString Payload; TArray<FPushRecipient> Recipients; };

    TArray<FPushSnapshotEntry> Snapshot;

    TWeakObjectPtr<AFicsitRemoteMonitoring> WeakThis(this);
    FEvent* SnapshotReady = FPlatformProcess::GetSynchEventFromPool(false);

    AsyncTask(ENamedThreads::GameThread, [WeakThis, &Snapshot, SnapshotReady]()
    {
        AFicsitRemoteMonitoring* Self = WeakThis.Get();
        if (Self && !Self->bShouldStop)
        {
            for (auto& Elem : Self->EndpointSubscribers)
            {
                if (Elem.Value.Num() == 0)
                {
                    continue;
                }

                bool bSuccess = false;
                int32 ErrorCode = 404;

                FRequestData RequestData;
                RequestData.bIsAuthorized = true;

                FString Json;
                Self->HandleEndpoint(Elem.Key, RequestData, bSuccess, ErrorCode, Json, EInterfaceType::Socket);

                FPushSnapshotEntry Entry;
                Entry.Payload = MoveTemp(Json);

                for (uWS::WebSocket<false, true, FWebSocketUserData>* Client : Elem.Value)
                {
                    if (const int32* FoundClientID = Self->ClientGenerations.Find(Client))
                    {
                        Entry.Recipients.Add({ Client, *FoundClientID });
                    }
                }

                if (Entry.Recipients.Num() > 0)
                {
                    Snapshot.Add(MoveTemp(Entry));
                }
            }
        }

        SnapshotReady->Trigger();
    });

    // Block the pacing thread until the snapshot is ready. No spin‑loop.
    SnapshotReady->Wait();
    FPlatformProcess::ReturnSynchEventToPool(SnapshotReady);

    if (!CapturedLoop)
    {
        return;
    }

    for (const FPushSnapshotEntry& Entry : Snapshot)
    {
        FTCHARToUTF8 Converted(*Entry.Payload);
        const std::string PayloadUtf8(Converted.Get(), Converted.Length());

        for (const FPushRecipient& Recipient : Entry.Recipients)
        {
            uWS::WebSocket<false, true, FWebSocketUserData>* Ws = Recipient.Ws;
            const int32 ClientID = Recipient.ClientID;

            CapturedLoop->defer([WeakThis, Ws, ClientID, PayloadUtf8]()
            {
                AFicsitRemoteMonitoring* Self = WeakThis.Get();
                if (!Self)
                {
                    return;
                }

                const int32* FoundClientID = Self->LoopLiveSockets.Find(Ws);
                if (FoundClientID && *FoundClientID == ClientID)
                {
                    Ws->send(PayloadUtf8, uWS::OpCode::TEXT);
                }
            });
        }
    }
}
```

This keeps the three‑thread protocol and generation tagging intact but makes the synchronization primitive clearer and removes the manual spin‑sleep loop.

---

### 2. Factor `ValidateWSSubscriptionEnvelope` into smaller helpers

`ValidateWSSubscriptionEnvelope` currently does envelope extraction, type checking, registry lookup, and error aggregation in one long function. You can keep all its semantics but split out the “normalize endpoints” and “registry filter” steps to reduce nesting and make the intent more discoverable:

```cpp
bool AFicsitRemoteMonitoring::ExtractEndpointNames(
    const TSharedPtr<FJsonObject>& JsonRequest,
    TArray<FString>& OutRawNames,
    FString& OutErrorMessage,
    TArray<FString>& OutProblems) const
{
    const TArray<TSharedPtr<FJsonValue>>* EndpointsArray;
    FString SingleEndpoint;

    if (JsonRequest->TryGetArrayField(TEXT("endpoints"), EndpointsArray))
    {
        for (int32 Index = 0; Index < EndpointsArray->Num(); ++Index)
        {
            const TSharedPtr<FJsonValue>& EndpointValue = (*EndpointsArray)[Index];
            if (!EndpointValue.IsValid() || EndpointValue->Type != EJson::String)
            {
                OutProblems.Add(FString::Printf(TEXT("endpoints[%d] is not a string"), Index));
                continue;
            }
            OutRawNames.Add(EndpointValue->AsString());
        }
        return true;
    }
    else if (JsonRequest->TryGetStringField(TEXT("endpoints"), SingleEndpoint))
    {
        OutRawNames.Add(SingleEndpoint);
        return true;
    }

    OutErrorMessage = TEXT("'endpoints' field is required and must be a string or an array of strings.");
    return false;
}

void AFicsitRemoteMonitoring::FilterRegisteredGetEndpoints(
    const TArray<FString>& RawNames,
    TArray<FString>& OutValidNames,
    TArray<FString>& OutProblems) const
{
    for (const FString& Name : RawNames)
    {
        const bool bIsRegisteredGet = APIEndpoints.ContainsByPredicate([&Name](const FAPIEndpoint& Endpoint)
        {
            return Endpoint.APIName == Name && Endpoint.Method == TEXT("GET");
        });

        if (bIsRegisteredGet)
        {
            OutValidNames.AddUnique(Name);
        }
        else
        {
            OutProblems.Add(FString::Printf(TEXT("Unknown endpoint: %s"), *Name));
        }
    }
}
```

Then `ValidateWSSubscriptionEnvelope` becomes a thin orchestrator:

```cpp
bool AFicsitRemoteMonitoring::ValidateWSSubscriptionEnvelope(
    const TSharedPtr<FJsonObject>& JsonRequest,
    FString& OutAction,
    TArray<FString>& OutValidNames,
    FString& OutErrorMessage) const
{
    FString RawAction;
    if (!JsonRequest->TryGetStringField(TEXT("action"), RawAction)
        || (RawAction != TEXT("subscribe") && RawAction != TEXT("unsubscribe")))
    {
        OutErrorMessage = FString::Printf(
            TEXT("Invalid or missing 'action' field (got '%s'); expected 'subscribe' or 'unsubscribe'."),
            *RawAction);
        return false;
    }
    OutAction = RawAction;

    TArray<FString> RawNames;
    TArray<FString> Problems;

    if (!ExtractEndpointNames(JsonRequest, RawNames, OutErrorMessage, Problems))
    {
        return false; // whole‑request reject
    }

    FilterRegisteredGetEndpoints(RawNames, OutValidNames, Problems);

    if (Problems.Num() > 0)
    {
        constexpr int32 MaxReportedProblems = 20;
        if (Problems.Num() > MaxReportedProblems)
        {
            const int32 Remainder = Problems.Num() - MaxReportedProblems;
            Problems.SetNum(MaxReportedProblems);
            Problems.Add(FString::Printf(TEXT("...and %d more"), Remainder));
        }
        OutErrorMessage = FString::Join(Problems, TEXT("; "));
    }

    return true;
}
```

Behavior (strict envelope validation, partial success, capped error aggregation, GET‑only endpoints) stays exactly the same, but the heavy branching and local arrays are now encapsulated in helpers, making the overall flow easier to follow.
</issue_to_address>

### Comment 2
<location path="Source/FicsitRemoteMonitoring/Public/FicsitRemoteMonitoring.h" line_range="121" />
<code_context>
 	bool bShouldStop = false;
 	bool bHasRunningPushDataLoop = false;

+	// Loop-thread-only monotonic counter used to mint each connection's ClientID in wsBehavior.open.
+	int32 NextClientIDCounter{};
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the new websocket client state, validation utilities, and player-name cache into focused helper types or accessors to keep AFicsitRemoteMonitoring’s responsibilities smaller and more coherent.

The new members and helpers materially increase the subsystem’s responsibilities and coupling. You can keep all behavior but reduce complexity by encapsulating some of this state into smaller focused helpers and hiding raw containers behind narrow APIs.

### 1. Encapsulate WS client tracking / generations

`NextClientIDCounter`, `LoopLiveSockets`, `CapturedLoop`, and `ClientGenerations` form a coherent concept: “WS client lifetime + ABA protection”. Pulling them into a small helper reduces the mental load in `AFicsitRemoteMonitoring` and centralizes invariants (loop-thread vs game-thread, generation checks).

**Example (header):**

```cpp
// New helper type
class FWSClientRegistry
{
public:
	// loop-thread side
	void OnLoopStarted(uWS::Loop* InLoop)
	{
		CapturedLoop = InLoop;
	}

	int32 MintClientID()
	{
		return ++NextClientIDCounter;
	}

	void TrackLoopSocket(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID)
	{
		LoopLiveSockets.Add(ws, ClientID);
	}

	void UntrackLoopSocket(uWS::WebSocket<false, true, FWebSocketUserData>* ws)
	{
		LoopLiveSockets.Remove(ws);
	}

	// game-thread side
	void TrackClient(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID)
	{
		ClientGenerations.Add(ws, ClientID);
	}

	void UntrackClient(uWS::WebSocket<false, true, FWebSocketUserData>* ws)
	{
		ClientGenerations.Remove(ws);
	}

	bool IsClientCurrent(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID) const
	{
		const int32* Found = ClientGenerations.Find(ws);
		return Found && *Found == ClientID;
	}

	uWS::Loop* GetLoop() const { return CapturedLoop; }

private:
	int32 NextClientIDCounter{};
	TMap<uWS::WebSocket<false, true, FWebSocketUserData>*, int32> LoopLiveSockets{};
	TMap<uWS::WebSocket<false, true, FWebSocketUserData>*, int32> ClientGenerations{};
	uWS::Loop* CapturedLoop{ nullptr };
};
```

Then the subsystem only holds the helper:

```cpp
private:
	FWSClientRegistry WSClients;
```

And existing calls become:

```cpp
// was: int32 ClientID = ++NextClientIDCounter;
int32 ClientID = WSClients.MintClientID();

// was: ClientGenerations.Add(ws, ClientID);
WSClients.TrackClient(ws, ClientID);

// was: IsClientCurrent(ws, ClientID);
bool bCurrent = WSClients.IsClientCurrent(ws, ClientID);
```

This keeps all semantics but makes ownership and lifetime rules localized to `FWSClientRegistry`.

### 2. Move validation helpers out of the subsystem class

`IsRegisteredEndpointName` and especially `ValidateWSSubscriptionEnvelope` are pure validation utilities depending only on `APIEndpoints`. Moving them into a small utility (or static/anonymous‑namespace helpers) narrows the private API surface of `AFicsitRemoteMonitoring` and avoids it becoming the dumping ground for all WS logic.

**Example (utility header):**

```cpp
namespace FicsitWSValidation
{
	bool IsRegisteredEndpointName(const TArray<FAPIEndpoint>& Registry, const FString& InName);

	bool ValidateWSSubscriptionEnvelope(
		const TArray<FAPIEndpoint>& Registry,
		const TSharedPtr<FJsonObject>& JsonRequest,
		FString& OutAction,
		TArray<FString>& OutValidNames,
		FString& OutErrorMessage);
}
```

Subsystem usage:

```cpp
// was: bool IsRegisteredEndpointName(const FString& InName) const;
bool AFicsitRemoteMonitoring::IsRegisteredEndpointName(const FString& InName) const
{
	return FicsitWSValidation::IsRegisteredEndpointName(APIEndpoints, InName);
}

// was: ValidateWSSubscriptionEnvelope(JsonRequest, OutAction, OutValidNames, OutErrorMessage);
bool bOk = FicsitWSValidation::ValidateWSSubscriptionEnvelope(
	APIEndpoints, JsonRequest, OutAction, OutValidNames, OutErrorMessage);
```

You retain all current behavior and comments but move the heavy logic out of the core subsystem type.

### 3. Encapsulate `PlayerNameCache` behind narrow accessors

To keep the header lean and avoid exposing caching details as raw state, wrap `PlayerNameCache` in simple getter/setter helpers and potentially group them in a small “player identity” helper type if it grows further.

**Example:**

```cpp
private:
	TMap<FString, FString> PlayerNameCache{};

	FString GetCachedPlayerName(const FString& UniqueId) const
	{
		if (const FString* Name = PlayerNameCache.Find(UniqueId))
		{
			return *Name;
		}
		return {};
	}

	void RememberPlayerName(const FString& UniqueId, const FString& DisplayName)
	{
		PlayerNameCache.Add(UniqueId, DisplayName);
	}
```

Call sites no longer touch the `TMap` directly:

```cpp
// was: PlayerNameCache.Add(UniqueIdStr, DisplayName);
RememberPlayerName(UniqueIdStr, DisplayName);

// was: PlayerNameCache.Find(UniqueIdStr)...
FString Name = GetCachedPlayerName(UniqueIdStr);
```

This keeps the “remember names across disconnect” feature intact while reducing how much of that concern is visible at the class level.

Taken together, these small extra types/helpers preserve all new functionality but make `AFicsitRemoteMonitoring` significantly easier to reason about by centralizing WS lifetime/invariants and moving pure validation/caching concerns out of the subsystem’s core surface.
</issue_to_address>

### Comment 3
<location path="Source/FicsitRemoteMonitoring/Private/Endpoints/World/PlayerLibrary.cpp" line_range="67" />
<code_context>
+	}
+}
+
 void UPlayerLibrary::getPlayer(UObject* WorldContext, FRequestData RequestData, TArray<TSharedPtr<FJsonValue>>& OutJsonArray) {

 	TArray<AActor*> FoundActors;
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting helper functions for live player metrics, location JSON, and cache access so `getPlayer` focuses on high-level orchestration instead of many nested responsibilities.

```markdown
The new behavior is solid, but `getPlayer` has become a “do‑everything” function. You can keep all semantics while reducing complexity by extracting a few focused helpers and hiding the subsystem’s `PlayerNameCache`.

### 1. Extract live player metrics into a helper

All the `IsValid` checks and component access can live in a single helper returning a small struct. This removes most of the nesting in `getPlayer` without changing behavior.

```cpp
struct FLivePlayerMetrics
{
    float Speed = 0.0f;
    bool bOnline = false;
    float PlayerHP = 0.0f;
    bool bDead = false;
    float Pitch = 0.0f;
    TArray<TSharedPtr<FJsonValue>> InventoryJsonArray;
    FString UserID; // empty if no valid net ID
};

static FLivePlayerMetrics CollectLivePlayerMetrics(AFGCharacterPlayer* PlayerCharacter, FString PlayerName,
                                                   AFicsitRemoteMonitoring* ModSubsystem,
                                                   TSet<FString>& VisitedIDs)
{
    FLivePlayerMetrics Metrics;

    if (!IsValid(PlayerCharacter))
        return Metrics;

    Metrics.Speed = PlayerCharacter->GetVelocity().Length() * 0.036f;
    Metrics.bOnline = PlayerCharacter->IsPlayerOnline();

    if (UFGInventoryComponent* Inv = PlayerCharacter->GetInventory())
    {
        TArray<FInventoryStack> Stacks;
        Inv->GetInventoryStacks(Stacks);
        auto Grouped = GetGroupedInventoryItems(Stacks);
        Metrics.InventoryJsonArray = GetInventoryJSON(Grouped);
    }

    if (UFGHealthComponent* Health = PlayerCharacter->GetHealthComponent())
    {
        Metrics.PlayerHP = Health->GetCurrentHealth();
        Metrics.bDead   = Health->IsDead();
    }

    if (AFGPlayerController* PC = PlayerCharacter->GetFGPlayerController())
    {
        float RawPitch = PC->GetControlRotation().Pitch;
        if (RawPitch > 180.f) RawPitch -= 360.f;
        Metrics.Pitch = FMath::Clamp(RawPitch, -90.f, 90.f);
    }

    if (const APlayerState* PS = PlayerCharacter->GetPlayerState())
    {
        const FUniqueNetIdRepl& NetId = PS->GetUniqueId();
        if (NetId.IsValid())
        {
            Metrics.UserID = NetId.ToString();
            if (!Metrics.UserID.IsEmpty())
                VisitedIDs.Add(Metrics.UserID);
        }
    }

    // cache write stays here if ModSubsystem is valid
    if (IsValid(ModSubsystem) && !Metrics.UserID.IsEmpty())
    {
        ModSubsystem->PlayerNameCache.Add(Metrics.UserID, PlayerName);
    }

    return Metrics;
}
```

Then `getPlayer` becomes:

```cpp
AFGCharacterPlayer* PlayerCharacter = Cast<AFGCharacterPlayer>(Player);
FString PlayerName = GetPlayerName(PlayerCharacter);

if (!PlayerName.IsEmpty())
    VisitedNames.Add(PlayerName);

FLivePlayerMetrics Metrics = CollectLivePlayerMetrics(PlayerCharacter, PlayerName, ModSubsystem, VisitedIDs);

TSharedPtr<FJsonObject> LocationJson = getActorJSON(Player);
LocationJson->Values.Add("pitch", MakeShared<FJsonValueNumber>(Metrics.Pitch));

JPlayer->Values.Add("Name", MakeShared<FJsonValueString>(PlayerName));
JPlayer->Values.Add("ClassName", MakeShared<FJsonValueString>(Player->GetClass()->GetName()));
JPlayer->Values.Add("location", MakeShared<FJsonValueObject>(LocationJson));
JPlayer->Values.Add("Speed", MakeShared<FJsonValueNumber>(Metrics.Speed));
JPlayer->Values.Add("Online", MakeShared<FJsonValueBoolean>(Metrics.bOnline));
JPlayer->Values.Add("PlayerHP", MakeShared<FJsonValueNumber>(Metrics.PlayerHP));
JPlayer->Values.Add("Dead", MakeShared<FJsonValueBoolean>(Metrics.bDead));
JPlayer->Values.Add("Inventory", MakeShared<FJsonValueArray>(Metrics.InventoryJsonArray));
```

This keeps all safety and crash‑proofing, but shrinks `getPlayer` significantly.

### 2. Encapsulate pitch+location JSON in a helper

`getPlayer` currently knows that `location` is a JSON object you can mutate to add `"pitch"`. You can hide this detail behind a dedicated helper:

```cpp
static TSharedPtr<FJsonObject> BuildPlayerLocationJson(AActor* Player, float Pitch)
{
    TSharedPtr<FJsonObject> LocationJson = getActorJSON(Player);
    LocationJson->Values.Add("pitch", MakeShared<FJsonValueNumber>(Pitch));
    return LocationJson;
}
```

Usage in `getPlayer`:

```cpp
auto LocationJson = BuildPlayerLocationJson(Player, Metrics.Pitch);
JPlayer->Values.Add("location", MakeShared<FJsonValueObject>(LocationJson));
```

This keeps `getActorJSON` untouched but makes the pitch concern local and reusable.

### 3. Wrap `PlayerNameCache` behind a subsystem API

Instead of reaching directly into `PlayerNameCache` (TMap) from endpoint code, add small helpers to `AFicsitRemoteMonitoring`. This preserves behavior but reduces cross‑module coupling and makes the offline enumeration loop cleaner.

Subsystem:

```cpp
// AFicsitRemoteMonitoring.h
void RememberPlayerName(const FString& UserID, const FString& Name);
void GetCachedPlayers(TArray<TPair<FString, FString>>& OutPlayers) const;

// AFicsitRemoteMonitoring.cpp
void AFicsitRemoteMonitoring::RememberPlayerName(const FString& UserID, const FString& Name)
{
    PlayerNameCache.Add(UserID, Name);
}

void AFicsitRemoteMonitoring::GetCachedPlayers(TArray<TPair<FString, FString>>& OutPlayers) const
{
    for (const auto& Pair : PlayerNameCache)
    {
        OutPlayers.Add(Pair);
    }
}
```

Then in `getPlayer`:

```cpp
TArray<TPair<FString, FString>> CachedPlayers;
if (bHasValidSubsystem)
{
    ModSubsystem->GetCachedPlayers(CachedPlayers);
    for (const auto& CachedPlayer : CachedPlayers)
    {
        const FString& UserID    = CachedPlayer.Key;
        const FString& CachedName = CachedPlayer.Value;

        if (VisitedIDs.Contains(UserID) || VisitedNames.Contains(CachedName))
            continue;

        OutJsonArray.Add(MakeShared<FJsonValueObject>(BuildOfflinePlayerJson(UserID, CachedName)));
    }
}
```

And inside `CollectLivePlayerMetrics` you call `ModSubsystem->RememberPlayerName(...)` instead of touching `PlayerNameCache` directly.

These small, targeted extractions keep all current functionality (offline entries, crash‑safe IDs, pitch, dedupe) but make `getPlayer` read as a sequence of high‑level steps instead of a deeply nested multi‑responsibility function.
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (complexity): Consider replacing the spin-wait in PushUpdatedData with an FEvent-based wait and splitting ValidateWSSubscriptionEnvelope into smaller helper functions to make the threading and validation logic clearer and easier to reason about.

The three‑thread choreography and generation maps are deliberate for safety, so I wouldn’t suggest reverting that. There are, however, a couple of focused changes that can reduce complexity (and some subtle risk) without changing behavior:

---

1. Avoid spin‑wait in PushUpdatedData snapshot

PushUpdatedData currently spins on bSnapshotComplete:

FThreadSafeBool bSnapshotComplete = false;

AsyncTask(ENamedThreads::GameThread, [WeakThis, &Snapshot, &bSnapshotComplete]()
{
    ...
    bSnapshotComplete = true;
});

// ...
while (!bSnapshotComplete)
{
    FPlatformProcess::Sleep(0.0001f);
}

This adds cognitive load and is easy to misuse (capturing by reference, manual sleep tuning). You can keep the same “fire‑and‑wait” shape but use a FEvent to make the synchronization explicit and cheaper:

void AFicsitRemoteMonitoring::PushUpdatedData()
{
    struct FPushRecipient { uWS::WebSocket<false, true, FWebSocketUserData>* Ws; int32 ClientID; };
    struct FPushSnapshotEntry { FString Payload; TArray<FPushRecipient> Recipients; };

    TArray<FPushSnapshotEntry> Snapshot;

    TWeakObjectPtr<AFicsitRemoteMonitoring> WeakThis(this);
    FEvent* SnapshotReady = FPlatformProcess::GetSynchEventFromPool(false);

    AsyncTask(ENamedThreads::GameThread, [WeakThis, &Snapshot, SnapshotReady]()
    {
        AFicsitRemoteMonitoring* Self = WeakThis.Get();
        if (Self && !Self->bShouldStop)
        {
            for (auto& Elem : Self->EndpointSubscribers)
            {
                if (Elem.Value.Num() == 0)
                {
                    continue;
                }

                bool bSuccess = false;
                int32 ErrorCode = 404;

                FRequestData RequestData;
                RequestData.bIsAuthorized = true;

                FString Json;
                Self->HandleEndpoint(Elem.Key, RequestData, bSuccess, ErrorCode, Json, EInterfaceType::Socket);

                FPushSnapshotEntry Entry;
                Entry.Payload = MoveTemp(Json);

                for (uWS::WebSocket<false, true, FWebSocketUserData>* Client : Elem.Value)
                {
                    if (const int32* FoundClientID = Self->ClientGenerations.Find(Client))
                    {
                        Entry.Recipients.Add({ Client, *FoundClientID });
                    }
                }

                if (Entry.Recipients.Num() > 0)
                {
                    Snapshot.Add(MoveTemp(Entry));
                }
            }
        }

        SnapshotReady->Trigger();
    });

    // Block the pacing thread until the snapshot is ready. No spin‑loop.
    SnapshotReady->Wait();
    FPlatformProcess::ReturnSynchEventToPool(SnapshotReady);

    if (!CapturedLoop)
    {
        return;
    }

    for (const FPushSnapshotEntry& Entry : Snapshot)
    {
        FTCHARToUTF8 Converted(*Entry.Payload);
        const std::string PayloadUtf8(Converted.Get(), Converted.Length());

        for (const FPushRecipient& Recipient : Entry.Recipients)
        {
            uWS::WebSocket<false, true, FWebSocketUserData>* Ws = Recipient.Ws;
            const int32 ClientID = Recipient.ClientID;

            CapturedLoop->defer([WeakThis, Ws, ClientID, PayloadUtf8]()
            {
                AFicsitRemoteMonitoring* Self = WeakThis.Get();
                if (!Self)
                {
                    return;
                }

                const int32* FoundClientID = Self->LoopLiveSockets.Find(Ws);
                if (FoundClientID && *FoundClientID == ClientID)
                {
                    Ws->send(PayloadUtf8, uWS::OpCode::TEXT);
                }
            });
        }
    }
}

This keeps the three‑thread protocol and generation tagging intact but makes the synchronization primitive clearer and removes the manual spin‑sleep loop.

---

2. Factor ValidateWSSubscriptionEnvelope into smaller helpers

ValidateWSSubscriptionEnvelope currently does envelope extraction, type checking, registry lookup, and error aggregation in one long function. You can keep all its semantics but split out the “normalize endpoints” and “registry filter” steps to reduce nesting and make the intent more discoverable:

bool AFicsitRemoteMonitoring::ExtractEndpointNames(
    const TSharedPtr<FJsonObject>& JsonRequest,
    TArray<FString>& OutRawNames,
    FString& OutErrorMessage,
    TArray<FString>& OutProblems) const
{
    const TArray<TSharedPtr<FJsonValue>>* EndpointsArray;
    FString SingleEndpoint;

    if (JsonRequest->TryGetArrayField(TEXT("endpoints"), EndpointsArray))
    {
        for (int32 Index = 0; Index < EndpointsArray->Num(); ++Index)
        {
            const TSharedPtr<FJsonValue>& EndpointValue = (*EndpointsArray)[Index];
            if (!EndpointValue.IsValid() || EndpointValue->Type != EJson::String)
            {
                OutProblems.Add(FString::Printf(TEXT("endpoints[%d] is not a string"), Index));
                continue;
            }
            OutRawNames.Add(EndpointValue->AsString());
        }
        return true;
    }
    else if (JsonRequest->TryGetStringField(TEXT("endpoints"), SingleEndpoint))
    {
        OutRawNames.Add(SingleEndpoint);
        return true;
    }

    OutErrorMessage = TEXT("'endpoints' field is required and must be a string or an array of strings.");
    return false;
}

void AFicsitRemoteMonitoring::FilterRegisteredGetEndpoints(
    const TArray<FString>& RawNames,
    TArray<FString>& OutValidNames,
    TArray<FString>& OutProblems) const
{
    for (const FString& Name : RawNames)
    {
        const bool bIsRegisteredGet = APIEndpoints.ContainsByPredicate([&Name](const FAPIEndpoint& Endpoint)
        {
            return Endpoint.APIName == Name && Endpoint.Method == TEXT("GET");
        });

        if (bIsRegisteredGet)
        {
            OutValidNames.AddUnique(Name);
        }
        else
        {
            OutProblems.Add(FString::Printf(TEXT("Unknown endpoint: %s"), *Name));
        }
    }
}

Then ValidateWSSubscriptionEnvelope becomes a thin orchestrator:

bool AFicsitRemoteMonitoring::ValidateWSSubscriptionEnvelope(
    const TSharedPtr<FJsonObject>& JsonRequest,
    FString& OutAction,
    TArray<FString>& OutValidNames,
    FString& OutErrorMessage) const
{
    FString RawAction;
    if (!JsonRequest->TryGetStringField(TEXT("action"), RawAction)
        || (RawAction != TEXT("subscribe") && RawAction != TEXT("unsubscribe")))
    {
        OutErrorMessage = FString::Printf(
            TEXT("Invalid or missing 'action' field (got '%s'); expected 'subscribe' or 'unsubscribe'."),
            *RawAction);
        return false;
    }
    OutAction = RawAction;

    TArray<FString> RawNames;
    TArray<FString> Problems;

    if (!ExtractEndpointNames(JsonRequest, RawNames, OutErrorMessage, Problems))
    {
        return false; // whole‑request reject
    }

    FilterRegisteredGetEndpoints(RawNames, OutValidNames, Problems);

    if (Problems.Num() > 0)
    {
        constexpr int32 MaxReportedProblems = 20;
        if (Problems.Num() > MaxReportedProblems)
        {
            const int32 Remainder = Problems.Num() - MaxReportedProblems;
            Problems.SetNum(MaxReportedProblems);
            Problems.Add(FString::Printf(TEXT("...and %d more"), Remainder));
        }
        OutErrorMessage = FString::Join(Problems, TEXT("; "));
    }

    return true;
}

Behavior (strict envelope validation, partial success, capped error aggregation, GET‑only endpoints) stays exactly the same, but the heavy branching and local arrays are now encapsulated in helpers, making the overall flow easier to follow.

[sourcery-ai]

issue (complexity): Consider encapsulating the new websocket client state, validation utilities, and player-name cache into focused helper types or accessors to keep AFicsitRemoteMonitoring’s responsibilities smaller and more coherent.

The new members and helpers materially increase the subsystem’s responsibilities and coupling. You can keep all behavior but reduce complexity by encapsulating some of this state into smaller focused helpers and hiding raw containers behind narrow APIs.

1. Encapsulate WS client tracking / generations

NextClientIDCounter, LoopLiveSockets, CapturedLoop, and ClientGenerations form a coherent concept: “WS client lifetime + ABA protection”. Pulling them into a small helper reduces the mental load in AFicsitRemoteMonitoring and centralizes invariants (loop-thread vs game-thread, generation checks).

Example (header):

// New helper type
class FWSClientRegistry
{
public:
	// loop-thread side
	void OnLoopStarted(uWS::Loop* InLoop)
	{
		CapturedLoop = InLoop;
	}

	int32 MintClientID()
	{
		return ++NextClientIDCounter;
	}

	void TrackLoopSocket(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID)
	{
		LoopLiveSockets.Add(ws, ClientID);
	}

	void UntrackLoopSocket(uWS::WebSocket<false, true, FWebSocketUserData>* ws)
	{
		LoopLiveSockets.Remove(ws);
	}

	// game-thread side
	void TrackClient(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID)
	{
		ClientGenerations.Add(ws, ClientID);
	}

	void UntrackClient(uWS::WebSocket<false, true, FWebSocketUserData>* ws)
	{
		ClientGenerations.Remove(ws);
	}

	bool IsClientCurrent(uWS::WebSocket<false, true, FWebSocketUserData>* ws, int32 ClientID) const
	{
		const int32* Found = ClientGenerations.Find(ws);
		return Found && *Found == ClientID;
	}

	uWS::Loop* GetLoop() const { return CapturedLoop; }

private:
	int32 NextClientIDCounter{};
	TMap<uWS::WebSocket<false, true, FWebSocketUserData>*, int32> LoopLiveSockets{};
	TMap<uWS::WebSocket<false, true, FWebSocketUserData>*, int32> ClientGenerations{};
	uWS::Loop* CapturedLoop{ nullptr };
};

Then the subsystem only holds the helper:

private:
	FWSClientRegistry WSClients;

And existing calls become:

// was: int32 ClientID = ++NextClientIDCounter;
int32 ClientID = WSClients.MintClientID();

// was: ClientGenerations.Add(ws, ClientID);
WSClients.TrackClient(ws, ClientID);

// was: IsClientCurrent(ws, ClientID);
bool bCurrent = WSClients.IsClientCurrent(ws, ClientID);

This keeps all semantics but makes ownership and lifetime rules localized to FWSClientRegistry.

2. Move validation helpers out of the subsystem class

IsRegisteredEndpointName and especially ValidateWSSubscriptionEnvelope are pure validation utilities depending only on APIEndpoints. Moving them into a small utility (or static/anonymous‑namespace helpers) narrows the private API surface of AFicsitRemoteMonitoring and avoids it becoming the dumping ground for all WS logic.

Example (utility header):

namespace FicsitWSValidation
{
	bool IsRegisteredEndpointName(const TArray<FAPIEndpoint>& Registry, const FString& InName);

	bool ValidateWSSubscriptionEnvelope(
		const TArray<FAPIEndpoint>& Registry,
		const TSharedPtr<FJsonObject>& JsonRequest,
		FString& OutAction,
		TArray<FString>& OutValidNames,
		FString& OutErrorMessage);
}

Subsystem usage:

// was: bool IsRegisteredEndpointName(const FString& InName) const;
bool AFicsitRemoteMonitoring::IsRegisteredEndpointName(const FString& InName) const
{
	return FicsitWSValidation::IsRegisteredEndpointName(APIEndpoints, InName);
}

// was: ValidateWSSubscriptionEnvelope(JsonRequest, OutAction, OutValidNames, OutErrorMessage);
bool bOk = FicsitWSValidation::ValidateWSSubscriptionEnvelope(
	APIEndpoints, JsonRequest, OutAction, OutValidNames, OutErrorMessage);

You retain all current behavior and comments but move the heavy logic out of the core subsystem type.

3. Encapsulate PlayerNameCache behind narrow accessors

To keep the header lean and avoid exposing caching details as raw state, wrap PlayerNameCache in simple getter/setter helpers and potentially group them in a small “player identity” helper type if it grows further.

Example:

private:
	TMap<FString, FString> PlayerNameCache{};

	FString GetCachedPlayerName(const FString& UniqueId) const
	{
		if (const FString* Name = PlayerNameCache.Find(UniqueId))
		{
			return *Name;
		}
		return {};
	}

	void RememberPlayerName(const FString& UniqueId, const FString& DisplayName)
	{
		PlayerNameCache.Add(UniqueId, DisplayName);
	}

Call sites no longer touch the TMap directly:

// was: PlayerNameCache.Add(UniqueIdStr, DisplayName);
RememberPlayerName(UniqueIdStr, DisplayName);

// was: PlayerNameCache.Find(UniqueIdStr)...
FString Name = GetCachedPlayerName(UniqueIdStr);

This keeps the “remember names across disconnect” feature intact while reducing how much of that concern is visible at the class level.

Taken together, these small extra types/helpers preserve all new functionality but make AFicsitRemoteMonitoring significantly easier to reason about by centralizing WS lifetime/invariants and moving pure validation/caching concerns out of the subsystem’s core surface.

[sourcery-ai]

issue (complexity): Consider extracting helper functions for live player metrics, location JSON, and cache access so getPlayer focuses on high-level orchestration instead of many nested responsibilities.

The new behavior is solid, but `getPlayer` has become a “do‑everything” function. You can keep all semantics while reducing complexity by extracting a few focused helpers and hiding the subsystem’s `PlayerNameCache`.

### 1. Extract live player metrics into a helper

All the `IsValid` checks and component access can live in a single helper returning a small struct. This removes most of the nesting in `getPlayer` without changing behavior.

```cpp
struct FLivePlayerMetrics
{
    float Speed = 0.0f;
    bool bOnline = false;
    float PlayerHP = 0.0f;
    bool bDead = false;
    float Pitch = 0.0f;
    TArray<TSharedPtr<FJsonValue>> InventoryJsonArray;
    FString UserID; // empty if no valid net ID
};

static FLivePlayerMetrics CollectLivePlayerMetrics(AFGCharacterPlayer* PlayerCharacter, FString PlayerName,
                                                   AFicsitRemoteMonitoring* ModSubsystem,
                                                   TSet<FString>& VisitedIDs)
{
    FLivePlayerMetrics Metrics;

    if (!IsValid(PlayerCharacter))
        return Metrics;

    Metrics.Speed = PlayerCharacter->GetVelocity().Length() * 0.036f;
    Metrics.bOnline = PlayerCharacter->IsPlayerOnline();

    if (UFGInventoryComponent* Inv = PlayerCharacter->GetInventory())
    {
        TArray<FInventoryStack> Stacks;
        Inv->GetInventoryStacks(Stacks);
        auto Grouped = GetGroupedInventoryItems(Stacks);
        Metrics.InventoryJsonArray = GetInventoryJSON(Grouped);
    }

    if (UFGHealthComponent* Health = PlayerCharacter->GetHealthComponent())
    {
        Metrics.PlayerHP = Health->GetCurrentHealth();
        Metrics.bDead   = Health->IsDead();
    }

    if (AFGPlayerController* PC = PlayerCharacter->GetFGPlayerController())
    {
        float RawPitch = PC->GetControlRotation().Pitch;
        if (RawPitch > 180.f) RawPitch -= 360.f;
        Metrics.Pitch = FMath::Clamp(RawPitch, -90.f, 90.f);
    }

    if (const APlayerState* PS = PlayerCharacter->GetPlayerState())
    {
        const FUniqueNetIdRepl& NetId = PS->GetUniqueId();
        if (NetId.IsValid())
        {
            Metrics.UserID = NetId.ToString();
            if (!Metrics.UserID.IsEmpty())
                VisitedIDs.Add(Metrics.UserID);
        }
    }

    // cache write stays here if ModSubsystem is valid
    if (IsValid(ModSubsystem) && !Metrics.UserID.IsEmpty())
    {
        ModSubsystem->PlayerNameCache.Add(Metrics.UserID, PlayerName);
    }

    return Metrics;
}

Then getPlayer becomes:

AFGCharacterPlayer* PlayerCharacter = Cast<AFGCharacterPlayer>(Player);
FString PlayerName = GetPlayerName(PlayerCharacter);

if (!PlayerName.IsEmpty())
    VisitedNames.Add(PlayerName);

FLivePlayerMetrics Metrics = CollectLivePlayerMetrics(PlayerCharacter, PlayerName, ModSubsystem, VisitedIDs);

TSharedPtr<FJsonObject> LocationJson = getActorJSON(Player);
LocationJson->Values.Add("pitch", MakeShared<FJsonValueNumber>(Metrics.Pitch));

JPlayer->Values.Add("Name", MakeShared<FJsonValueString>(PlayerName));
JPlayer->Values.Add("ClassName", MakeShared<FJsonValueString>(Player->GetClass()->GetName()));
JPlayer->Values.Add("location", MakeShared<FJsonValueObject>(LocationJson));
JPlayer->Values.Add("Speed", MakeShared<FJsonValueNumber>(Metrics.Speed));
JPlayer->Values.Add("Online", MakeShared<FJsonValueBoolean>(Metrics.bOnline));
JPlayer->Values.Add("PlayerHP", MakeShared<FJsonValueNumber>(Metrics.PlayerHP));
JPlayer->Values.Add("Dead", MakeShared<FJsonValueBoolean>(Metrics.bDead));
JPlayer->Values.Add("Inventory", MakeShared<FJsonValueArray>(Metrics.InventoryJsonArray));

This keeps all safety and crash‑proofing, but shrinks getPlayer significantly.

2. Encapsulate pitch+location JSON in a helper

getPlayer currently knows that location is a JSON object you can mutate to add "pitch". You can hide this detail behind a dedicated helper:

static TSharedPtr<FJsonObject> BuildPlayerLocationJson(AActor* Player, float Pitch)
{
    TSharedPtr<FJsonObject> LocationJson = getActorJSON(Player);
    LocationJson->Values.Add("pitch", MakeShared<FJsonValueNumber>(Pitch));
    return LocationJson;
}

Usage in getPlayer:

auto LocationJson = BuildPlayerLocationJson(Player, Metrics.Pitch);
JPlayer->Values.Add("location", MakeShared<FJsonValueObject>(LocationJson));

This keeps getActorJSON untouched but makes the pitch concern local and reusable.

3. Wrap PlayerNameCache behind a subsystem API

Instead of reaching directly into PlayerNameCache (TMap) from endpoint code, add small helpers to AFicsitRemoteMonitoring. This preserves behavior but reduces cross‑module coupling and makes the offline enumeration loop cleaner.

Subsystem:

// AFicsitRemoteMonitoring.h
void RememberPlayerName(const FString& UserID, const FString& Name);
void GetCachedPlayers(TArray<TPair<FString, FString>>& OutPlayers) const;

// AFicsitRemoteMonitoring.cpp
void AFicsitRemoteMonitoring::RememberPlayerName(const FString& UserID, const FString& Name)
{
    PlayerNameCache.Add(UserID, Name);
}

void AFicsitRemoteMonitoring::GetCachedPlayers(TArray<TPair<FString, FString>>& OutPlayers) const
{
    for (const auto& Pair : PlayerNameCache)
    {
        OutPlayers.Add(Pair);
    }
}

Then in getPlayer:

TArray<TPair<FString, FString>> CachedPlayers;
if (bHasValidSubsystem)
{
    ModSubsystem->GetCachedPlayers(CachedPlayers);
    for (const auto& CachedPlayer : CachedPlayers)
    {
        const FString& UserID    = CachedPlayer.Key;
        const FString& CachedName = CachedPlayer.Value;

        if (VisitedIDs.Contains(UserID) || VisitedNames.Contains(CachedName))
            continue;

        OutJsonArray.Add(MakeShared<FJsonValueObject>(BuildOfflinePlayerJson(UserID, CachedName)));
    }
}

And inside CollectLivePlayerMetrics you call ModSubsystem->RememberPlayerName(...) instead of touching PlayerNameCache directly.

These small, targeted extractions keep all current functionality (offline entries, crash‑safe IDs, pitch, dedupe) but make getPlayer read as a sequence of high‑level steps instead of a deeply nested multi‑responsibility function.

[coredmp95]

Superseded by the upstream contribution PR against porisius:dev (the branch the maintainer targets for feature work): porisius#298. Closing this intra-fork PR.