search

DoubleDeez / MDFramework

A multiplayer C# game framework for Godot 3.4 Mono.
https://discord.gg/UH49eHK
MIT License
77 stars 13 forks source link

Game clock and network synch #17

Closed Beider closed 4 years ago

Beider commented 4 years ago

So I finished the second part of the Synchronizer now which is the game clock, it has a tick counter that ticks on all network clients at the same speed and it automatically keeps all clients in synch. I have added some questions at the bottom.

How it works

The game clock uses the maximum ping of any player to decide how far behind network objects should run. There is a video here, just pause at any time and you can see that the top value for the current tick is almost in synch for both clients. You can also see that the remote offset automatically adjusts as ping increases / decrease. Ensuring that remote objects are not further behind than they need to be.

Now my idea is to add two more things that use this,

  1. MDClockedNetworkDataNode Which will be a script you can just attach to a node on your networked objects. By default it has an interval for how often it sends updates. It allows you to add / get MDClockedNetworkProperty.

  2. MDClockedNetworkProperty A property that can be either Interpolated or a one shot thing. You can also set if it is always sent along with the other properties in your MDClockedNetworkDataNode or if it is sent only when you trigger it. Can be overwritten for custom properties

The idea here is that you would add this to your player and maybe add some properties like, Position (Vector) Rotation (Vector) Shoot (bool) (manually triggered, one shot)

Then the MDClockedNetworkDataNode will send updates for position and rotation along with game tick every time it updates. On the clients when you fetch the value it will automatically be interpolated for you based on the game ticks. Then for the Shoot property you can check it every frame and it will only return true on the game tick where it happened (or first game tick that is processed after this tick).

The idea here at least for my own game is to use this to ensure all networked entities synch up properly and that events are triggered at the correct time / location. Since we are interpolating between ticks and events happen at any given tick it should in theory happen exactly the same on all clients (albeit a bit behind for networked clients, based on ping).

I expect to have everything done later this week sometime.

Questions Is this something you think would fit in the framework? Does it conflict with your interpolation changes? If you think it fits, do you think the idea for MDClockedNetworkDataNode and MDClockedNetworkProperty is fine or would you rather implement this part differently?

DoubleDeez commented 4 years ago

Here's the source for my position replication component. I think it could be done more generically rather than being position/rotation specific. I was thinking would properties on the MDReplicated attribute to tune interpolation and send rate (similar to what you suggested in #10 ) https://gist.github.com/DoubleDeez/9e032411a5578264285f6c68905da8fa

I do think this idea would fit into the framework but I think we'll need to let people decide if the Property value should be sent out whenever it changes or if it should be every N frames or M seconds and interpolated on the clients between updates. Also instead of create a MDClockedNetworkDataNode and MDClockedNetworkProperty attribute, this could be made as part of MDReplicator and MDReplicated, with the settings on MDReplicated. That would kinda roll this more into #10 where the replicated properties can be tuned via the MDReplciated attribute.

Beider commented 4 years ago

The main benefit I see from adding a time component (or in this case game ticks) that is synchonized across clients is that it allows for networked objects to execute things at the right time and also be completely in synch.

The best example I can think of here is a player that is interpolated and also has a shoot signal that is sent from server to client. I had this issue in my own game, my interpolation works fine and looks smooth, but it is of course naturally a little bit behind where the actual character is on the other client.

Then when a shoot signal is sent I send the position and the direction the bullet is fired in, this causes it to instantly fire on the other client, however since the player character is behind where it is on the server when this signal arrives it just spawns in the middle of nowhere. I could send just a fire and direction however that causes the bullet to spawn in a different place on the client and the server.

This is what the time component would solve for me, then I can wait with the shoot signal until the player is moved to the right position.

Video of this problem in my own game here at around 1:30 mark

If it would be possible to have the game ticks be an optional feature of the replicator that would probably solve this, but then the replicator will also have to store the values and maybe not set a variable until the time it is supposed to be set.

Another advantage of the custom class I suggested will be that you will be able to override it and have your own custom handling code. For instance if you want a networked game where you can reverse time you would already have that possibility once you have the game ticks.

I will keep implementing my solution as I thought out for now, then once it is done maybe it is easier to see how the Replicator could be adjusted to feature the same functionality. If it could be made easier to use than the way I thought out I am all for it.

