Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
03-19-2013, 05:11 PM
Hi All,
While working on the mob AI, I found a couple of things regarding the packet handling for mobs positions (RelativeMove,Teleport packet) that were different in MCServer from the vanilla server. For example, the vanilla server sends a teleport packet every 400 ticks (or if the movement was more than 4 blocks), and a move relative packet every 60 ticks while MCserver sends a teleport packet every 2 seconds (or if it the movement was more than 4 blocks) and move relative packet on every tick. Also, the vanilla server spams the client on each tick with a Entity Velocity packet (only if there velocity is greater than zero) while the MCServer doesn't use that packet. So I was thinking that we can implement the packet handling similiar to the vanilla server so we will have less compatibility issues with the clients.
Also, I was thinking that this should be a function on the entity class, so it can be available for all entities and we will only have to maintain the code in one location. So it will be something like this:
Inside the entity class:
Code: void replicateMovement(const cClientHandle * a_Exclude = null);
What do you guys think?
Posts: 6,485
Threads: 176
Joined: Jan 2012
Thanks: 131
Given 1074 thank(s) in 852 post(s)
Sounds reasonable.
My only concern (so typical of me): will it be configurable? I think for large servers it'd be advantageous if they could refrain from sending too much data when they're overloaded, so perhaps make the velocity packet spamming "once in N ticks", where N is a server property that can be set through the INI and perhaps even programmatically (by plugins).
Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
(03-19-2013, 05:20 PM)xoft Wrote: Sounds reasonable.
My only concern (so typical of me): will it be configurable? I think for large servers it'd be advantageous if they could refrain from sending too much data when they're overloaded, so perhaps make the velocity packet spamming "once in N ticks", where N is a server property that can be set through the INI and perhaps even programmatically (by plugins).
That's a great idea. So during normal load the value of N is zero (just like the vanilla server), but once it goes under heavy load, it can go up as high as 60 ticks (more than that will be pointless due to the Move Relative packet).
I think that as the first step, we should implement it configurable via INI.
Posts: 6,485
Threads: 176
Joined: Jan 2012
Thanks: 131
Given 1074 thank(s) in 852 post(s)
The N must be larger than zero. "Every 0 ticks" is kinda wrong
So it should be set to 1.
And later on we might develop a plugin that will detect server overload and will adjust performance based on it. The plugin could then increase this N, it could maybe also limit the viewdistance for players, or limit spawning, whatever.
I don't even think INI configuration is that important, but I'm not against it; if you want it, go ahead
Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
Cool, one thing is that for this function (replicateMovement()) to properly work, the dirty flags have to be always set when there is a change. That's why I am thinking of moving the m_Pos,m_Speed, m_Rot to private members, leave the dirty flags as protected members (so any override function knows if the position/speed/rotation has changed), and make sure that anything that access these members go through the public functions
Posts: 6,485
Threads: 176
Joined: Jan 2012
Thanks: 131
Given 1074 thank(s) in 852 post(s)
Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
Cool, I am implementing the function. I thought of changing the name to:
Code: virtual void FlushToClients(const cClientHandle * a_Exclude = NULL);
It's a more meaningful name. It actually represents what the function will be doing.
Posts: 6,485
Threads: 176
Joined: Jan 2012
Thanks: 131
Given 1074 thank(s) in 852 post(s)
I don't like the fact that the new name says nothing about what it's flushing - is it the entity metadata, the movement, or even its existence? How about BroadcastMovementUpdate() ?
Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
(03-21-2013, 05:49 PM)xoft Wrote: I don't like the fact that the new name says nothing about what it's flushing - is it the entity metadata, the movement, or even its existence? How about BroadcastMovementUpdate() ?
Sure, that sounds good.
Posts: 91
Threads: 6
Joined: Dec 2012
Thanks: 1
Given 4 thank(s) in 3 post(s)
I implemented the BroadcastMovementUpdate function and moved m_Pos,m_Rot, and m_Speed to private members (wow, there were a lot of places where they were used hehe... tons of refactoring ). I test falling sand, some regular player movement and some mobs and it seems ok. So, can someone please do some more regression testing to make sure I didn't break anything?
This is the first step, now I am going to look to see if I can optimize some of the refactored code that I just did. Maybe add some helper functions like AddSpeedX, AddSpeedY, AddSpeedZ, AddPosX, AddPosY, AddPosZ, AddPos, etc...
PS: Also, I added the physics function call in the mobs class, so now they can move. They move kind of goofy, but that's the AI.
|