Ensure players always have the alias they were created from

[tudddorrr]

No description provided.

[tudddorrr]

/oc review

[tudddorrr]

Review: PR #196 – Ensure players always have the alias they were created from

Overall: This is a reasonable change that adds defensiveness around the CurrentAlias setter. There is one significant gap where the guard logic is bypassed.

---

🟡 Major: CurrentPlayer setter bypasses the alias initialization guard

File: Assets/Talo Game Services/Talo/Runtime/Talo.cs, line 61

The new guard logic in the CurrentAlias setter (lines 48–55) ensures that when a PlayerAlias is assigned, its player.aliases array is populated if empty. However, the CurrentPlayer setter on line 61 bypasses this entirely:

public static Player CurrentPlayer
{
    get => _currentAlias?.player ?? null;
    set => _currentAlias.player = value;  // bypasses CurrentAlias setter
}

Why this matters: The only usage of the CurrentPlayer setter is in PlayersAPI.Update() (line 165):

Talo.CurrentPlayer = res.player;

This directly replaces _currentAlias.player with the player object returned from the server after a PATCH update. If the API response's player object has a null or empty aliases array (which is precisely the scenario this PR aims to prevent), the guard logic in CurrentAlias won't execute because CurrentPlayer setter writes directly to _currentAlias.player, not through the CurrentAlias property setter.

A subsequent call to Player.GetAlias() (which does aliases.Length > 0) would then throw a NullReferenceException if aliases is null.

Suggested fix: Apply the same guard logic inside the CurrentPlayer setter, or have PlayersAPI.Update() reassign through CurrentAlias instead:

public static Player CurrentPlayer
{
    get => _currentAlias?.player ?? null;
    set
    {
        _currentAlias.player = value;
        if (value != null && (value.aliases?.Length ?? 0) == 0)
        {
            value.aliases = new[] { _currentAlias };
        }
    }
}

---

✅ No issues found with

• The guard logic itself – correctly handles null and empty arrays.
• The readonly modifier on _crypto, _continuity, _socket – these are private fields assigned once in the static constructor; making them readonly is appropriate and safe.
• Backwards compatibility – the change is additive and doesn't break existing public API contracts.

---

Summary

Severity	Issue	Action
🟡 Major	CurrentPlayer setter bypasses alias initialization guard	Apply guard logic in CurrentPlayer setter or redirect through CurrentAlias

The PR is a net positive and safe to merge after addressing the CurrentPlayer setter gap.