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.

Sign up for GitHub

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

Jump to bottom

Fix - Optimized NetworkTransform on all networked prefabs in the Invaders sample [MTT-7599] #168

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Fix - Optimized NetworkTransform on all networked prefabs in the Invaders sample [MTT-7599] #168

wants to merge 6 commits into from
+461 −214

Conversation

bastienunity
Copy link

@bastienunity bastienunity commented May 3, 2024 •
edited

Description

The gameplay feeling is very different on clients vs. host in the Invaders Sample.
This PR is changing the interpolation of netwrok transforms to false in order to fix the issue.
Because the game is not using any physics or animations to move transforms in the scene, the server itself is not doing any interpolation between positions and simply forces the new transform position of enemies or bullets. Removing the interpolation prevents the client from smoothing that movement locally and makes all movements looks like they are behaving the same on the host and any client.

While updating the NetworkTransform, we also removed most of the sync values of the transform to keep only the ones that changes during the gameplay.
Players and SuperEnemy only need to sync their postion.x because they can't move vertically.
Enemies only need to sync their position.x and position.y.
Shields don't move, their NetworkTransform has been removed.
Bullets only need to sync their position.y because they only move vertically.

Before changes Host vs. Client

pre-changes-invaders.mp4

After these changes Host vs. Client

post-changes-invaders.mp4

Issue Number(s)

MTT-7599 - Turned interpolation to false on all the networked transform to make movements between host and client render movements the same way.

Contribution checklist

  • Tests have been added for the project and/or any internal package
  • Release notes have been added to the project changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink

@bastienunity
Optimize NetworkTransform so they do not interpolate and do not sync …
635dcb6 
…unecessary transform data

The server is not interpolating the ennemies position and given the game doesn't have any animation, not interpolating anything allows for the same movement feeling between clients and host.
@bastienunity
Further refine network transform optimizations.
432feee
@bastienunity bastienunity requested a review from a team as a code owner May 3, 2024 18:44
@bastienunity bastienunity changed the base branch from main to develop May 3, 2024 18:47
Elfi0Kuhndorf
Elfi0Kuhndorf previously approved these changes May 6, 2024
View reviewed changes
Copy link

@Elfi0Kuhndorf Elfi0Kuhndorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks smooth!

fernando-cortez
fernando-cortez previously approved these changes May 6, 2024
View reviewed changes
Copy link
Collaborator

@fernando-cortez fernando-cortez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure that they were improvements, but what were the changes to Invaders Game Manager and SceneTransitionHandler prefabs?
Just listed would be fine.

@bastienunity
Copy link
Author

@fernando-cortez I think they got reserialized because I touched them, but haven't changed anything, the diff is only the component order that changed, not sure why.
I can revert them to avoid the noise in the PR.

@fernando-cortez
Copy link
Collaborator

@fernando-cortez I think they got reserialized because I touched them, but haven't changed anything, the diff is only the component order that changed, not sure why.
I can revert them to avoid the noise in the PR.

Ah that makes sense! I think that's fine if they go through.

RikuTheFuffs
RikuTheFuffs requested changes May 14, 2024
View reviewed changes
Basic/Invaders/Assets/Prefabs/bullet.prefab
@@ -10,10 +10,10 @@ GameObject:
m_Component:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 2859882261451710602}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastienunity it seems the order of scripts was changed here as well, is this intended? If not, can you revert it to avoid potential issues?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, funny story... When selecting the prefab, the Inspector refreshes after one frame and moves the NetworkObject component in third place on every single Prefab I select in the project. So any further changes made to the prefab will save the NetworkObject component position changed...
I can manually revert that change outside of Unity, but because it is forced and automatic, I think it's better to just commit it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Netcode is forcing reserialization, and we're going to keep running into this change in subsequent PRs, this can go through.
Let's expose our code to potential race conditions and address them, if they arise!

Basic/Invaders/Assets/Prefabs/enemy1.prefab
@@ -10,11 +10,11 @@ GameObject:
m_Component:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 2037077820506562951}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Basic/Invaders/Assets/Prefabs/enemy2.prefab
@@ -10,11 +10,11 @@ GameObject:
m_Component:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 1361474663871475650}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Basic/Invaders/Assets/Prefabs/enemy3.prefab
@@ -10,11 +10,11 @@ GameObject:
m_Component:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 7651551376209856180}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Basic/Invaders/Assets/Prefabs/enemyBullet.prefab
@@ -11,9 +11,9 @@ GameObject:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 6100000}
- component: {fileID: 2336033219118315817}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Basic/Invaders/Assets/Prefabs/player.prefab
@@ -10,10 +10,10 @@ GameObject:
m_Component:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 5053919295125167455}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Basic/Invaders/Assets/Prefabs/shield.prefab
@@ -14,7 +14,6 @@ GameObject:
- component: {fileID: 6100000}
- component: {fileID: 11400000}
- component: {fileID: 6564576217164106930}
- component: {fileID: 8844355272195160781}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what component was removed here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the NetworkTransform, Shields are spawned and destroyed but never move, so they don't need to sync their Transform at all.

Basic/Invaders/Assets/Prefabs/superEnemy.prefab
@@ -11,11 +11,11 @@ GameObject:
- component: {fileID: 400000}
- component: {fileID: 21200000}
- component: {fileID: 5000000}
- component: {fileID: 3955316467729786863}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RikuTheFuffs RikuTheFuffs RikuTheFuffs requested changes

@Elfi0Kuhndorf Elfi0Kuhndorf Elfi0Kuhndorf left review comments

@fernando-cortez fernando-cortez Awaiting requested review from fernando-cortez

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bastienunity @fernando-cortez @RikuTheFuffs @Elfi0Kuhndorf