Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: networkvariablebase not being reinitialized if networkobject persists between sessions #3181

Open
wants to merge 11 commits into
base: develop-2.0.0
Choose a base branch
from

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Dec 18, 2024

This PR resolves the issue where a NetworkObject instance with a NetworkBehaviour that includes one or more NetworkVariableBase derived class properties persists between network sessions is not having its NetworkVariableBase.Initialize method invoked again when spawned again. This resulted in clients not being updated to changes in the NetworkVariableBase derived class properties for a period of time equal to the duration of the last spawned time frame from its previous network session.

MTTB-881

Changelog

  • Fixed: Issue where NetworkVariableBase derived classes were not being re-initialized if the associated NetworkObject instance was not destroyed and respawned.

Testing and Documentation

  • Includes new integration test NetworkVariableBaseInitializesWhenPersisted.
  • No documentation changes or additions were necessary.

This will "re-initialize" NetworkVariableBase derived classes when the instance is not destroyed and repurposed.
adding changelog entry
Adding PR number to changelog entry
? This test needs more comments.
…einitialized-if-networkobject-persists-between-sessions
Changing the approach to this fix while also cleaning up the initialize process so it can be invoked when the derived class is not yet fully spawned (or just recently instantiated) without potentially assigning the wrong NetworkManager instance. It is being invoked multiple times anyway, so this just gradually initializes all of the required elements until it is finally considered "initialized".
This also prevents OnInitialize from being invoked multiple times during this process and OnInitialize is only invoked as a last step to being considered "initialized".

Added a Deinitialize step to NetworkVariableBase when despawning a NetworkObject where the initialized status is reset. This allows for the next spawning of the NetworkObject (if in-scene placed or pooled) to run through the initialization process.
reverting the changes to WhenServerChangesSmoothValue_ValuesAreLerped as the recent update resolves the issue from the previous fix.
Removing exception.
Adjusting when we update the last time.
Logging warning if log level is developer and there is no NetworkTimeSystem.
remove the try catch because evidently it is the only way we can catch certain exceptions during unit testing when duplicating the value (i.e. serializing).
@michalChrobot michalChrobot added stat:awaiting triage Status - Awaiting triage from the Netcode team. port:1.x-needed This issue needs to be ported to 1.X branch and removed type:backport-release-1.0.0 stat:awaiting triage Status - Awaiting triage from the Netcode team. labels Jan 16, 2025
This validates the fix for resetting the NetworkVariableBase.LastUpdated property when a NetworkObject is persisted (i.e. recycled via in-scene placed NetworkObject or object pools).

Also minor update to prevent the integration test from logging a blank entry if there has been no logs added.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review January 22, 2025 00:00
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner January 22, 2025 00:00
The first iteration should ignore the time delta as that could be impacted by the test or VM running the test.
NoelStephensUnity added a commit that referenced this pull request Jan 24, 2025
Back-port of  #3181 final fix.
@NoelStephensUnity NoelStephensUnity removed the port:1.x-needed This issue needs to be ported to 1.X branch label Jan 24, 2025
NoelStephensUnity added a commit that referenced this pull request Jan 24, 2025
…sists between sessions (#3217)

* fix

Back-port of  #3181 final fix.

* test

The test to validate this PR.

* update

Only log something if we have something to log.

* test

remove verbose mode.

* update

adding changelog entry
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.

3 participants