Property "controller" in "NetworkIdentity"

[ValDesu]

Minor update to the NetworkIdentity component: a new property, controller added to provide a convenient way to determine the owner, defaulting to PlayerID.Server if there is no explicit owner.

Use case:

public void SomeFunction()
    {
        if (!isController)
        {
            SomeFunction_TargetRpc((PlayerID)controller);
            return;
        }

        SomeFunction_Execute();
    }

It avoids checking if the owner is null and getting the Server instead.

Summary by CodeRabbit

• New Features
  • Networked objects now expose a resolved control identity that clearly indicates whether an object is controlled by the owning player or by the server. If the owner lacks authority or is not connected, control automatically falls back to the server, making ownership/state decisions more predictable and simplifying multiplayer synchronization.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds public PlayerID? GetController(bool ownerHasAuthority) to PurrNet.NetworkIdentity. When ownerHasAuthority is false it resolves to PlayerID.Server; when true it returns owner if connected, otherwise PlayerID.Server. No other ownership methods changed.

Changes

Cohort / File(s)	Summary
NetworkIdentity changes Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs	Added public PlayerID? GetController(bool ownerHasAuthority) to centralize controller selection: returns PlayerID.Server when ownerHasAuthority is false or when owner is null/not connected; otherwise returns the current owner.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix parameter passing in GiveOwnership #250: Also updates ownership-related behavior in NetworkIdentity.cs, touching the same owner state and ownership semantics.

Suggested labels

released on dev

Poem

🐰 I hopped through code with careful paws,
A small new method to mend the cause.
If owner sleeps or lacks the might,
Server steps in to hold the light. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title refers to adding a 'controller' property to NetworkIdentity, but the actual change implements a GetController() method instead, not a property.	Update the title to reflect the actual implementation: 'Add GetController method to NetworkIdentity' or similar, to accurately describe the method-based approach adopted.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs`:
- Line 320: Change the controller property to return a non-nullable PlayerID by
updating its return type from PlayerID? to PlayerID and return owner ??
PlayerID.Server so callers never get a nullable; update the property signature
named "controller" and ensure it references the existing "owner" field and
"PlayerID.Server" constant using the null-coalescing operator.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 436c0f52-c303-4e9d-84f5-f6a66fe9f04c

📥 Commits

Reviewing files that changed from the base of the PR and between 39ed6a6 and 1057c71.

📒 Files selected for processing (1)

• Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs

[BlenMiner]

This isn't fully correct; owner can also be disconnected with the persistent rules. So the most correct would be to check if owner && hasConnectedOwner. That being said isController is dependent on scenario, IsController(ownerAuth) is the full correct approach. So if you want a controller then having a GetController(ownerAuth) or something is required.

[coderabbitai]

♻️ Duplicate comments (1)

Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs (1)

292-299: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Return a non-nullable PlayerID from GetController.

On Line 292, the method is typed as nullable, but current branches always resolve to a concrete controller. Keeping PlayerID? preserves unnecessary null handling for callers and weakens the convenience API this PR introduces.

🔧 Proposed fix

-public PlayerID? GetController(bool ownerHasAuthority)
+public PlayerID GetController(bool ownerHasAuthority)
 {
     if (!ownerHasAuthority) return PlayerID.Server;

     if (!hasConnectedOwner) return PlayerID.Server;

-    return owner;
+    return owner.Value;
 }

#!/bin/bash
# Verify the signature and inspect call sites for nullable handling/casts.
ast-grep --pattern $'public PlayerID? GetController(bool ownerHasAuthority) { $$$ }'
rg -n -C2 --type=cs '\bGetController\s*\('

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs` around
lines 292 - 299, The method GetController currently returns a nullable PlayerID
but all code paths return a concrete PlayerID (using PlayerID.Server or owner);
change the signature from PlayerID? to PlayerID and update any call sites if
they expect null; specifically edit the public PlayerID? GetController(bool
ownerHasAuthority) to public PlayerID GetController(bool ownerHasAuthority),
keep the existing branches that return PlayerID.Server when !ownerHasAuthority
or !hasConnectedOwner and return owner otherwise, and search for usages of
GetController to remove any unnecessary null-checks/casts introduced by the
nullable signature.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs`:
- Around line 292-299: The method GetController currently returns a nullable
PlayerID but all code paths return a concrete PlayerID (using PlayerID.Server or
owner); change the signature from PlayerID? to PlayerID and update any call
sites if they expect null; specifically edit the public PlayerID?
GetController(bool ownerHasAuthority) to public PlayerID GetController(bool
ownerHasAuthority), keep the existing branches that return PlayerID.Server when
!ownerHasAuthority or !hasConnectedOwner and return owner otherwise, and search
for usages of GetController to remove any unnecessary null-checks/casts
introduced by the nullable signature.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5049cd12-6ba1-4608-a9e3-6154e75b6d96

📥 Commits

Reviewing files that changed from the base of the PR and between f7163e9 and 3c72c14.

📒 Files selected for processing (1)

• Assets/PurrNet/Runtime/Components/NetworkIdentity/NetworkIdentity.cs