Stumped! Why isn't my application sending packets?

Recommended Posts

Tim Lawton    179

Hey there,

I'm trying to create a server-client architecture application, which sends a struct called PLAYER which holds x,y,z as a float, the instance of PLAYER is Player.

Defined like so

struct PLAYER
{
float x, y, z;
};
PLAYER Player; 

#define SERVER_ADDRESS	"127.0.0.1"
#define SERVER_PORT		17000

SOCKET Socket;
int SizeInt;


These structs are available on both server and client applications, I then have this code to setup a socket on the

SERVER application:

void Application::Init_Winsock()
{
WSAStartup(MAKEWORD(2,2), &Winsock);

if(LOBYTE(Winsock.wVersion) !=2 || HIBYTE(Winsock.wVersion) !=2)	//Check Version
{
WSACleanup();	//Cleanup and end if not version 2
return;
}

//Create Socket
Socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);

//Input Reciever Information
RemoteAddress.sin_family = AF_INET;						//Set the Family Address
RemoteAddress.sin_port = SERVER_PORT;					//Set Port

//Start the recieve thread
}


Then a static function to receive packets from clients

DWORD WINAPI Application::RecvThread(LPVOID knock)
{
Application *instance = static_cast<Application *>(knock);
while(true)
{
PLAYER Recv;

instance->Player = Recv;
}
}


That struct is then used like this to move the object

D3DXMatrixTranslation(&matTrans, Player.x, Player.y, Player.z);


CLIENT Application:

There is a similar init_winsock method:

void Application::Init_Winsock()
{
WSAStartup(MAKEWORD(2,2), &Winsock);

if(LOBYTE(Winsock.wVersion) !=2 || HIBYTE(Winsock.wVersion) !=2)	//Check version
{
WSACleanup();	//Cleanup and return false if version is wrong
return;
}

//Create Socket
Socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);

RemoteAddress.sin_family = AF_INET;								//Set the Address Family
RemoteAddress.sin_port = SERVER_PORT;							//Set Port
}


Then a function to send packets to the Server's Address

void Application::Send_Packet(PLAYER* data)
{
}


The DirectX11 object has initialization  and rendered to the "Player.x/y/z" values in both applications.

Although the object does not appear on the Servers screen when a client connects and starts moving around, modifying the "Player.x/y/z" values with WASD keys, I can't figure out why

Edited by Xooch

Share on other sites
slicer4ever    6760

Hey there,

I'm trying to create a server-client architecture application, which sends a struct called PLAYER which holds x,y,z as a float, the instance of PLAYER is Player.

Defined like so

struct PLAYER
{
float x, y, z;
};
PLAYER Player;


don't know if you not posting anything else is due to what Cornstalks said, but if it isn't.....we're ganna need a bit more....

Share on other sites
Tim Lawton    179

Sorry I hit post by accident, then when I went to do edit my internet cut out. Resulting in this confusing

Share on other sites
slicer4ever    6760

what is your network setup? are you only sending to the loopback address, or are you trying to connect to another computer on your lan, or over the internet? have you checked firewalls?

you also arn't checking if bind has failed, or if you are getting any errors.

Share on other sites
Tim Lawton    179

I'm running both the applications on the local host, hence my SERVER_ADDRESS is 127.0.0.1

Fair point about the error checking, I will add that now and see what results I get.

EDIT: Definitely nothing wrong with the code, I've error checked it all. Either the Player.x/y/z isn't storing the values correct, I believe it is tho as I am able to edit their values with the movement keys so the PLAYER struct should update with it...

I will attempt to run one of my simpler networking application and see if it might be my firewall

UPDATE: My other networking applications seem to work fine, using almost the exact same code.

Edited by Xooch

Share on other sites
rip-off    10976

Can you post your updated code with the error checking? You are using threads, but not synchronisation, which is another problem.

Share on other sites
Tim Lawton    179

This is the updated functions to check for errors:

CLIENT

bool Application::Init_Winsock()
{
bool r;

r = WSAStartup(MAKEWORD(2,2), &Winsock);
if(!r)
{return false;}

if(LOBYTE(Winsock.wVersion) !=2 || HIBYTE(Winsock.wVersion) !=2)	//Check version
{
WSACleanup();	//Cleanup and return false if version is wrong
return false;
}

//Create Socket
Socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);

