Repository navigation
chore: enable unified netcode from N4E 7.2.0, only for hybrid prefab sessions #4186
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
base: develop-3.x.x
Are you sure you want to change the base?
Changes from all commits
40e08a8
e107a83
f18de5d
d82f061
b842d40
8ce01e5
d27e77c
2eaca3d
04b99d7
74508be
4c567d9
0267481
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,7 +289,7 @@ | |
| } | ||
|
|
||
| // Clears any registration left behind by a prior Initialize call. | ||
| if (NetworkTransform != null) | ||
| if (NetworkTransform) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We decided we liked the explicit |
||
| { | ||
| NetworkTransform.UnregisterRigidbody(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -282,7 +282,8 @@ public virtual void SetDirty(bool isDirty) | |
| if (m_IsDirty) | ||
| { | ||
| #if UNIFIED_NETCODE | ||
| if (m_NetworkBehaviour != null) | ||
| // The implicit bool skips a destroyed behaviour, whose name the warning reads; ?. would not. | ||
| if (m_NetworkBehaviour) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, we decided we prefer explicit != null checks. Makes the code easier to read. We don't need that code comment either. The AI's are terrible at reading code comments so we should just fix whatever made the AI try to make this change in the first place. |
||
| { | ||
| m_NetworkBehaviour.WarnIfNetworkVariableWrittenInPredictionLoop(Name); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we still need this to run those test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I saw that yes. In theory would be nice if those tests would run by default without any specific variable (which I guess will be true once we bump our minimal dependency on N4E to 7.2.0 right? And then we can remove this job)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. But it has caveats.
The purpose behind the unified tests is that there are more tests that we "opt out" of running as hybrid than "opt-in". It is possible, but I am marking that down as a follow up PR to this one (to avoid delaying this from merging).
Noel-Bot 🤖 has a plan that is being vetted. If you want can create a Jira ticket as a follow up to this PR with any other conditions to make that possible? Otherwise, it will be added to my "pre-GA release things to do". 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just adding to "pre-GA release things to do" will be fine