Beider commented 4 years ago

I finally got it working, I commited it on my own branch so you can check it out if you want. Still need to clean up the code a bit but it seems to work pretty well now. The only thing I think is really needed is the ability to have the network update interval be automatically adjusted based on ping (with a min/max probably), if the ping is too low and the interval is too high then the synchronization doesn't work too well.

It also auto adjusts how far behind each client is based on the maximum ping to other clients, so if the ping goes up or down the client will stay as close behind the server as possible.

If you got the time have a look at my code and maybe you have some ideas how we could make it fit better with the framework. One cool thing I like with my current setup is that in theory you could make a global synchronization node. Then have it assign networked properties to each object that need them. I didn't do that for the example, in the example each object is sending it's own update at an interval. However it wouldn't be hard to have the position of all objects being sent at the same time.

Here is a video with explanation

DoubleDeez commented 4 years ago

I haven't had a chance to look into this in detail yet but I will. One thing that came to mind though is how does it feel for clients? For example, if you're replicating movement won't the player feel the input delay?

Beider commented 4 years ago

That would be up to your game though, personally I plan on having each individual client of my game be the master of their character's position. So it will only tell the other clients where they are moving and what they are doing, in other words it will feel as responsive as single player. But my game will be a coop game so I don't really care too much about cheating, at least not as much as I would in a competetive game.

That being said if you are going with an authorative server model I would assume you could have a similar system except you would simply send position corrections back from the server if the client tries to do something illegal.

I think prediction logic and such is very hard to write into the network library itself though, since how you want to do prediction depends a lot on what you are dealing with. For instance if you have an AI character you could simply have the server send some commands and rely on the commands being executed the same way on all clients.

That being said the advantage with having each network property being a class as I have made it now vs the replicator is that you could write any sort of handling code you want within the class. As it is now it is just providing a string to be sent and it gets the same string back on the other side. So you could have a very complex class that tracks multiple things and compresses it down as much as you like.

Perhaps we could make room for both solutions in the framework? So if you just want quick and functional networking without all the hassle you use the replicator. But if you want more control we also have such a system in place.

As it stands there are probably some bugs in my code and it could need some cleanup. I just focused on getting it working for now, making it good enough for the framework would come second.

I will take some time tomorrow or sunday and write a simple top down shooter example, that would probably make it easier to get a good feel for how it works.

Beider commented 4 years ago

I finished the shooting game example, source code is already on my fork.

Here is a video of it in action.

There are two things I think needs to be added, first one is simple which is simply the ability to send data for some future tick. Currently you can use SetValue to set the value at the current tick, however being able to send for the future means you can trigger an action on all clients at a specific tick.

Second is a global interval sending class, right now each MDClockedNetworkedNode sends it's own data to itself when it's interval is up. This is nice since all it's properties that are set to be sent on an interval is sent together. However for further optimization it might be nice to be able to batch multiple networked nodes together, for instance if you are just sending position for each node on an interval it would be nice to be able to batch together 20-30 nodes and just send all positions at once.

DoubleDeez commented 4 years ago

I finally had the opportunity to watch the videos and read through the code for this. I like the concept and believe it has value in being part of the framework. I'm trying to figure out how it can be best integrated in the framework.

The goal of the framework is to make it easier for the average user to add network capabilities to their game and figuring out syncing is a fairly complex concept so I'm hoping we can get it to a point where they just need to add an attribute to their properties (or something of similar complexity) and have it synchronize.

There's some parallels between your Clocked Networking implementation and what the framework currently has and I think we can merge them a bit. This is a bit of a brainstorm so the my idea is evolving a bit as I type:

This external tracking approach does lose the ability for users to override behaviour by subclassing / implementing their own values but I think we need to design for the basic user first and add the option to customize on top of that. I think the clocked networking is very valuable and should be the default behaviour as I think it will solve a lot of problems for people that just want easy to use networking. I think it will also put us in a good position for improving interpolation/prediction.

Sorry this is so long and a bit chaotic, hopefully what I'm saying makes sense. Let me know what you think.

Beider commented 4 years ago

I generally agree with what you said, so I looked through the replicator code and tried to see how we could combine the two. So here are my suggestions.

Ideas for stuff that I am not sure if is valuable but I quite like.

Open Questions

