Sign in to follow this  
Tim Lawton

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; 

Additional declarations in the header file

#define SERVER_ADDRESS	"127.0.0.1"
#define SERVER_PORT		17000

WSADATA Winsock;
SOCKET Socket;
sockaddr_in RemoteAddress;
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
	ZeroMemory(&RemoteAddress, sizeof(RemoteAddress));		//Clear Struct
	RemoteAddress.sin_family = AF_INET;						//Set the Family Address
	RemoteAddress.sin_port = SERVER_PORT;					//Set Port
	bind(Socket, (sockaddr*)&RemoteAddress, sizeof(RemoteAddress));

	//Start the recieve thread
	CreateThread(NULL, 0, RecvThread, (void*)this, 0, NULL);
}

 

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;
		recvfrom(instance->Socket, (char*)&Recv, sizeof(PLAYER), 0, (sockaddr*)&instance->RemoteAddress, &instance->SizeInt);

		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);

	//Input Receiver Information
	ZeroMemory(&RemoteAddress, sizeof(RemoteAddress));				//Clear Struct
	RemoteAddress.sin_family = AF_INET;								//Set the Address Family
	RemoteAddress.sin_addr.s_addr = inet_addr(SERVER_ADDRESS);		//Set IP Address
	RemoteAddress.sin_port = SERVER_PORT;							//Set Port
}

 

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

void Application::Send_Packet(PLAYER* data)
{
	sendto(Socket, (char*)data, sizeof(PLAYER), 0, (sockaddr*)&RemoteAddress, sizeof(sockaddr));
}

 

 

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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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);

	//Input Receiver Information
	ZeroMemory(&RemoteAddress, sizeof(RemoteAddress));				//Clear Struct
	RemoteAddress.sin_family = AF_INET;								//Set the Address Family
	RemoteAddress.sin_addr.s_addr = inet_addr(SERVER_ADDRESS);		//Set IP Address
	RemoteAddress.sin_port = SERVER_PORT;							//Set Port
	
	return true;
}

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

	r = sendto(Socket, (char*)data, sizeof(PLAYER), 0, (sockaddr*)&RemoteAddress, sizeof(sockaddr));
	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;
	
	//Read user input
	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
	ZeroMemory(&RemoteAddress, sizeof(RemoteAddress));		//Clear Struct
	RemoteAddress.sin_family = AF_INET;						//Set the Family Address
	RemoteAddress.sin_port = SERVER_PORT;					//Set Port

	r = bind(Socket, (sockaddr*)&RemoteAddress, sizeof(RemoteAddress));
	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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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;
}

Share this post


Link to post
Share on other sites

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

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this