OS X Port Questions

[Corun]

Hi, I'm the person who is porting elc to Mac OS X. It's not far from complete but I do have a few questions:

 

At the moment there seems to be a problem that occurs sometimes when a bag tries to be removed.

 

In void remove_bag(int which_bag) in items.c the line:

sector=SECTOR_GET(objects_list[bag_list[which_bag].obj_3d_id]->x_pos, objects_list[bag_list[which_bag].obj_3d_id]->y_pos);

 

Is crashing with segfaults a lot. I don't think it's an endian problem. Have any of you been experiencing this problem?

 

Here's a teaser screenshot (with a few bits not working that you can probably see):

http://www.evula.org/corun/elscreen.jpeg

 

I also have a request. Please write your code and think about endians!

Here's how it goes:

 

If you are a reading something from a file or from the server then you must swap it.

 

So, imagine 'data[10]' is a buffer which contains an int a short and a float(in that order) from the server

 

You are doing this:

int myInt = *((int*)data);
short myShort = *((short*)(data + sizeof(int));
float myFloat = *((float*)(data + sizeof(int) + sizeof(short));

 

What you should do is this:

int myInt = SDL_SwapLE32(*((int*)data));
short myShort = SDL_SwapLE16(*((*((short*)(data + sizeof(int)));
float myFloat = SwapLEFloat(*((float*)(data + sizeof(int) + sizeof(short)));

 

(Note, SwapLEFloat is not part of SDL but it will be in misc.c when the OS X port is commited).

 

The same thing goes if you pointer cast in to struct. You should do this:

typedef struct
{
     int myInt;
     short myShort;
     float myFloat;
} FileHeader;

FileHeader he;
he = *((FileHeader*)data);

he.myInt = SDL_SwapLE32(he.myInt);
he.myShort = SDL_SwapLE16(he.myShort)
he.myFloat = SwapLEFloat(he.myFloat);

 

The good thing about this is that SDL #defines SDL_SwapLE32(X) (X) and friends on little endian machines so it won't slow anything down for you PC people but what this will do is it will make us big endian people happy :-).

[Leeloo]

At the moment there seems to be a problem that occurs sometimes when a bag tries to be removed.

 

In void remove_bag(int which_bag) in items.c the line:

sector=SECTOR_GET(objects_list[bag_list[which_bag].obj_3d_id]->x_pos, objects_list[bag_list[which_bag].obj_3d_id]->y_pos);

 

Is crashing with segfaults a lot. I don't think it's an endian problem. Have any of you been experiencing this problem?

I would suggest writing which_bag and bag_list[which_bag].obj_3d_id to stdout in the start of the function (make sure to fflush(stdout) after the each one, so that if it is written out before segfaulting.

 

If one of those are in byte-swapped that's a certain segfault.

[Wytter]

Yes, do what Leeloo suggests. The which_bag is just an Uint8 and should never exceed 200, but perhaps you missed something in the process so it's actually getting values > 200 - would be strange though. It would have to be some kind of memory corruption error if the obj_3d_id doesn't exist as it's a variable that's set in the client directly.

 

If you have access to gdb or perhaps even a gdb frontend, please use it

[frak]

void SDLNet_Write16(Uint16 value, void *area)

value
   The 16bit number to put into the area buffer 
area
   The pointer into a data buffer, at which to put the number 

Put the 16bit (a short on 32bit systems) value into the data buffer area in network byte order. This helps avoid byte order differences between two systems that are talking over the network.

 

u should have used those SDL_net functions in the first place =P

[Entropy]

Yes, we should have used them, but, well, none of us have an Apple computer so we couldn't test it anyway.

But from now on we should be endian 'friendly'.

[Corun]

void SDLNet_Write16(Uint16 value, void *area)

value
   The 16bit number to put into the area buffer 
area
   The pointer into a data buffer, at which to put the number 

Put the 16bit (a short on 32bit systems) value into the data buffer area in network byte order. This helps avoid byte order differences between two systems that are talking over the network.

 

u should have used those SDL_net functions in the first place =P

I wouldn't recommend using them because they will make networking a lot worse in some cases; If SDL_net is compiled with TCP_NODELAY #defined then it will send one tcp packet for each SDLNet_Write*() function and there would be a lot of unneccessary bytes used in the TCP headers.

 

What we really want is to do it like it is done currently and make sure that TCP_NODELAY is #defined when SDL_net is compiled.

 

(Note: Normally, when stuff is sent with TCP there is an algorithm which collects data together to try and send a large clump at once. This can cause there to be a short wait between write()s and the actual sending of the data. TCP_NODELAY turns of the algorithm and makes write() send stuff instantly which is what we want.)

[Malaclypse]

What we really want is to do it like it is done currently and make sure that TCP_NODELAY is #defined when SDL_net is compiled.

Doesn't this lead into problems eventually? Because we can't suspect players to compile SDL_net for themselves, ensuring TCP_NODELAY is defined when compiling, wouldn't we have the need to include (a self-compiled) SDL_net with the distribution of the client?

[Corun]

What we really want is to do it like it is done currently and make sure that TCP_NODELAY is #defined when SDL_net is compiled.

Doesn't this lead into problems eventually? Because we can't suspect players to compile SDL_net for themselves, ensuring TCP_NODELAY is defined when compiling, wouldn't we have the need to include (a self-compiled) SDL_net with the distribution of the client?

I don't see how it would lead to problems. What I am suggesting is distributing the Windows client with SDL_net compiled with TCP_NODELAY defined. This would possibly decrease lag slightly for the windows users. For people compiling it themselves from cvs we could put something in the readme recommending that they compile their SDL_net with TCP_NODELAY defined. It won't make things worse if they don't (because that is how it is done currently) But it would make it better for those that did it.

[Entropy]

I think the default SDL_net is compiled with TCP_NODELAY (I think that because the packets are merged togather, many times).

[Malaclypse]

I don't see how it would lead to problems. What I am suggesting is distributing the Windows client with SDL_net compiled with TCP_NODELAY defined.

That's what I meant with problems. So we would have to compile SDL_net for windows users, which isn't part of EL. This is an extra compilation, where we should instead use the default installation on the users computer. And as of today, we imho could suspect SDL to be installed even on windows computers, as there are more games then just EL that use and/or support it. Ok, those are not really problems, but imho this is no clean solution, even more in the attempt to unify the distribution process.

[Entropy]

EL comes with the SDL DLLs

[frak]

If SDL_net is compiled with TCP_NODELAY #defined then it will send one tcp packet for each SDLNet_Write*() function and there would be a lot of unneccessary bytes used in the TCP headers.

here is how u use SDLNet_Write16

// put my number into a data buffer to prepare for sending to a remote host
char data[1024];
Sint16 number=12345;
SDLNet_Write16((Uint16)number,data);

an integer is written into a buffer. nothing is send here.

You probably muddled up SDLNet_Write* and SDLNet_TCP_Send.

Same with SDLNet_Read*(). Those functions do not receive anything. You have to receive data with SDLNet_Recv(TCPsocket sock, void *data, int maxlen). U can then use the read functions on the buffer pointed to by void * data.

[Corun]

It does? Cool :-). I just guessed at what Write16 and friends did. I should really have looked at the Docs :-).

In that case we should most definitily be using them... But.. Oh well :-)

 

We should still make sure that the SDL DLLS that come with the windows ELC had TCP_NODELAY defined when they were compiled, though.

 

An update on the Mac OS X Port:

 

- All endian stuff seems is done I think (Some random guy gave me a sword and helmet for no reason in a trade )

- Found the bag bug. Bags weren't being added properly.

- Pasting doesn't work.

- Delete key doesn't work in text fields.

- Loading is a little slow.

- I need to make sure I've put #ifdef OSX and #ifdef EL_BIG_ENDIAN around all my changes.

[crusadingknight]

You rock Keep up the good work.

Edited March 21, 2005 by crusadingknight

[Wytter]

You should go through some of the new code for books as well. The following places will need your attention:

 

multiplayer.c:924
                                open_book(*((Uint16*)(in_data+3)));

multiplayer.c:930
                               read_network_book(in_data+3, data_lenght-3);

multiplayer.c:936
                               close_book(*((Uint16*)(in_data+3)));

books.c:410
               *((Uint16*)(str+1))=id;

books.c:427-428
       b=get_book(*((Uint16*)(data+1)));
       if(!b) b=read_book(file_name,data[0],*((Uint16*)(data+1)));

books.c:447
       int l=*((Uint16*)(data));

books.c:456
       l=*((Uint16*)(data));

books.c:463-466
       x=*((Uint16*)(data));
       y=*((Uint16*)(data+2));
       w=*((Uint16*)(data+4));
       h=*((Uint16*)(data+6));

books.c:484
       int l=*((Uint16*)(data+4));

books.c:489-490
       b=get_book(*((Uint16*)(data+1)));
       if(!b) b=create_book(buffer,*data,*((Uint16*)(data+1)));

books.c:500
               l=*((Uint16*)(data+1));

books.c:725-726
                                       *((Uint16*)(str+1))=b->id;
                                       *((Uint16*)(str+3))=b->have_server_pages;

books.c:750
                               *((Uint16*)(str+1))=b->id;

books.c:781
                               *((Uint16*)(str+1))=b->id;

I wasn't sure on how you were doing this, so I'd leave it up to you to endianize it.

[Entropy]

Corun, do you have an account on berlios.de?

If not, please make one, I want to give you CVS R/W access.

[Corun]

Created. UN: Corun

 

I will CVS my changes later this week.

 

Thanks for those references Wytter, those shouldn't take too long to endianize.

[Wytter]

I just added you as a developer

[Wytter]

Hey Corun,

 

Could you give a status update of the port?

[Corun]

Sorry, I've been a little busy with other things

 

I will commit something today and give a binary to someone binary somewhere(who, how?)

[Wytter]

That sounds great :-)

 

Hmm, how large is the binary ? If it's < 3MB, just send it to my mail (the one on berlios).

 

EDIT: Btw, just in case - could you make it a patch first, and submit it to the patch section? That way we can make sure that the CVS is working on all platforms afterwards as well...

Edited April 3, 2005 by Wytter

[Corun]

That sounds great :-)

 

Hmm, how large is the binary ? If it's < 3MB, just send it to my mail (the one on berlios).

 

EDIT: Btw, just in case - could you make it a patch first, and submit it to the patch section? That way we can make sure that the CVS is working on all platforms afterwards as well...

I dunno how to make a patch :-/. I'll just email you a zip of the source and binary at some point or I'll email a link if it's too big.

[crusadingknight]

uh,

cvs diff [filename] ... > [patchname].patch

(As I assume you know, replace ... with more filenames, if you have them.)

 

At least, I assume Mac OS X has the command-line cvs, since it is Unix-based...

Edited April 4, 2005 by crusadingknight

[Corun]

Patch posted. Email with link to fully made version ready for download sent to wytter.

[Grum]

Couple of questions:

*) What is the reason for renaming Move() to Move1(), and GetColor() to GetColor1()?

*) why is global.h included in translate.h? gcc doesn't complain on my machine

*) would it be useful to make a separate makefile for OSX?

 

I committed your patch with a few small changes, and left Move() and translate.h untouched. Let me know if that needs change. Could you check out a clean copy of CVS and tell me if everything works?