You seem to have a better handle on C# than me so perhaps you got ideas on how this could be done better. I will not be able to put too much work into this for the next week or two because of real life commitments. Are you able to work on this in the near future and would you like to split up this work?

I would suggest if you got time that you do the base rework of the MDReplicator (extracting the interface, maybe adding the groups) without the game ticks and interpolation, then I could add additional subtypes that implements the game clock and clock based interpolation once I have time again.

DoubleDeez commented 4 years ago

I'm in the middle of a long distance move so I won't have time for the next few weeks but afterwards I'll put some work into this. My strength is in C++ so some of that has carried over into C#.

Numbered the points so it's easier to refer to them:

  1. Removing MDReplicatedType.Always sounds good.
  2. The replication groups is definitely something we should have. I agree with everything here.
  3. Is the automatic grouping based on trying to balance out the updates? Like if there are 10 properties updating at 1s intervals, the framework tries to schedule updates to be 1 every 6 frames (for 60fps).
  4. Yeah, I don't know if we should do it as flags or not for that (eg. can you opt into interpolation and prediction separately?) Either way I think the default should be auto-detected (eg. for a float that updates at interval it would be interpolated and predicted).
  5. Agreed
  6. I'm not understanding what you're suggesting here with the interfaces, do you mean something like making ReplicatedMember and implementing a version for each type? It looks like C# supports a basic version of template specialization (types have to be known at compile time to work properly)
  7. Yeah they could send their own Rpc's but I think there will still be value in the rest of the system when opting out of Game Clock.
  8. A bit confused about this one, are you wanting the ability to set a callback that is called when the peer receives an updated value for an MDReplicated property? That would be just a property with a custom setter. Or are you wanting the ability to specify a callback that is called when the master of the property changes the value?
  9. A OneShot replication is essentially just a normal RPC right? We could create a new type of RPC call that internally will track the GameClock time and send the necessary data along to be called on the peer on the right frame.
  10. My methodology for these types of problems are expose as a setting whenever possible and make the default whatever is the expected behaviour, but I'm not too sure what that would be in the this case. Would an inexperienced dev expect that?
Beider commented 4 years ago

  1. I would imagine that we try to auto balance, but also consider that there is no point in splitting it up if there are too few values. Maybe we should specify updates in physics frames instead of milliseconds since that works out more evenly with the engine.
    Example: So say you update every 6 physics frames for interval. If you only got 5 values you want to send we could just send them all every 6th frame. However once we hit some preset point, for example 10 values we split them into two groups of 5. These two groups are sent every 6th frame but 3 frames apart, so every 3 frames we send something. Then once both those group hits 10 again we split into 4 groups of 5 and that are sent on separate frames. Then finally once all hit 10 again we split into 6 groups and from there on we just keep adding while trying to keep the groups balanced.
    You could get away with two settings for this, how often you want updates which would in turn also decides how many groups you get max as we only send 1 group per frame (unless we are forced to send more because of differing settings). And what the "soft max size" for the group is, which would be the point groups are divided if we still have free groups.
    There is a second part to this that is important as well, we need to figure out how we send group definitions between clients. Since clients will need to know what is in the group and in what order the group data is. Sending this data every time the group is sent could create massive overhead which we probably want to avoid.
    I would suggest we identify each group with a sequntial int (unique per peer) and each time we modify a group we create a completely new group ID and send the definition as RPC reliable to all clients. If a client recieves an update for a group ID it doesn't know it will just hang onto it until the definition arrives. Of course this has the problem that if group definitions change often and your groups are large you end up with a lot of overhead for sending group information over the nextwork.
    Another option would be to give each replicated value an identifier and always sending the identifier along with the value. So when sending a whole group we would just send pairs of identifiers and values. Less overhead when groups change but more overhead with every packet.
    At the bottom of this class you can see an example of how i did it, in my case I assume all clients have the properties in the same order for the given datanode. So I just send all the values as arguments to an RPC call and assume the puppet has the same order as the master.

  2. I agree, let's keep it simple and enable it by default and rather allow opting out of it.

.

  1. What I mean is that we convert ReplicatedMember to an interface instead of a class, 
    public interface ReplicatedMember
{
object GetValue();
void SetValue(object Value, uint Tick = 0); // If GameClock is not active tick would just be 0
MDReplicatedType GetReplicationType();
MDReliability GetReliability();
void SubscribeToChangedEvent(OnValueChangedEventHandler handler);
}

    You can see I did something same for my own class here.

