feat: NetworkTransform now uses NetworkVariables instead of RPCs

[SamuelBellomo]

Refactoring NetworkTransform to use NetworkVariables instead of RPCs.
We're losing a few features with that refactor:

• interpolation should be handled by net vars themselves. still TBD, currently unhandled
• same for extrapolation
• range checks should use better aoi. still TBD
• IsMoveValidDelegate. Since this is now using a new authority enum, it doesn't make much sense to have a client authoritative network transform with a server side check. This could be revisited with an eventual prediction.

What we gain with that refactor:

• This fixes the late-join issue with objects not updating to their server position
• Authority is now explicitly defined, making it clearer. It needs to be manually set with the default being server authoritative.
• Channel is now actually used
• Snapshot work will be able to use this to test.

[SamuelBellomo]

Currently looking into writing tests based on Albin's multi instance tools

[jeffreyrainy]

Why do we need to change it?
Or, more generally, why do we still have this?

It only seems to affect v.Settings.SendTickrate = FixedSendsPerSecond;

And the SendTickrate is only used by NetworkList, NetworkSet and NetworkDictionary. As such, I'm tempted to say: let's remove FixedSendsPerSecond for NetworkTransform and simply leave v.Settings.SendTickrate at default value. If it breaks anything, then I missed something :-)

[LukeStampfli]

+1 for this. Maybe we want NetworkTransform to expose a SendTickrate property which then updates the position, rotation, scale variable at the same time? Not sure whether that's confusing though and individual control over update rate of for instance scale could be nice.

[SamuelBellomo]

To me, the fact only list set and dict are using SentTickRate is a problem by itself. I wasn't aware of that and that should be fixed.
A use case I can see for this is this:
if I have a far away object, we could have a relevancy system that reduces global tick rate. However, if I have a giant far away, I DO want to see that giant's detailed updates. I would want to override that giant's tick rate to keep it at this value.
Not sure I see a use case for having different control per variable though. Maybe if I know my scale will change in a linear way, I could save on bandwidth and have it update once per second for example and just interpolate it?

[jeffreyrainy]

If there's a valid use case for the user to set properties about priority, importance or desired rates, the user should do it at the NetworkVariable level, and Snapshot should accommodate/consider those values. Then, NetworkTransform can expose those underlying values. My point was that NetworkTransform should not attempt to drive this rate, as it kinda did in the past.

[SamuelBellomo]

yep, that's why I'm still exposing that value but not using it. I didn't want to remove too much functionality from the old NetworkTransform.
Once we have that feature available at the NetVar level, we could plug it in, what do you think?

[jeffreyrainy]

Can we elaborate, maybe with a comment in code on the plan, here? The NetworkTransform would check distance between its value what it last set on the NetworkVar and not update it unless it is large enough ? I'm curious how that plays with snapshot values received from the authoritative server.

[SamuelBellomo]

I was thinking this could be more on the network variable itself. If a server increases a Netvar int by +0.05, the netvar would actually not transmit that info and would wait for the value to be even more different.
The setting in the NetworkTransform would be to just apply it to our netvars when available? What do you think?

[jeffreyrainy]

This is a very good step. However, it puts us in a sort of transitional state where we lose interpolation. We should only merge to develop code that is complete and that we can cut a release with. In this case, if a release was made, customers would lose interpolation and get jitter.
Would it be a better idea to merge this into snapshot-system ? Probably not because we'd have the same problem at m1b.
Would it be a better idea to have a networkvariable-something branch where we put the other networkvariable changes and wait for interpolation to be present to merge to develop ?

[LukeStampfli]

IsMoveValidDelegate. Since this is now using a new authority enum, it doesn't make much sense to have a client authoritative network transform with a server side check. This could be revisited with an eventual prediction.

I disagree with this statement. IsMoveValidDelegate could still be a feature to have some level of checks when running in client authority mode. Not saying we shouldn't remove it just that I don't agree with the explanation.

Few small things I pointed out which might need fixing

[SamuelBellomo]

This is a very good step. However, it puts us in a sort of transitional state where we lose interpolation. We should only merge to develop code that is complete and that we can cut a release with. In this case, if a release was made, customers would lose interpolation and get jitter.
Would it be a better idea to merge this into snapshot-system ? Probably not because we'd have the same problem at m1b.
Would it be a better idea to have a networkvariable-something branch where we put the other networkvariable changes and wait for interpolation to be present to merge to develop ?

We could also just put this in a NetworkTransformV2 class that would live along side NetworkTransform. This way we could all start experimenting with it, without breaking the current NetworkTransform?
I really don't like having multiple "develop" branches. That makes it too easy to have code rot.

[LukeStampfli]

However, it puts us in a sort of transitional state where we lose interpolation.

I disagree, we never had interpolation in a networked sense. The implementation which existed was flawed and thus not a feature. I do not think that the quality of the position synchronization gets worse with this change.

[SamuelBellomo]

However, it puts us in a sort of transitional state where we lose interpolation.

I disagree, we never had interpolation in a networked sense. The implementation which existed was flawed and thus not a feature. I do not think that the quality of the position synchronization gets worse with this change.

If you look at the sample scene with the new NetworkTransform, movements for ghosts are jaggy. They weren't with the old NetworkTransform. Sure users could increase their tick rate (right now it's set at 0.05), but that's not the same thing. They do lose "something" even if that something is flawed.

[SamuelBellomo]

IsMoveValidDelegate. Since this is now using a new authority enum, it doesn't make much sense to have a client authoritative network transform with a server side check. This could be revisited with an eventual prediction.

I disagree with this statement. IsMoveValidDelegate could still be a feature to have some level of checks when running in client authority mode. Not saying we shouldn't remove it just that I don't agree with the explanation.

Few small things I pointed out which might need fixing

I think this goes back to our discussion on nomenclature. To me, as soon as you have server side checks, you're not client authoritative anymore.
But yeah, I see what you mean, it could be useful. It feels like any solution we'll add for server validation will be heavily influenced by whatever solution we have with our current refactors. I added an authority check here just on principle, but even that will change. I feel like this should still be removed for now.

[0xFA11]

I don't think this is "the ultimate solution" for NetworkTransform syncing but it's definitely better than what we used to have — so, I think we should have it. 🚀

[jeffreyrainy]

I don't think this is "the ultimate solution" for NetworkTransform syncing but it's definitely better than what we used to have — so, I think we should have it. 🚀

I'm curious about what you mean here. How are NetworkVariables not the best construct ? If there's any shortcomings, it would be better to list them now, before we built further on top, especially with the Snapshot system.