RemoteAddress.sin_family = AF_INET;								//Set the Address Family
RemoteAddress.sin_port = SERVER_PORT;							//Set Port

return true;
}

bool Application::Send_Packet(PLAYER* data)
{
bool r;

if(!r)
{return false;}

return true;
}


The client is sending the updated version of the Player Struct every frame, using the update function be called by the Win32 class.

bool Application::Update()
{
bool r;

r = m_Input->Update();
if(!r)
{return false;}

//Chcek for Escape
if(m_Input->IsEscapePressed() == true)
{return false;}

//Update System
m_Timer->Update();

//Update Input
r = HandleInput(m_Timer->GetTime());
if(!r)
{return false;}

//Update Networking
Send_Packet(&Player);		//Send packets to server

//Render Graphics
r = RenderGraphics();
if(!r)
{return false;}

return true;
}


SERVER

bool Application::Init_Winsock()
{
bool r;

r = WSAStartup(MAKEWORD(2,2), &Winsock);
if(!r)
{return false;}

if(LOBYTE(Winsock.wVersion) !=2 || HIBYTE(Winsock.wVersion) !=2)	//Check Version
{
WSACleanup();	//Cleanup and end if not version 2
return false;
}

//Create Socket
Socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);

//Input Reciever Information
RemoteAddress.sin_family = AF_INET;						//Set the Family Address
RemoteAddress.sin_port = SERVER_PORT;					//Set Port

if(!r)
{return false;}

//Start the recieve thread
r = CreateThread(NULL, 0, RecvThread, (void*)this, 0, NULL);
if(!r)
{return false;}

return true;
}

DWORD WINAPI Application::RecvThread(LPVOID knock)
{
bool r;

Application *instance = static_cast<Application *>(knock);

while(true)
{
PLAYER Recv;

r = recvfrom(instance->Socket, (char*)&Recv, sizeof(PLAYER), 0, (sockaddr*)&instance->RemoteAddress, &instance->SizeInt);
if(!r)
{return false;}

instance->Player = Recv;
}
}


Share on other sites
hplus0603    11347
sendto() may return a value < 0 if it fails. Testing "not" on the return value is not good enough for error checking.

sin_port argument should be passed through htons(). However, it turns out that, because you don't do this in either sender or receiver, they will both get the same result :-)

recvfrom() doesn't seem to check errors at all. You should.

Share on other sites
Tim Lawton    179

hmm, whats a better way of testing these functions then? Can you give me some sample code, if not too much trouble, please

Share on other sites
Tim Lawton    179

While debugging it this code returned false

	r = WSAStartup(MAKEWORD(2,2), &Winsock);
if(!r)
{return false;}


which would suggest the socket isnt being made and I have no idea why tho

Edited by Xooch

Share on other sites
Tim Lawton    179

Upon adding that error checking method to my other networking application using similar code, it also caused my server to no longer receive updates on the client's position, so I have removed that. Does anyone know why this isn't working, its really starting to bug me :/

Share on other sites
slicer4ever    6760

While debugging it this code returned false

	r = WSAStartup(MAKEWORD(2,2), &Winsock);
if(!r)
{return false;}


which would suggest the socket isnt being made and I have no idea why tho

[url="http://msdn.microsoft.com/en-us/library/windows/desktop/ms742213%28v=vs.85%29.aspx"]http://msdn.microsoft.com/en-us/library/windows/desktop/ms742213%28v=vs.85%29.aspx[/url]

why exactly did you think zero for that function means failure?

I'm looking over your code, and nothing terrible jumps out to me.  have you tried changing to tcp to make sure everything's setup correctly, then switch back to udp?

Share on other sites
rip-off    10976

Upon adding that error checking method to my other networking application using similar code, it also caused my server to no longer receive updates on the client's position, so I have removed that. Does anyone know why this isn't working, its really starting to bug me :/

Step 1: error checking. Step 2: error handling/logging. For example, the [url="http://msdn.microsoft.com/en-us/library/windows/desktop/ms742213(v=vs.85).aspx"]documentation for WSAStartup[/url] says:

[b]Return value[/b]
If successful, the WSAStartup function returns zero. Otherwise, it returns one of the error codes listed below.
The WSAStartup function directly returns the extended error code in the return value for this function. A call to theWSAGetLastError function is not needed and should not be used.