Then we can make some generic default implementation that would work on any member type. Then add a custom interpolating one (just like i did).
Here is my default implementation that works on any value, only difference from replicator is that I just convert them to strings for sending.

And here is my interpolated Vector2 which is just an extension of the base class that does interpolation.

  1. So the callback that I defined in the interface above is how I did it for my own class. On the network master the callback would be called every time the property is changed. On the client it was called every tick if interpolation was on since we would have a new interpolated value. Otherwise for non interpolated or OnChange values it was called when it was changed. Finally for OneShot it was called just on the frame it was supposed to trigger (which would be instant on the network master)
    You can see how I used it in the example here. The OnShoot is actually the same on master and puppet, while for position updates the master does it separate from the client as master is using MoveAndSlide(). The thing that makes it nice is that I don't need to check for value changes in PhysicsProcess() on the puppets. They just get their changes through the callback. I should probably just have disabled processing for the puppets to not waste the processing time for the single bool check every frame.
    Of course since with the replicator you would be replicating a member of the class you could choose how to do it depending on the use case. If you got something like position maybe you want to just do it in PhysicsProcess() while if you got something that rarely changes you do it in the callback.

  2. Sounds like a good solution

  3. Maybe just leave it as is. It wouldn't be hard to add separate estimation per client in the game clock in the future if we feel it is necessary.

DoubleDeez commented 4 years ago

For 3: The auto balancing algorithm sounds good. For the second part I had not thought of the issues here. Why do the group IDs need to be networked? It's just a concept the replicator will use to schedule updates right? As for setting values, we can do the inverse of what I did in the replicator for checking for changes via reflection. We will still have to send a NodePath or ID that we setup ourselves (likely with a solution similar to what you proposed for Group IDs but instead it would be Node IDs). Then using the C# reflection system we can set the values we receive by RPC. For 6: Okay yeah, this is what I thought. I'll look into solutions on how this can be accomplished best in C# when I have some time. In C++ we would have a default template with template specializations where needed. For 8: Right, but with the change of making clocked network values externally tracked, the equivalent in the new setup for position would be

[MDReplicated]
Vector2 NetworkedPosition;

then you could instead use a custom setter to do your OnPositionChanged call

    protected Vector2 _position;
    [MDReplicated]
    protected Vector2 NetworkedPosition {
        get
        {
            return _position;
        }
        set
        {
            _position = value;
            OnPositionChanged();
        }
    }

Adding a callback param to MDReplicated would just be syntactic sugar at that point, which has value but isn't functionally necessary.

Beider commented 4 years ago

Number 3. What I was mainly thinking about was sending the entire group as a single RPC call to reduce overhead by not sending as many packets. That being said, I don't know if the underlying network implementation already does something like this. But overall there is no point in this optimization if it works without doing so, so just assigning a unique id to each replicated property and making sure all clients have that id would probably be good enough.

About 8, yeah I suppose it is mostly just a convenience so that you would not have to do that. I just thought it would have looked cleaner if your looked like

[MDReplicated(MDReliability.Reliable, MDReplicatedType.Interval, OnPositionChanged)]
protected Vector2 Position;

Finally let me know what you figure out for 6, I only have a few hours this week to do work on my godot game so I will probably spend some time actually working on my game as I spent all my time lately on networking.

DoubleDeez commented 4 years ago

3: Yeah I want to avoid optimizing in areas that should be optimized in the engine itself as that kind of detracts from the point of the framework of being an addition to the Godot networking.

8: yeah exactly, I'm not saying we don't do it, just that it isn't a requirement.

and yeah I'll update here with anything I find for solutions to 6

Beider commented 4 years ago

Since I had a bit of time today I decided to modify the replicator to use an interface to see how this would look.

Here is the code on my branch.

You can see I moved most of the logic inside the replicated node, this way we could make a different instance of it that uses the game clock and simply instance the correct version depending on if game clock is active or not.

The interface for the replicated member turned out way simpler than I thought,

interface IReplicatedMember
{
    bool ShouldReplicate();

    void Replicate(int JoinInProgressPeerId);

    MDReplicatedType GetReplicatedType();

    bool IsReliable();
}

