Skip to content

feat: other players loading progress: replacing owner-net-variables with server rpcs #630

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

Conversation

LPLafontaineB
Copy link
Contributor

Description

This draft PR shows an alternative solution to MTT-2239, using server-authoritative network variables instead of owner-writable ones. We can decide if we prefer this solution over the other one (#580 )

Issue Number(s)

Contribution checklist

  • 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

LPLafontaineB and others added 30 commits November 26, 2021 09:26
* Renamed class and file to reflect naming conventions
* Added namespace
* Replaced magic string and const with serialized fields
This validation doesn't make sense anymore since in-scene imps are now in a different scene than the NavigationSystem
Replaced cooldown coroutine with one that unloads the scene after a delay. The coroutine is stopped in OnTriggerEnter, and before starting a new one if it wasn't stopped before.
…loading

Reverted to polling to decide when to load/unload scenes instead of doing it in events to handle those intermediary states better
* Added support for non-networked game and for before the client connects
LPLafontaineB and others added 20 commits March 24, 2022 13:51
* this is to handle cases where the SceneLoaderWrapper has not spawned yet (i.e. in the case of client synchronization)
…nt finishes sync before stopping loading screen

Co-authored-by: Sam Bellomo <[email protected]>
@LPLafontaineB LPLafontaineB added the 5-Design Review Draft to review design ideas before committing to future work label Apr 27, 2022
@LPLafontaineB LPLafontaineB changed the title feat: replacing owner-net-variables with server rpcs feat: other players loading screen: replacing owner-net-variables with server rpcs Apr 28, 2022
@LPLafontaineB LPLafontaineB changed the title feat: other players loading screen: replacing owner-net-variables with server rpcs feat: other players loading progress: replacing owner-net-variables with server rpcs Apr 28, 2022
else
{
m_LocalProgress = value;
UpdateClientProgressServerRpc(NetworkManager.LocalClientId, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

should extract this, shouldn't put heavy operations in properties

Copy link
Contributor

Choose a reason for hiding this comment

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

need to keep track if value changed or not

Copy link
Contributor

Choose a reason for hiding this comment

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

need to keep track if value changed or not

else
{
m_LocalProgress = value;
UpdateClientProgressServerRpc(NetworkManager.LocalClientId, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

should extract this, shouldn't put heavy operations in properties

Base automatically changed from feature/other-players-loading-progress to develop May 3, 2022 21:19
@LPLafontaineB
Copy link
Contributor Author

Closing this because we decided the first solution was better

@LPLafontaineB LPLafontaineB deleted the feature/other-players-loading-progress-serverrpc-version branch October 3, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5-Design Review Draft to review design ideas before committing to future work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants