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

[HostSync, Sync] should be Sync with arguments #6559

Closed
matekdev opened this issue Oct 5, 2024 · 5 comments
Closed

[HostSync, Sync] should be Sync with arguments #6559

matekdev opened this issue Oct 5, 2024 · 5 comments
Assignees
Labels
blocked This depends on something else before we can work on it

Comments

@matekdev
Copy link

matekdev commented Oct 5, 2024

For?

S&Box

What can't you do?

#6467

It seems like a lot of users don't know that you can use [HostSync] and [Sync] together. The only reason I found out about it was when I was reading the documentation. I think we should change the way this attribute is used.

How would you like it to work?

Here is a suggestion from Trundler #6467 (comment)
Maybe we have a single attribute where you can pass in an enum to determine who is allowed to set the value of the property?

  • Owner
  • OwnerHost
  • Host

I haven't thought about this too much, but, I'm sure you'll have a good idea.

What have you tried?

N/A

Additional context

No response

@handsomematt
Copy link
Member

[HostSync] should be a parameter on [Sync], there's a lot of other API changes we want to do around here but we're probably blocked by needing proper code upgraders.

@handsomematt handsomematt added the blocked This depends on something else before we can work on it label Oct 23, 2024
@CarsonKompon CarsonKompon moved this to Proposal in S&box Api Proposals Nov 27, 2024
@garrynewman
Copy link
Member

HostSync should certainly be [Sync( From.Host )] somehow.

With HostSync, something I'd like to be able to do is have a [Sync(host)] so the host is authoritive over the value, and authoritive over sending it to everyone else, but I'd like to be able to have changes predicted locally, and only corrected if they're wrong.

For example

[Sync( host, predicted )]
public int AmmoCount {get; set;}

// called on weapon owner, and host - somehow, maybe [Rpc( HostPredicted )] I dunno
void ShootGun( ray etc )
{
   AmmoCount--;

if ( IsHost )
{
  //    Do Damage
}
   FireWeapon();
}

In the scenario above the gun shot would happen locally and on the host, the host would be responsible for dealing damage, and would be authoritive on the ammo count.

This would obviously need some careful logic so we don't update the value on the client if it was correct at the time of prediction.

@youarereadingthis
Copy link

youarereadingthis commented Nov 27, 2024

prediction

The sooner we get functional prediction the better.

We hosted a playtest where at least 30 people arrived, and there were a number of cases leading to us making things host sided because of inherent networking issues with the current networking model. Stuff with no other rational explanation than clients just randomly not receiving/sending critical information for whatever reason.

To make a long story short: prediction would let us make a more secure, far more reliable experience. And it IS a problem. A serious one for medium-large scale multiplayer games.

@garrynewman
Copy link
Member

I do wonder whether instead of hiding all the prediction we'd be better off doing something like

[Sync( host )]
public Predicted<int> AmmoCount { get; set; }

although going down that line fruther

[Sync( host )]
public PredictedLerped<float> AmmoCount { get; set; }

@matekdev
Copy link
Author

If you want to rename this issue since it took on a life of its own feel free. I don't have anything to add yet... but hopefully I'll have some helpful thoughts on how to tackle this...

@garrynewman garrynewman changed the title [HostSync, Sync] should be more obvious to users [HostSync, Sync] should be Sync with arguments Nov 28, 2024
@garrynewman garrynewman moved this from Proposal to Approved in S&box Api Proposals Dec 10, 2024
@kurozael kurozael self-assigned this Dec 11, 2024
@kurozael kurozael moved this from Approved to Done in S&box Api Proposals Dec 14, 2024
@kurozael kurozael closed this as completed by moving to Done in S&box Api Proposals Dec 14, 2024
@github-project-automation github-project-automation bot moved this from Ready to Done in s&box tracker Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This depends on something else before we can work on it
Projects
Status: Done
Development

No branches or pull requests

5 participants