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

Set record input props for all players, not just authority #309

Closed

Conversation

TheYellowArchitect
Copy link
Contributor

@TheYellowArchitect TheYellowArchitect commented Oct 16, 2024

Solves #250

Basically registers the properties in that variable. Otherwise, it is always the same as _auth_input_props and made obsolete by it. But it shouldn't be so, because _record_input_props shouldn't care about authority, just like _record_state_props doesn't care about authority.

Currently, without this PR, there is no way to get the input property entries of a RollbackSynchronizer whose authority we don't have. Which is illogical given we know them via

@export var input_properties: Array[String]

Not to mention that this variable (_record_input_props) currently exists, but is completely unused (it cannot be used without this anyway)

@TheYellowArchitect TheYellowArchitect changed the title Record input props Set record input props for all players, not just authority Oct 16, 2024
@elementbound
Copy link
Contributor

But it shouldn't be so, because _record_input_props shouldn't care about authority, just like _record_state_props doesn't care about authority.

It absolutely should care about authority. The variables _auth_state_props and _auth_input_props are the properties that we have ownership over. On the other hand, _record_state_props and _record_input_props are the properties we should record.

We record all state properties all the time, because even if we don't own those properties, we still need a history to rewind to. This is in contrast to inputs, where we don't record anything we don't own, but instead wait for those properties to arrive from the owner. This is why _record_input_props and _auth_input_props have the same content.

So imo the better call would either to remove _record_input_props, or - better yet - use that for recording inputs instead of _auth_input_props as it is now.


Currently, without this PR, there is no way to get the input property entries

You're talking about a private property, so even with this PR there wouldn't be a supported way to get input property entries. You can still grab the property names either way.

(it cannot be used without this anyway)

False, see first half of my comment.

@elementbound
Copy link
Contributor

Done in #320

@TheYellowArchitect
Copy link
Contributor Author

So imo the better call would either to remove _record_input_props, or - better yet - use that for recording inputs instead of _auth_input_props as it is now.
👍

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.

2 participants