If this is the approach we want to go for I would move the ReplicatedMember to it's own class MDReplicatedMember and introduce a new MDClockedReplicatedMember.

Then I would probably introduce a third replicated class that does interpolation and prediction.

I think the code could need some adjustments as well but in general I think this concept would work quite well, it also allows people to implement their own replication classes. I was thinking of making the picking of which replicated type to use a virtual method and add the standard virtual replicater type to the game instance. Then people could just override the replicator if they want to have custom replication implementations.

DoubleDeez commented 4 years ago

I'm thinking we should have MDReplicatedMember as a base then it could be extended for additional functionality (adding game clock awareness, interp, etc). Then we would also need some form of factory so that custom ReplicatedMember types could be added by framework users without changing the framework

Beider commented 4 years ago

Yeah I was thinking of having MDReplicatedMember be the base for MDClockedReplicatedMember. I guess I could remove the interface in that case and we just override it. Also considering the rest of the framework generally use virtual methods I was thinking of just adding something like this below to MDReplicator.

protected virtual IReplicatedMember CreateReplicatedMember(MemberInfo Member, MDReplicated RepAttribute, Node Instance)
{
    return new ReplicatedMember(Member, RepAttribute.Reliability == MDReliability.Reliable, RepAttribute.ReplicatedType, WeakRef.WeakRef(Instance));
}

And then adding this to the GameInstance

protected virtual Type GetReplicatorType()
{
    return typeof(MDReplicator);
}

I will also need to make the MDReplicator a Node as clocked values can't just use Rset directly, so I need to send the values to the replicator first then pass them to the ReplicatedMember, so I would instance the MDReplicator from the GameInstance like everything else.

I think I will have a little bit more time than I estimated over the next few days so I will see what I can get done.

DoubleDeez commented 4 years ago

Ah yeah, the clock stuff will need RPCs on the replicator so that makes sense.

Beider commented 4 years ago

So I finished the initial implementation of this now, it was fairly easy to get stuff to work. However there are probably bugs and I could really need you to look over the reflection part as I think it may not be optimal (nor possibly quite stable), but that can probably wait a bit. I also saw mentioned that reflection is pretty slow in C# and that you can improve it with something like FastMember, might be worth looking into?

Anyway here is my new replicator.

I introduced a new NetworkKeyIdMap class that is managed by the server, it contains uint keys that correspond to NodePath#MemberName strings that I use in the KeyToMemberMap. This allows it to only send a uint over the network for replication instead of the whole path. I also added a buffer in case values arrive before the key pair.

I also adjust so we now use interval, it is currently set to 300 which I used for testing but I guess a more sensible default would be 100 milliseconds.

Inside CreateReplicatedMember it detects if game clock is active and then it looks for Vector2 members and applies the interpolated vector2 implementation if they are found, if not standard clocking is used.

Finally the individual implementations are here. They are pretty similar to what I already made for my old replication.

I will probably work a bit more on this tomorrow trying to clean it up a bit and look for bugs, etc.. But this will certainly need a proper review and we probably need more tests. I think for now I found a bug that already existed in the replicator, OnChange values are not sent to new players when they join. So if it never changes after they joined they will never get the value.

My todo list currently is

Stuff that should be considered later

DoubleDeez commented 4 years ago

I think the slow part of reflection is going from strings to Members and FastMember helps that but we don't do that since we store the MemberInfos which should be reasonably fast.

I took a quick glance over the Replicator and the ReplicatedMember types and they look good, just some minor things I would call out in a review once it's up.

The OnChange bug sounds right, last time I was working on this I remember there being some join in progress issues so that's probably it.

For OneShot I was thinking we do something along the lines of extending the MDRpc* functions to check if the gameclock is enabled and if it is, it instead calls an Rpc on the GameSession node, giving it all the info to call the function locally with the tick number and stuff.

What do you mean by "Easy to use change listener instead of having to add this yourself"?

Beider commented 4 years ago

What do you mean by "Easy to use change listener instead of having to add this yourself"?

What I mentioned above about being able to do a listener directly in the declaration so you don't have to make a property and do the whole set thing. It just looks much cleaner this way

[MDReplicated(MDReliability.Reliable, MDReplicatedType.Interval, OnPositionChanged)]
protected Vector2 Position;

And yeah custom RPC implementation that respects game ticks would be an appropriate solution. I will add that to my todo list.