MFirstPerson / $firstPerson sync between client and server
by Aaron Oneal · in Torque Game Engine · 09/21/2005 (3:49 am) · 30 replies
I'm afraid I'm missing something really obvious, but how are the mFirstPerson variables kept in sync between the client and server? It seems to me that the current implementation relies completely on the client and server being local and sharing the variable. Even worse, every connection shares the variable since it's a static global, so even if they were being synced, every client would get updated or could toggle the server copy. Am I missing something really obvious or does the handling of mFirstPerson need to be totally rewritten?
About the author
#2
$mFirstPerson is a client side only Script variable, and is never networked, so no other clients (or the server) are aware of it.
09/21/2005 (8:10 pm)
MFirstPerson is a member variable of the Player class, and therefore is unique to every instantiation of a Player object--therefore, it's not "shared".$mFirstPerson is a client side only Script variable, and is never networked, so no other clients (or the server) are aware of it.
#3
mFirstPerson is a static member of the GameConnection class (I'm not referring to the Player object variable). It is bound in GameConnection::consoleInit() as follows:
It is then used in getCameraObject() for determining whether or not to return the camera or the control object.
So if I'm on the client and I toggle that variable to false, it will still be true (the default setting) on the server unless my game instance is both client and server and therefore sharing that static variable. And since they're sharing that static variable, mFirstPerson on the server will be toggled for all clients if the local client toggles it, even if it isn't replicated out to all the other connected client instances.
Again, unless I'm missing something (which would be a blessing since I'm working on a 3rd person chasecam), this is not the correct way for this flag to be handled (assuming it's even required). It needs to be a member variable, not static, and its state needs to be replicated between client and server so that:
1) State is the same on both client and server for the client in question
2) Changes to this flag on one client do not affect the client or server copies of this flag for other clients
09/22/2005 (10:21 am)
I'm fairly certain there's a problem here, please check over this carefully.mFirstPerson is a static member of the GameConnection class (I'm not referring to the Player object variable). It is bound in GameConnection::consoleInit() as follows:
Con::addVariable("firstPerson", TypeBool, &mFirstPerson);It is then used in getCameraObject() for determining whether or not to return the camera or the control object.
ShapeBase* GameConnection::getCameraObject()
{
// If there is no camera object, or if we're first person, return
// the control object.
if( !mControlObject.isNull() && (mCameraObject.isNull() || mFirstPerson))
return mControlObject;
return mCameraObject;
}So if I'm on the client and I toggle that variable to false, it will still be true (the default setting) on the server unless my game instance is both client and server and therefore sharing that static variable. And since they're sharing that static variable, mFirstPerson on the server will be toggled for all clients if the local client toggles it, even if it isn't replicated out to all the other connected client instances.
Again, unless I'm missing something (which would be a blessing since I'm working on a 3rd person chasecam), this is not the correct way for this flag to be handled (assuming it's even required). It needs to be a member variable, not static, and its state needs to be replicated between client and server so that:
1) State is the same on both client and server for the client in question
2) Changes to this flag on one client do not affect the client or server copies of this flag for other clients
#4
I interpret this code as: allowing a way for clients to force 1st person - it's basically a preference. All the important stuff is already replicated properly.
09/22/2005 (3:41 pm)
Can you make a reproduction case where this doesn't work as expected?I interpret this code as: allowing a way for clients to force 1st person - it's basically a preference. All the important stuff is already replicated properly.
#5
Yes, it probably could be networked and avoid having to do it manually, but since it is a persistent field instead of a member variable that is accessed via an accessor method + console method, it is the developers responsibility to make sure the flags stay synched.
09/22/2005 (6:27 pm)
And if you will not how it is used, you will see that every time a camera mode is changed (via a commandToServer request), the local variable is also changed on the client so that they stay in synch.Yes, it probably could be networked and avoid having to do it manually, but since it is a persistent field instead of a member variable that is accessed via an accessor method + console method, it is the developers responsibility to make sure the flags stay synched.
#6
Scenario:
1) Client A hosts a networked game and joins the mission.
2) A camera object is assigned to Client A's game connection (this assignment is correctly synced during packet writes).
3) The static firstPerson variable is in its default state of true.
4) Calls to getCameraObject() on ClientA and the Server return the Control object because firstPerson is true.
5) Client B joins the mission, also has a camera assigned, also in firstPerson.
6) Client A tabs to set firstPerson to false. This sets firstPerson for him (Client A) and the Server to false since they are the same static variable. Every GameConnection on Client A's machine now has firstPerson set to false since they all share the same static variable.
7) So, on Client A and every to client connection on Client A's Server, calls to getCameraObject() now return the Camera object. However, calls to getCameraObject() on Client B still return the Control object because client B has not changed the firstPerson variable.
This means, the server state of firstPerson is out of sync with the client state of firstPerson, so camera transforms used downstream for rendering, collision detection, etc. are all broken. The control object may be facing one direction while the camera is facing an entirely different direction. The client thinks one thing and the server thinks something else. That's a serious issue for anyone implementing a 3rd person camera in a networked environment.
For local connections the static variable ironically works in your favor and keeps client/server in sync, thus keeping the real problem from being visible. But as soon as you go multiplayer with a camera object that needs to keep client/server state synced, the whole thing goes down the drain.
09/22/2005 (6:48 pm)
I seem to be having a difficult time getting my point across. Please understand we're talking about a static firstPerson variable, so the instance is shared by both the client and server. That breaks the following situation, which I find very important.Scenario:
1) Client A hosts a networked game and joins the mission.
2) A camera object is assigned to Client A's game connection (this assignment is correctly synced during packet writes).
3) The static firstPerson variable is in its default state of true.
4) Calls to getCameraObject() on ClientA and the Server return the Control object because firstPerson is true.
5) Client B joins the mission, also has a camera assigned, also in firstPerson.
6) Client A tabs to set firstPerson to false. This sets firstPerson for him (Client A) and the Server to false since they are the same static variable. Every GameConnection on Client A's machine now has firstPerson set to false since they all share the same static variable.
7) So, on Client A and every to client connection on Client A's Server, calls to getCameraObject() now return the Camera object. However, calls to getCameraObject() on Client B still return the Control object because client B has not changed the firstPerson variable.
This means, the server state of firstPerson is out of sync with the client state of firstPerson, so camera transforms used downstream for rendering, collision detection, etc. are all broken. The control object may be facing one direction while the camera is facing an entirely different direction. The client thinks one thing and the server thinks something else. That's a serious issue for anyone implementing a 3rd person camera in a networked environment.
For local connections the static variable ironically works in your favor and keeps client/server in sync, thus keeping the real problem from being visible. But as soon as you go multiplayer with a camera object that needs to keep client/server state synced, the whole thing goes down the drain.
#7
09/22/2005 (6:59 pm)
@Stephen: You must be referring to toggleCamera(). Yes, that works great and is probably what Ben is referring to as the "important stuff". This issue is strictly related to the use of firstPerson and how it affects calls to getCameraObject(). See toggleFirstPerson() and how it toggles the static variable mFirstPerson of GameConnection which affects every connection on the machine, client and server alike. The current implementation just isn't network safe.
#8
2) Even when you are the host of a multi-player game, you have your objects ghosted to you from the "server" side. No code in stock torque actually changes any server instantiation of mFirstPerson (ironically, I think you've pointed out a design issue, but not an implementation one--mFirstPerson never actually changes server side).
3) "Client side" code (with the single exception of purely single player games, where I fully admit there are some client side/server side issues IF you do not follow the strict server-client implementation requirements of the networking model) changes to firstPerson are never transmitted to the server side, so what client A, B, whatever does with their $firstPerson variable, others don't know about it.
If you are actually -seeing- the things you've described, can you confirm you see them in a pure stock implementation? The code that performs these operations has been in Torque for years, and I've never seen the issue you are talking about, much less not seen anything like this reported. Realm wars, tribes, etc. have all had this code as far as I am aware for quite a long time.
09/22/2005 (7:25 pm)
1) camera transforms should never be used for collision, ever. In addition, most of the time no collision detection should ever be done on the client (barring rare circumstances like object selection code with a click and pick implementation).2) Even when you are the host of a multi-player game, you have your objects ghosted to you from the "server" side. No code in stock torque actually changes any server instantiation of mFirstPerson (ironically, I think you've pointed out a design issue, but not an implementation one--mFirstPerson never actually changes server side).
3) "Client side" code (with the single exception of purely single player games, where I fully admit there are some client side/server side issues IF you do not follow the strict server-client implementation requirements of the networking model) changes to firstPerson are never transmitted to the server side, so what client A, B, whatever does with their $firstPerson variable, others don't know about it.
If you are actually -seeing- the things you've described, can you confirm you see them in a pure stock implementation? The code that performs these operations has been in Torque for years, and I've never seen the issue you are talking about, much less not seen anything like this reported. Realm wars, tribes, etc. have all had this code as far as I am aware for quite a long time.
#9
Let me respond to your points.
1) That's just not true for a 3rd person chasecam, and I don't mean the kind that comes out of the box since that's not a real 3rd person chasecam, it's based off of the transform of the control object. For a 3rd person camera not based off of control object transforms, of course the camera transforms have to be used for collision since you must cast rays to determine where the camera should be and what it should be looking at, and that's not right behind the player's head. A tethered 3rd person chase camera is not always directly behind the player and often pointing in a completely different direction.
Control objects in a 3rd person chase mode also move relative to the camera's orientation, so the camera transforms are used by the control object to determine the direction to move. If you've never worked with this kind of camera, I suggest checking out some of Cory's resources which do a great job indicating the areas of the engine that get used when implementing something like this. If you haven't done it, I can see why you've never encountered this kind of issue.
2) This also isn't true. Are you familiar with static variables? If someone is both a client and hosting a server, both the client and server share the same instance of mFirstPerson because every GameConnection on the box (well, that instance of the game) uses that same variable. It absolutely is toggled on the server. Even worse, every outgoing client connection from that client's server gets toggled because they all share the same instance. I have emphasized this static issue over and over, but your responses seem to disregard the static variable declaration and assume it's a non-static member variable -- which is the correction I'm suggesting be made to make it work like you seem to think it already does. Please clarify that you understand the variable declaration is static and the implications that has for all GameConnections on a shared client/server machine.
3) This happens only with a second party client right now as my scenario example showed with Client B. Client A and his server are synced due to a fluke in the design -- the fact that mFirstPerson is declared static.
I am looking at the pure stock / clean 1.4 RC2 code and it is a problem. There's no doubt this has worked in the past in the games you mentioned because of the following reasons:
1) The 3rd person camera in those games is not a true 3rd person camera but an offset from the control object.
2) Camera transforms were not used for collisions (other than scoping).
3) Control object / player movement was absolute, not relative to the camera object.
You simply can not implement a real 3rd person chasecam with the current implementation and expect it to work correctly in a multiplayer environment. Obviously I have the source code and know enough about the issue to fix this for my own project, but I'm trying to help everyone out here by getting the problem acknowledged and fixed in HEAD. I know if you'll take the time to seriously look at the places in the code I've indicated, and the usage I'm talking about, you'll see the issue plain as day.
09/23/2005 (5:48 am)
Stephen, please don't take this the wrong way, I'm trying to help everyone here, but you're missing some very key points of what I'm trying to tell you guys. I know you're reluctant to admit a problem with such a long standing piece of code, but please take this seriously, it is a critical issue for anyone creating a game that uses a 3rd person camera ala Zelda or Mario 64 -- something none of those games you mentioned do.Let me respond to your points.
1) That's just not true for a 3rd person chasecam, and I don't mean the kind that comes out of the box since that's not a real 3rd person chasecam, it's based off of the transform of the control object. For a 3rd person camera not based off of control object transforms, of course the camera transforms have to be used for collision since you must cast rays to determine where the camera should be and what it should be looking at, and that's not right behind the player's head. A tethered 3rd person chase camera is not always directly behind the player and often pointing in a completely different direction.
Control objects in a 3rd person chase mode also move relative to the camera's orientation, so the camera transforms are used by the control object to determine the direction to move. If you've never worked with this kind of camera, I suggest checking out some of Cory's resources which do a great job indicating the areas of the engine that get used when implementing something like this. If you haven't done it, I can see why you've never encountered this kind of issue.
2) This also isn't true. Are you familiar with static variables? If someone is both a client and hosting a server, both the client and server share the same instance of mFirstPerson because every GameConnection on the box (well, that instance of the game) uses that same variable. It absolutely is toggled on the server. Even worse, every outgoing client connection from that client's server gets toggled because they all share the same instance. I have emphasized this static issue over and over, but your responses seem to disregard the static variable declaration and assume it's a non-static member variable -- which is the correction I'm suggesting be made to make it work like you seem to think it already does. Please clarify that you understand the variable declaration is static and the implications that has for all GameConnections on a shared client/server machine.
3) This happens only with a second party client right now as my scenario example showed with Client B. Client A and his server are synced due to a fluke in the design -- the fact that mFirstPerson is declared static.
I am looking at the pure stock / clean 1.4 RC2 code and it is a problem. There's no doubt this has worked in the past in the games you mentioned because of the following reasons:
1) The 3rd person camera in those games is not a true 3rd person camera but an offset from the control object.
2) Camera transforms were not used for collisions (other than scoping).
3) Control object / player movement was absolute, not relative to the camera object.
You simply can not implement a real 3rd person chasecam with the current implementation and expect it to work correctly in a multiplayer environment. Obviously I have the source code and know enough about the issue to fix this for my own project, but I'm trying to help everyone out here by getting the problem acknowledged and fixed in HEAD. I know if you'll take the time to seriously look at the places in the code I've indicated, and the usage I'm talking about, you'll see the issue plain as day.
#10
I explained why it isn't networked...why it's static I honestly don't know. Change it to not be static, or change it so that it's not a persistent field but networked instead...you have the source code.
09/23/2005 (10:03 am)
So I guess what you are telling me is that the orbit cam implementation on my dedicated client/server environment doesn't work. Even though we've seen it work many times :)I explained why it isn't networked...why it's static I honestly don't know. Change it to not be static, or change it so that it's not a persistent field but networked instead...you have the source code.
#11
09/23/2005 (10:18 am)
Of course the orbit cam works because it's set as the control object, not the camera object. You're not reading what I'm posting, you're making assumptions, and bad ones. You clearly still don't have a grasp of what the static variable is going to do on a shared client/server and your examples still aren't taking into account that I'm referring to what happens in getCameraObject() as a result. Why are you so eager to wash your hands of this instead of just taking a look at what I'm talking about?
#12
Many, many, MANY people have made the very simple code changes that are part of any implementation to have things work for them perfectly for their project.
You are absolutely correct, it could be implemented differently/more correctly. It is also of trivial importance given that there are so many workarounds.
There are at last count 4 different resources that implement different camera types, and each accomplishes things correctly...so pick one and use it!
If you feel this strongly about it, post a resource, and when someone else comes into the exact same thing that you have, they will have something to look at!
Any by the way, please do not make assumptions about what we have and have not looked at. I not only studied the code, I also looked at two different resources, as well as my own personal code while researching your posts.
09/23/2005 (10:39 am)
And you still aren't listening to what I am saying:Many, many, MANY people have made the very simple code changes that are part of any implementation to have things work for them perfectly for their project.
You are absolutely correct, it could be implemented differently/more correctly. It is also of trivial importance given that there are so many workarounds.
There are at last count 4 different resources that implement different camera types, and each accomplishes things correctly...so pick one and use it!
If you feel this strongly about it, post a resource, and when someone else comes into the exact same thing that you have, they will have something to look at!
Any by the way, please do not make assumptions about what we have and have not looked at. I not only studied the code, I also looked at two different resources, as well as my own personal code while researching your posts.
#13
You're implying that using another resource to implement a different camera type will avoid the issue. That is also not a correct assumption, it does not.
All of the resources implementing external camera types that rely upon set/getCameraObject() are going to suffer from this issue. This is not something that happens in just my particular situation, and it's a glaring defect in the code that is going to cause problems. Why would you not want the code in HEAD to be correctly implemented? And yes, I say correctly, because what's there is an implementation based on flawed design. I don't believe the full extent of the problem has been understood or you would be a lot more concerned about making it right.
09/23/2005 (11:00 am)
You're implying that I've made some change to the code or screwed up "simple code changes" to make this not work in my case and that somehow other people had no problem. That is not a correct assumption.You're implying that using another resource to implement a different camera type will avoid the issue. That is also not a correct assumption, it does not.
All of the resources implementing external camera types that rely upon set/getCameraObject() are going to suffer from this issue. This is not something that happens in just my particular situation, and it's a glaring defect in the code that is going to cause problems. Why would you not want the code in HEAD to be correctly implemented? And yes, I say correctly, because what's there is an implementation based on flawed design. I don't believe the full extent of the problem has been understood or you would be a lot more concerned about making it right.
#14
If you could provide a patch that fixes this case whilst also allowing the tab and other existing functionality to work properly, I'll gladly review it for inclusion in trunk. Time is short, though, as 1.4 final will be going out soon, at which point bug fixes won't get pushed out to the installers until the next Torque update. I think you do have a bug here, but I may not have time to review it fully on my own, without a patch, before 1.4 ships - it's not a showstopper, and at this point something like 200 issues have already been resolved on 1.4. 1.4 needs to get out sometime! :)
Thanks,
Ben
09/23/2005 (10:49 pm)
Hey Aaron,If you could provide a patch that fixes this case whilst also allowing the tab and other existing functionality to work properly, I'll gladly review it for inclusion in trunk. Time is short, though, as 1.4 final will be going out soon, at which point bug fixes won't get pushed out to the installers until the next Torque update. I think you do have a bug here, but I may not have time to review it fully on my own, without a patch, before 1.4 ships - it's not a showstopper, and at this point something like 200 issues have already been resolved on 1.4. 1.4 needs to get out sometime! :)
Thanks,
Ben
#15
09/24/2005 (6:19 am)
Thank you! I'll work on it this afternoon and hopefully I can get something out to you today that corrects it.
#16
09/24/2005 (7:41 am)
Fixed it. Let me migrate the changes to a patch and I'll send it out shortly.
#17
I've e-mailed the patch to you. The primary changes are to gameConnection.h and gameConnection.cc. The secondary changes were made to the default.bind.cs files and one small change to racingGui.cs. Overall, the changes are pretty minimal, and it corrects the problems mentioned in the thread. The general approach implemented in the patch is:
1. Changed static mFirstPerson to non-static and removed console variable init.
2. Added an mUpdateFirstPerson variable to know when to synchronize changes between client/server.
3. Added setFirstPerson() to GameConnection to toggle the appropriate flags and exposed it as a ConsoleMethod.
4. Changed writePacket/readPacket to transmit state between client/server on change of mUpdateFirstPerson from either side.
5. Changed default.bind.cs to use the new method to toggle state.
Thank you for reviewing!
- Aaron
09/24/2005 (8:52 am)
Ben,I've e-mailed the patch to you. The primary changes are to gameConnection.h and gameConnection.cc. The secondary changes were made to the default.bind.cs files and one small change to racingGui.cs. Overall, the changes are pretty minimal, and it corrects the problems mentioned in the thread. The general approach implemented in the patch is:
1. Changed static mFirstPerson to non-static and removed console variable init.
2. Added an mUpdateFirstPerson variable to know when to synchronize changes between client/server.
3. Added setFirstPerson() to GameConnection to toggle the appropriate flags and exposed it as a ConsoleMethod.
4. Changed writePacket/readPacket to transmit state between client/server on change of mUpdateFirstPerson from either side.
5. Changed default.bind.cs to use the new method to toggle state.
Thank you for reviewing!
- Aaron
#18
09/24/2005 (11:10 am)
Thank you, Aaron, I'll look at this soon!
#19
Please let me know kindly if this matters or not.
Will the changes you are submitting possibly affect third party/resource camera mods? If so, how?
09/24/2005 (3:54 pm)
Aaron,Please let me know kindly if this matters or not.
Will the changes you are submitting possibly affect third party/resource camera mods? If so, how?
#20
If your project wants to do things similar to his project in the way of handling third person camera, then it's a great adjustment to the design and implementation--but that doesn't make it a "bug" that needs to be "fixed" in stock. You are making quite a few changes to low level design and implementation for something that isn't a problem for 95%+ of Torque users...
09/24/2005 (3:57 pm)
That's my biggest concern with this actually. I have absolutely no problem with his analysis, or his specific project's needs, but he defines a very unique circumstance that implements things differently from the original design. Making the changes above I submit (without having done any specific testing) will possibly cause modifications to be required in the hundreds of resources and projects that may utilize $firstperson/mFirstPerson for other things or in other ways, and isn't a required "fix" for stock. If your project wants to do things similar to his project in the way of handling third person camera, then it's a great adjustment to the design and implementation--but that doesn't make it a "bug" that needs to be "fixed" in stock. You are making quite a few changes to low level design and implementation for something that isn't a problem for 95%+ of Torque users...
Associate Kyle Carter