<list of error codes omitted>

So WSAStartup is not returning false, it is returning its "success" value, which you are misinterpreting as failure. It is important to thoroughly read the documentation for the functions you are using about what kind of error codes can be returned.

You should at the very least write a log message somewhere when a fatal error occurs, and preferably provide a notification to the user rather than crashing to desktop. Most APIs have a way to get a human (or at least, geek) readable error description, which will help enormously in diagnosing such failures when they happen to a random user on the internet, who might want to play your game but might not be particularly technical.

A few things still need some work:

[list]

[*] You aren't checking for errors creating the socket in Application::Init_Winsock(). In addition, you only perform partial cleanup were something to go wrong in the middle of this function.

[*] Your recvfrom() call should check that the number of bytes received was (at least) the number that is expected. It might not be particularly likely, but you could collide with an existing port number, and another application may attempt to send a message to the port you are listening on (for UDP in particular, you can easily pick up broadcasted discovery messages on a local network).

[*] You still have not addressed the thread synchronisation problems in your code. A simple fix would be to ensure that all accesses to Application::Player are controlled by a [url="http://msdn.microsoft.com/en-us/library/windows/desktop/ms682530(v=vs.85).aspx"]Critical section[/url]. There are more sophisticated ways of handling this, particularly if your "Player" variable is currently being accessed all over the place.

[*] Your background thread's error handling isn't very useful. If a receive fails, you silently (i.e. without even printing a log message) return "false". Where does this return value go? Well, the operating system will store the return value, but your code does not appear to have anything listening for the thread unexpectedly dying.

[/list]

A relatively simple alternative to multi-threading is to use "non-blocking" sockets. This means you can try to receive in your main loop without stalling the game logic until new data arrives. I would recommend this to someone who is still getting to grips with socket programming. Trying to learn and understand multi-threaded programming is significantly harder, and trying to do both at the same time is a recipe for confusion and error.

When you have your code working, you might also consider writing some code to detect and handle the case where the client or server dies or is shut down. Something as simple as checking if no messages have been received for a reasonable period of time, and then assuming the remote peer is "gone" and informing the user.

Share on other sites
Tim Lawton    179

Can't you offer me some sample code, I don't understand fully what you mean

Share on other sites
ApochPiQ    23003
We're not here to write your code for you ;-)

The advice offered in this thread should be ample to solve your problem. If you have specific areas you don't understand, please point them out directly. Just saying you don't understand is not useful, since we have no idea what you actually want explained in more detail.

Share on other sites
Tim Lawton    179

I'm unsure how to create these advance ways of error checking, I've put watches on the variables and they all seem to be taking in the expected values, still, there is no networking. I've used very very similar code before on another project, which was transferring only x,y values.

So all I've done is added a z value to the struct, and its not working

Share on other sites
ApochPiQ    23003
It's a very simple process, albeit work-intensive and tedious at times:
• Find an API that you call that is not currently error-checked
• Look up the documentation for that API to determine what it does (if anything!) to indicate failure
• Write some simple checks (usually just if statements) to see if that failure happened at runtime
• Decide on how to handle failures: throw an exception, display a runtime error message, etc.
• Implement your chosen mechanism for failure-response
• Repeat from the top
It's also worth noting that "it's not working" is not helpful to anyone. Describe what you expect to happen, what actually happens, and try and pinpoint where the two are not identical. Edited by ApochPiQ

Share on other sites
rip-off    10976

Can't you offer me some sample code, I don't understand fully what you mean

Which part didn't you understand?

Share on other sites
Tim Lawton    179

Can't you offer me some sample code, I don't understand fully what you mean

Which part didn't you understand?

How to code these error checks you speak of as mine isn't getting the job done

Share on other sites
rip-off    10976

Well, I cannot see your current code.

At the very least, add logging when an error occurs using an external API. You should not need to use a debugger to diagnose such errors. When a connection fails on another user's machine, they may not have a debugger or the skills required to effectively use one. Something not unlike this:

int result = SomeLib_DoStuff(/* ... */);
if(result != SOME_LIB_STUFF_DONE) {
std::stringstream error;
error << "DoStuff returned failure code: " << result << ", error message: " << SomeLib_GetError();
log(error.str());
return false;
}


Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account