feat: implement tick-based synchronized NetworkVar#482
feat: implement tick-based synchronized NetworkVar#482jeffreyrainy merged 42 commits intodevelopfrom
Conversation
0xFA11
left a comment
There was a problem hiding this comment.
yep, needs to be RFC'd first :)
| // special value to indicate "No tick information" | ||
| private const ushort k_NoTick = 65535; | ||
| // Number of ticks over which the tick number wraps back to 0 | ||
| private const ushort k_TickPeriod = 65000; |
There was a problem hiding this comment.
if you just want to make sure you're never as large as k_NoTick you could k_NoTick - 1
There was a problem hiding this comment.
yeah, was trying to leave some space "just in case", but this is an internal value that could change. k_noTick - 1 would be clearer. Will do.
mattwalsh-unity
left a comment
There was a problem hiding this comment.
- Had some minor stuff and questions in the PR
- Going forward looking to have unit tests. Would you conceive and write some?
9cdd1ae to
9145e51
Compare
tests done but outside, as discussed. Does that work for you? |
I still have a todo, for the RFC, on the diagram you suggested and the comments about future possibilities. It's coming up shortly. Still, if we could unblock this PR, it would be great :-) |
I think you could submit your test component we discussed yesterday (MLAPI/Tests/Manual/...) alongside with comments explaining what you did, what happened and how you verified this feature works as intended. that'd be your sign-off on testing and verification which could be the source of truth for now, up until we have a better testing infrastructure setup IMHO. what do you think @mattwalsh-unity ? |
| void ReadField(Stream stream); | ||
| /// <param name="localTick">The local network tick at which this var was written, on the machine it was written </param> | ||
| /// <param name="remoteTick">The remote network tick at which this var was sent by the host </param> | ||
| void ReadField(Stream stream, ushort localTick, ushort remoteTick); | ||
| /// <summary> | ||
| /// Reads delta from the reader and applies them to the internal value | ||
| /// </summary> | ||
| /// <param name="stream">The stream to read the delta from</param> | ||
| /// <param name="keepDirtyDelta">Whether or not the delta should be kept as dirty or consumed</param> | ||
| void ReadDelta(Stream stream, bool keepDirtyDelta); | ||
| /// <param name="localTick">The local network tick at which this var was written, on the machine it was written </param> | ||
| /// <param name="remoteTick">The remote network tick at which this var was sent by the host </param> | ||
| void ReadDelta(Stream stream, bool keepDirtyDelta, ushort localTick, ushort remoteTick); |
There was a problem hiding this comment.
if you think this PR introduces a breaking change, we should also call that out to be more cautious I think.
9145e51 to
e66b6a1
Compare
e66b6a1 to
7f1678c
Compare
…nualtest as testing
21a6685 to
7f1678c
Compare
| --- !u!114 &2690316626396496521 | ||
| MonoBehaviour: | ||
| m_ObjectHideFlags: 0 | ||
| m_CorrespondingSourceObject: {fileID: 0} | ||
| m_PrefabInstance: {fileID: 0} | ||
| m_PrefabAsset: {fileID: 0} | ||
| m_GameObject: {fileID: 8685790303553767886} | ||
| m_Enabled: 1 | ||
| m_EditorHideFlags: 0 | ||
| m_Script: {fileID: 11500000, guid: 962d0654df408407d8055453c9020f2b, type: 3} | ||
| m_Name: | ||
| m_EditorClassIdentifier: |
There was a problem hiding this comment.
@jeffreyrainy — I think we probably shouldn't have merged this change (this is ManualNetworkedVarTest component being attached to Cube.prefab here which is not in testproject on develop branch)
I'll revert this change in one of my PRs next week.
This is the PR for sync-networked-vars RPC