Announcement: Be excellent to each other.


Caravel Forum : Caravel Boards : Development : Portability
New Topic New Poll Post Reply
Poster Message
ross
Level: Delver
Rank Points: 30
Registered: 05-17-2003
IP: Logged
icon Portability (0)  
I was wondering if after the next release of Drod that Erik (and whoever else if responsible for deciding) would consider changes to the way that the code handles Unicode and some other platform dependant code (pretty much anything that starts with _ is non-standard) ?

Specifically I was hoping that we could remove nearly every single #ifdef (include guards aside) and move the functionality into platform specific files. Schik has already started this to a point but there really should be the equivalent of a Ports.h (maybe called Platform.h) for each OS. The top of each file would then look like

#include <platform.h> // included from relevant dir

rather than

#if defined _WIN32
#include <windows.h>
#elif defined __APPLE__ || defined __sgi
#include <ctypes.h>
#include <Ports.h>
#endif

As you might have noticed this is a fair amount of work and would involv re-structuring the layout of files to some extent but it would mean that to port Drod to another platform there is a well-defined set of functions that you must implement in order to get it to work, without just adding more #ifdef MyOS and making the code eventually unreadable.

I know asking people to make these changes when it doesn't necessarily benefit them is quite drastic but think of all the people on other OS' that are missing out :)

05-31-2003 at 01:39 PM
View Profile Send Private Message to User Show all user's posts Quote Reply
ErikH2000
Level: Legendary Smitemaster
Avatar
Rank Points: 2794
Registered: 02-04-2003
IP: Logged
icon Re: Portability (0)  
ross wrote:
...
Specifically I was hoping that we could remove nearly every single #ifdef (include guards aside) and move the functionality into platform specific files.
Well, I guess a few reservations are coming up in my head, but I want to make sure I understand you first. So what kinds of things does platform.h have in it?

... As you might have noticed this is a fair amount of work and would involv re-structuring the layout of files to some extent ...

Okay, we will talk about it, but put on a thick skin. We might leave some of your ideas behind because of the time cost that large changes can incur.

In any case, thanks for thinking about this, Ross.

-Erik

____________________________
The Godkiller - Chapter 1 available now on Steam. It's a DROD-like puzzle adventure game.
dev journals | twitch stream | youtube archive (NSFW)
05-31-2003 at 10:53 PM
View Profile Send Email to User Show all user's posts This architect's holds Quote Reply
ross
Level: Delver
Rank Points: 30
Registered: 05-17-2003
IP: Logged
icon Re: Re: Portability (0)  
Well primarily platform.h would have in some things that already exist in the code.

For instance macosx/platform.h would likely have
implementations of wcslen(), stricmp() and so forth that are not available on the OS X platform. Obviously the windows/platform.h would not need to have these and would probably just include <windows.h>. Linux also has the wcs* funcs (I think) and would therefore just #include <wchar.h>.

Other functions which are endian centric could also be moved into the platform.h isolating them from the code.

I know it seems like an unnecessary amount of work but I really do dislike conditional compilation :)

I've got a pretty thick skin, so no need to worry. Actually I just feel that I'm intruding a bit, having only been around a week or two and all the hard work already having been done, and suggesting changes like this. No need to worry if you disagree you can just tell me when an idea stinks :)

06-01-2003 at 12:14 AM
View Profile Send Private Message to User Show all user's posts Quote Reply
ErikH2000
Level: Legendary Smitemaster
Avatar
Rank Points: 2794
Registered: 02-04-2003
IP: Logged
icon Re: Re: Re: Portability (0)  
ross wrote:
Well primarily platform.h would have in some things that already exist in the code.

For instance macosx/platform.h would likely have
implementations of wcslen(), stricmp() and so forth that are not available on the OS X platform. Obviously the windows/platform.h would not need to have these and would probably just include <windows.h>. Linux also has the wcs* funcs (I think) and would therefore just #include <wchar.h>.

Other functions which are endian centric could also be moved into the platform.h isolating them from the code.

I know it seems like an unnecessary amount of work but I really do dislike conditional compilation :)
I would also like to reduce the amount of conditional compilation, but it doesn't seem important to move all of it into one file (i.e. platform.h). In some cases, I think this could make the code organization more difficult to understand.

For example, we have endian conversions being performed in CDbPackedVars::UnpackBuffer(). I wouldn't want to move the unpack buffer function into platform.h/cpp or have it be a separate included file that is included by platform.h. For the simplest organization, the buffer-unpacking code belongs in that class and in that file.

(My apologies, if I've misunderstood you and you weren't proposing to isolate the platform-specific code this way.)

To make the above function easier to maintain, I think we need a few macros that give endian-influenced values consistent byte orders for storage. And we replace, the #ifdef _sgi blocks with the macros. Elsewhere, like in platform.h/ports.h, the macro is defined to do different things on different platforms. So code like this:

#ifdef __sgi
	LittleToBig(&wVarNameSize);
#endif



...could instead be written like:
  wVarNameSize = S2L16(wVarNameSize);



"N2L16" = Convert a 16-bit integer from storage byte order to local byte order, which might be the same, in which case a variable would just be assigned to itself, and probably compile out.

It occurs to me that you might have been proposing about the same thing as me. My basic point is that it is not my goal to move all conditional compilation into platform.h, when code should instead be grouped by its functionality.

Side note: There is very little that we use from windows.h and its vast number of declarations tends to cause problems. So if I have to use it, I include it from a .CPP and whatever functionality it uses will be exposed by functions in that module. Example is SysTimer.h/cpp in the "BackEndLib" project.

-Erik

____________________________
The Godkiller - Chapter 1 available now on Steam. It's a DROD-like puzzle adventure game.
dev journals | twitch stream | youtube archive (NSFW)
06-01-2003 at 08:29 PM
View Profile Send Email to User Show all user's posts This architect's holds Quote Reply
ross
Level: Delver
Rank Points: 30
Registered: 05-17-2003
IP: Logged
icon Re: Re: Re: Re: Portability (0)  
ErikH2000 wrote:
I would also like to reduce the amount of conditional compilation, but it doesn't seem important to move all of it into one file (i.e. platform.h). In some cases, I think this could make the code organization more difficult to understand.

I agree to some extent that it would make the build a little more complicated, but if anything I think the code would be *easier* to understand. You know exactly where to look to find the function you require, if it's not in platform.h then it's available on your platform.


ErikH2000 wrote:
For example, we have endian conversions being performed in CDbPackedVars::UnpackBuffer(). I wouldn't want to move the unpack buffer function into platform.h/cpp or have it be a separate included file that is included by platform.h. For the simplest organization, the buffer-unpacking code belongs in that class and in that file.

I can see your point, but the macro mentioned below would likely be used here and defined (or not - depending on computer) in the platform.h. I'm sort of suggesting everything EXCEPT the endian dependant code really. I'm not making this very clear ...

ErikH2000 wrote:
To make the above function easier to maintain, I think we need a few macros that give endian-influenced values consistent byte orders for storage. And we replace, the #ifdef _sgi blocks with the macros. Elsewhere, like in platform.h/ports.h, the macro is defined to do different things on different platforms. So code like this:

I like this idea, although normally I'm not a big fan of macros.

ErikH2000 wrote:
It occurs to me that you might have been proposing about the same thing as me. My basic point is that it is not my goal to move all conditional compilation into platform.h, when code should instead be grouped by its functionality.

Oh I agree wholeheartedly. Most of my opinions on this are coloured a little by
www.chris-lott.org/resources/cstyle/ifdefs.pdf even though it is about C.
ErikH2000 wrote:
Side note: There is very little that we use from windows.h and its vast number of declarations tends to cause problems.

There isn't a lot of windows specific stuff in there, but everything with _ at the beginning is pretty much windows specific (even when other platforms have the same func without the _), not a big job to replace them I guess (and I have). Mostly.

Well there's plenty of time to discuss it, I don't want to distract you too much during this release so we can always dicuss it later on.
06-02-2003 at 09:34 AM
View Profile Send Private Message to User Show all user's posts Quote Reply
ErikH2000
Level: Legendary Smitemaster
Avatar
Rank Points: 2794
Registered: 02-04-2003
IP: Logged
icon Re: Portability (0)  
ross wrote:
ErikH2000 wrote:
I would also like to reduce the amount of conditional compilation, but it doesn't seem important to move all of it into one file (i.e. platform.h). In some cases, I think this could make the code organization more difficult to understand.
I agree to some extent that it would make the build a little more complicated, but if anything I think the code would be *easier* to understand. You know exactly where to look to find the function you require, if it's not in platform.h then it's available on your platform.
It's like you're saying "put the peanut butter with the butter". And I'm saying "put the peanut butter with the peanuts". Classification like this can be arbitrary, and one person may favor one way for subjective reasons.

I think I'm willing to put some things into a platform module, and others should just stay put. A general rule would be that application-specific functionality (i.e. CDbBase methods) should stay in their original modules.
ErikH2000 wrote:
For example, we have endian conversions being performed in CDbPackedVars::UnpackBuffer(). I wouldn't want to move the unpack buffer function ...

I can see your point, but the macro mentioned below would likely be used here and defined (or not - depending on computer) in the platform.h. I'm sort of suggesting everything EXCEPT the endian dependant code really. I'm not making this very clear ...
Yeah, I agree. Use a macro for the endian code, and we don't have to consider moving several functions.

I think that we aren't disagreeing over much here, and we should just get to specifics. A suggested way to go about this:

1. at some point, you make a summary list of proposed changes and show it to me. i.e. something like:

- use macros for endian conversions
- remove project-wide wchar.h includes,
and have wchar.h optionally be included
from platform.h
- prefix every variable name with "ross_".
- ...whatever else...

2. We'll discuss and I might line item veto a few things. I might be a bit of a bully about it, just because there isn't time to come to agreement on every issue no matter how reasonable an opposing point of view may be.
...
Well there's plenty of time to discuss it, I don't want to distract you too much during this release so we can always dicuss it later on.
Thanks, man. Your comments deserve more of a reply, but for now I probably won't say much more.

-Erik

____________________________
The Godkiller - Chapter 1 available now on Steam. It's a DROD-like puzzle adventure game.
dev journals | twitch stream | youtube archive (NSFW)
06-03-2003 at 05:33 PM
View Profile Send Email to User Show all user's posts This architect's holds Quote Reply
New Topic New Poll Post Reply
Caravel Forum : Caravel Boards : Development : Portability
Surf To:


Forum Rules:
Can I post a new topic? No
Can I reply? No
Can I read? Yes
HTML Enabled? No
UBBC Enabled? Yes
Words Filter Enable? No

Contact Us | CaravelGames.com

Powered by: tForum tForumHacks Edition b0.98.8
Originally created by Toan Huynh (Copyright © 2000)
Enhanced by the tForumHacks team and the Caravel team.