Skip to content

Invoke OnObserversActive when the last observer is removed - #1086

Open
jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/observers-active-last-removal
Open

jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/observers-active-last-removal

Conversation

@jimdroberts

Copy link
Copy Markdown
Contributor

Problem

NetworkObject.OnObserversActive is documented as "Called when this NetworkObject losses all observers or gains observers while previously having none." Since 4.7.0 it does not fire when the last observer is removed through RemoveObserver, for example when a client leaves range, changes scene or disconnects. It still fires on gain and on ClearObservers. The internal ObserversActiveChanged_LevelOfDetail hook is skipped in the same case. Code that pauses work for unobserved objects on this event never gets the "now unobserved" edge.

Cause

Runtime/Object/NetworkObject/NetworkObject.Observers.cs L256-264 on main:

int startCount = Observers.Count;
bool removed = Observers.Remove(connection);
...
if (removed && startCount == 0)
    ObserversActiveChanged();

If removed is true, startCount was at least 1, so the condition can never be true. In 4.7.0R (6d1754c) TryInvokeOnObserversActive(startCount) was replaced by this inline check. The old helper fired on Observers.Count == 0 && startCount > 0. The add path (L236, startCount == 0) and ClearObservers (L221, startCount > 0) were converted correctly.

Fix

startCount == 0 → startCount == 1. This is the "lost the last observer" edge. Because removed is true, it is equivalent to the pre-4.7.0 Observers.Count == 0 && startCount > 0.

Behavior change

OnObserversActive (and the level-of-detail hook) now fire when an object's last observer is removed via RemoveObserver. This is the documented behavior, and the behavior before 4.7.0.

RemoveObserver invoked ObserversActiveChanged when removed && startCount == 0,
which can never be true. Use startCount == 1, the last-observer edge, as
TryInvokeOnObserversActive did before 4.7.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant