trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

mrimer wrote:
Just a question. I've noticed you've rearranged some of the vars in constructor initialization lists. I was wondering what the warning was that requires doing that to remove it?

That's to fix a warning about the compiler reordering the initialization lists to match the declaration order in the header file. The reason for the reordering is probably for performance (even though i test compile without optimization..), and my guess as to why it warns about this is in case the initializers have side effects that the other initializers depend on. Which they don't have in DROD, thankfully.


[Edited by trick on 06-20-2003 at 08:15 PM GMT: added that i have optimization off when test compiling]

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Hooray, all the latest Linux port fixes are committed! Thanks, Gerry.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

File: DROD-Linux-Patches-2003-06-28.zip (19.7 KB)
Downloaded 125 times.
License: Other
From: Unspecified

It's that time again. Patch-time! Wohoo! I attached a zip with patches for BackEndLib, DRODLib, FrontEndLib (small fixes for those), and DROD itself (but see below before you get your hopes too high). Mostly the usual stuff to fix, but i fixed a potential bug or two as well (always-true's, unsigned abs, etc). Also commented out some unused variables, and removed -D_DEBUG from the Makefiles (since it's defined in BackEndLib/Assert.h now)

I also added one-instance checking for linux to Main.cpp. I decided to go with the process list option for now, as that doesn't require any writable files, cleanup and such. The only problem that may happen is if the user runs another program called 'drod' simultaneously with DROD, but that's unlikely. Anyway, if anyone runs into any trouble with this it's no problem changing to the lock-file option later.

So, does DROD compile and run on linux now ? Sadly, no. There's two files in DROD that require fixing: CreditsScreen.cpp and WinScreen.cpp. Those have wide char strings using the L"string" syntax, which results in wchar_t strings.. I could fix those myself, but i prefer not to, as i haven't completed the game yet (shock! horror!) and don't want to ruin the experience. I've been playing at my sister's computer when i'm there, tho, and i'm going there after i've posted this, so i should be done soon-ish (eh)

One question about the makefiles: I'd like to have them include a common config file and a master Makefile, so that i don't need to edit all of them to change the compile options (switching from debug to release, for example). Is there a place in the CVS we could put that config and master Makefile ? Ideally it would be in the top dir, but i understand if you don't want to clutter that up.

- Gerry

[Edited by trick on 06-28-2003 at 05:49 PM GMT: Confirmed that the zip-file works. Yay!]

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Yeah, there's a Master directory where stuff like that goes.

(Good work! I'll d/l the patch and start working on it. And I'll take care of the end-of-game text.)

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

I'm about halfway through the patch-work. Gerry, do you have a makefile for the DROD project?

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Okay, I'm done adding your changes (whew!). Thanks again. Found even a few more bugs as I was entering them in. I've just committing everything.

[Edited by mrimer on 06-28-2003 at 07:44 PM]

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Gerry, I finished putting all the endgame text into the DB, so you should be able to finish porting DROD now. Hooray! I can't wait!

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

ross
Level: Delver
Rank Points: 30
Registered: 05-17-2003
IP: Logged

Does the end game text use L"" to define wchar strings? I've just come across the same problem and was wondering if my copy from CVS was massively out of date.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

If it's not there today (using anonymous login), check again tomorrow. I think they should all be gone.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

ross
Level: Delver
Rank Points: 30
Registered: 05-17-2003
IP: Logged

mrimer wrote:
If it's not there today (using anonymous login), check again tomorrow. I think they should all be gone.

I've got developer read access now so it should be okay. Cheers.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

I got the update now (sorry for the delay..), and tried to compile. DROD/CreditsScreen.cpp uses wcstok, so i added a DROD version of that to BackEndLib/Wchar.cpp, and updated CreditsScreen.cpp to use the new function (no spoilers in sight, thankfully ). I think that should be ok, but have a look to be sure

After doing that (plus some of the usual minor fixes), everything compiles with only a few warnings*, but it doesn't link, because of missing std::char_traits<unsigned short> methods in standard c++ . What we're missing:

::assign(unsigned short&, unsigned short const&)
::length(unsigned short const*)
::copy(unsigned short*, unsigned short const*, unsigned)
::move(unsigned short*, unsigned short const*, unsigned)
::compare(unsigned short const*, unsigned short const*, unsigned)

Shouldn't be much of a problem overloading those, except they're in std, and i seem to remember that overloading anything in there is undefined. I have to look into that. Any have any ideas ?

* The few warnings i mentioned is mostly unsigned comparision >= 0 inside boundary checking macros that may be used with both signed and unsigned arguments (afaik), harmless. There's also these warnings:

DbDemos.cpp:490: warning: choosing `CAttachableWrapper<T>::operator T&() [with T = UINT]' over `CAttachableWrapper<T>::operator T() const [with T = UINT]'
DbDemos.cpp:490: warning: for conversion from `CAttachableWrapper<UINT>' to ` unsigned int'
DbDemos.cpp:490: warning: because conversion sequence for the argument is better

(with variations) in DRODLib/DbDemos.cpp line 490, 1378, and 1390, and DROD/GameScreen.cpp line 1284, 1337 and 1344. Those are harmless also, but can be fixed by fx. adding an operator* to CAttachableWrapper and using that to dereference the object where those warnings appear (so that fx. DbDemos line 1378 would be return **pBool;. Don't think it should break anything, but i don't really understand why it should be necessary - the operators already there should be enough.. What do you say ?

- Gerry

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

File: DROD-Linux-Patches-2003-07-03.zip (4.4 KB)
Downloaded 121 times.
License: Other
From: Unspecified

And here's the patches

The Makefile.linux belongs in the DROD subdir.

- Gerry

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

I've put in your changes and they're all committed. Thanks!

[Edited by mrimer on 07-03-2003 at 10:48 PM]

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Gerry, I can't recall whether I've asked this before: do you have an up-to-date version of your linux c++ compiler (are you using gcc?) and are there any newer ones that might support what you're missing?

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

Yep, i'm using the latest stable version of gcc/g++ available (don't want to get into unstable versions for obvious reasons).

I looked into this problem more, and found that the problem lies in that the official C++ standard requires that char_traits are specialized for types char and wchar_t, but nothing else. This works in windows since wchar_t is the same as unsigned short there, but in linux wchar_t is something else, so it breaks. The standard doesn't require specialization for other built-in types like unsigned short, and so it isn't included. Also, just to make things difficult, specializing a template that uses only built-in types is illegal. It may work right now (tho i haven't tested it), but then it might break at any future or past verison of libstdc++, and we don't want that.

The solution is making WCHAR a class with one unsigned short member (and nothing else), and creating a new char_traits specialization for that class. Don't worry about this, i'll see what i can do.

(By the way, there's some other threads around the forum that i'd like to reply to, but right now i really need to sleep. Just wanted to give you an update to this first. I'll get back to you again )

- Gerry

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

trick wrote:
...but it doesn't link, because of missing std::char_traits<unsigned short> methods in standard c++ . What we're missing:

::assign(unsigned short&, unsigned short const&)
::length(unsigned short const*)
::copy(unsigned short*, unsigned short const*, unsigned)
::move(unsigned short*, unsigned short const*, unsigned)
::compare(unsigned short const*, unsigned short const*, unsigned)

You know, I'm thinking these functions are really simple to write. We should probably just write our own and put them in WChar.h/.cpp. In fact, we don't have to write anything. We've already got all those functions, don't we? Just change all the calls using the ones above to our own. Do you think it would work?

[Edited by mrimer on 07-05-2003 at 06:57 AM]

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

mrimer wrote:
You know, I'm thinking these functions are really simple to write. We should probably just write our own and put them in WChar.h/.cpp. In fact, we don't have to write anything. We've already got all those functions, don't we? Just change all the calls using the ones above to our own. Do you think it would work?

I thought about that, but most of those functions aren't called directly, they're called from other templates:

CreditsScreen.o(.gnu.linkonce.t._ZNSbItSt11char_traitsItESaItEEC1EPKtRKS1_+0x13): In function `std::basic_string<unsigned short, std::char_traits<unsigned short>, std::allocator<unsigned short> >::basic_string[in-charge](unsigned short const*, std::allocator<unsigned short> const&)':
/usr/lib/gcc-lib/i686-pc-linux-gnu/3.2.2/include/g++-v3/bits/stl_alloc.h:238: undefined reference to `std::char_traits<unsigned short>::length(unsigned short const*)'

(and that's one of the shorter errors). So, if we go that way, we need to hunt down all the topmost unimplemented functions, including all implicit conversions, create own versions of those and replace all the calls, and hope there's not any other templates that depend on the unimplemented stuff.

I found that some of the errors pop up when using c_str, so i tried replacing all the WSTRING related c_str calls with a dummy conversion function (in a copy of the source tree i made for that purpose), and the result was.. no change. I'm pretty sure i didn't miss any WSTRING c_str's, so the reason is probably implicit conversions. Hunting down all those doesn't sound like fun.

So, compared with that, creating our own char_traits seems both the easier and better way to go.

- Gerry

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

Big black warning: DO NOT APPLY THE ATTACHED PATCH! It will break everything. I only attached it to show you what i'm doing. Don't add CharTraits.h either, yet.

And now, the story: After talking with some helpful friendly people at the libstdc++ list, i got the solution and started applying it to DROD. I just did BackEndLib for now, to show you what i'm doing - i don't think you will like it, but it is the only solution that is legal C++ . The contents of CharTraits.h is actually to be part of libstdc++ as an extension, thanks to one too many people having problems with this (the last straw being myself. *ahem*).

In addition to that, i added a byte order check/define to Ports.h, and fixed towlower, iswlower, iswupper, iswspace and iswdigit. A problem with these is that they are dependent on the current locale, and the small functions i did don't care about that yet. That could be a problem once exotic translations starts appearing, but for now, they serve their purpose.

Comments welcome

- Gerry


[Edited by trick on 07-09-2003 at 08:43 PM GMT: Removed the patch]

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

File: DROD-Patches.zip (16.7 KB)
Downloaded 121 times.
License: Other
From: Unspecified

Finished what i started above. Most fixes are that WSTRING stuff, but i did some of the usual fixes as well. I also removed the operator* i used in the previous patch, since it resulted in erratic behaviour.. We need to use .value in stead, now.

Anyway, now DROD compiles, links, and actually even runs in linux, complete with turning heads and music! It also crashes after about 10 seconds, but hey, it's progress

I had to temporarily define a few missing MID constants for it to work, tho.. I guess you forgot to update CVS. Anyway, i just defined them as 0 to get it to compile (not included in the patch, of course). The missing MIDs are: MID_DemoIgnored, MID_HoldIgnored, MID_PlayerSavesIgnored, MID_OverwriteHoldPrompt, MID_PlayerIgnored, MID_OverwritePlayerPrompt and MID_MakeModifiedHoldCopy.

Patches attached

- Gerry


[Edited by trick on 07-09-2003 at 08:46 PM GMT: MID_MakeModifiedHoldCopy]

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

File: drod-linux.png (579.9 KB)
Downloaded 124 times.
License: Other
From: Unspecified

And here's a screenshot of DROD in linux

- Gerry

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Wow, you're awesome Gerry! Okay then, I'll start applying your diffs...as soon as I've check everything in. I've got a large commit coming with a bunch of fixes for build 28, so stay tuned.

(The crash after 10 seconds is probably when it tries to go to the demo screen. Matt, didn't you see this same thing on your Irix port?)

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

Schik
Level: Legendary Smitemaster

Rank Points: 5383
Registered: 02-04-2003
IP: Logged

Yeah, I saw a crash at that time too... I think for me it was the endian problem, but trick shouldn't have that problem unless he's swapping bytes when he shouldn't...

____________________________
The greatness of a nation and its moral progress can be judged by the way it treats its animals.
--Mahatma Gandhi

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Okay, I've committed everything for build 28 and will start on the diffs. The DRODLib project files have been added to again.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

I've put in the changes for BackEndLib and they're not compiling on Windows. There's a pile of errors (probably mostly redefinition), starting at:

c:\\caravel\\backendlib\\chartraits.h(116) : error C2989: 'char_traits<struct character<_E> >' : template class has already been defined as a non-template class
c:\\caravel\\backendlib\\chartraits.h(116) : error C2988: unrecognizable template declaration/definition
c:\\caravel\\backendlib\\wchar.h(67) : error C2371: 'WCHAR' : redefinition; different basic types
c:\\program files\\microsoft visual studio\\vc98\\include\\winnt.h(144) : see declaration of 'WCHAR'
c:\\caravel\\backendlib\\wchar.h(77) : error C2039: 'value_type' : is not a member of '`global namespace''

...

Could you try compiling your code on a Windows machine and sending me any fixes you make?

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

mrimer wrote:
I've put in the changes for BackEndLib and they're not compiling on Windows.

Oh. Damn.

Could you try compiling your code on a Windows machine and sending me any fixes you make?

If i had Visual Studio, i could, but i don't - i could compile using cygwin, of course, but that probably wouldn't give any problems as it's just a windows versions of gcc..

c:\\caravel\\backendlib\\chartraits.h(116) : error C2989: 'char_traits<struct character<_E> >' : template class has already been defined as a non-template class
c:\\caravel\\backendlib\\chartraits.h(116) : error C2988: unrecognizable template declaration/definition
c:\\caravel\\backendlib\\wchar.h(67) : error C2371: 'WCHAR' : redefinition; different basic types
c:\\program files\\microsoft visual studio\\vc98\\include\\winnt.h(144) : see declaration of 'WCHAR'
c:\\caravel\\backendlib\\wchar.h(77) : error C2039: 'value_type' : is not a member of '`global namespace''

Um. Okay. To fix the last two, try adding #undef WCHAR to the top of Wchar.h, and change the W_t(x) macro to:

#define W_t(x) {x}


That should take care of those (and we don't need the WCHAR windows defines anyway, so that shouldn't be a problem, at least). By the way, what does the windows declaration of WCHAR look like ?

About the rest.. It's not complaining about the first template (struct character), so that one's okay, right ? But then VC complains about the template class already having been defined (as non-template ?). How is that possible ? We're using a new class that was just defined a few lines above! The only explaination i can find is that the character class is defined and used somewhere before, even though VC doesn't report it is being redeclared.

Hm, what happens if you comment out #include "CharTraits.h" and the operators below ? If VC already define the char traits for unsigned short, and the character is equal to what we have in there, it may work. I doubt we're that lucky, tho.

If that doesn't work, could you try changing the name of the character class to something else ? (CustomChar or something.. Just replace all character<T> in CharTraits.h and Wchar.h by CustomChar<T> ).

If none of that works, i don't know, and it's kinda difficult for me to figure it out since i don't have VC.. I have to talk to those friendly people again, but my email server is down (yes, again).

I guess i could try ripping out all the template stuff and replace it by hardcoded WCHAR stuff, but i don't see how that would be different, really.

Damn.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

Schik wrote:
Yeah, I saw a crash at that time too... I think for me it was the endian problem, but trick shouldn't have that problem unless he's swapping bytes when he shouldn't...

I'm pretty sure the linux code isn't swapping any bytes (the byte-swapping code is commented out, so it can't even happen by mistake), so it has to be something else causing it.

I'll have a look at it (of course), but we need to fix this windows problem first

- Gerry

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Thanks! Using

#define W_t(x) {x}

instead of the former got rid of about 300 of the 350 errors. Commenting out "CharTraits.h" and the template operators fixed the rest. Now the only thing it's left complaining about are the undefined .value or ->value members, which makes sense. I don't understand -- why do we have to make explicit reference to 'value'? Can't we just overload the operator[] (or operator*) to reference it?

Specifically, I'm thinking of something along the lines of:
template<typename T>
inline value_type& operator[](const char_type* __s, size_t __n)
{ return __s[__n].value; }

Then we wouldn't have to add those dozens of .values everywhere we access the chars, right? (Maybe I'm wrong and this is impossible. Just let me know what you think.)

[Edited by mrimer on 07-10-2003 at 03:52 AM GMT: Added the template function example]

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

Oh, in "VC98/Include/Wtypes.h", WCHAR is:

#ifndef _WCHAR_DEFINED
#define _WCHAR_DEFINED
typedef wchar_t WCHAR;
#endif

and you have

typedef unsigned short wchar_t;

written in over a dozen different other include files, by the look of it.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

trick
Level: Legendary Smitemaster
Rank Points: 2580
Registered: 04-12-2003
IP: Logged

mrimer wrote:
Thanks! Using

#define W_t(x) {x}

instead of the former got rid of about 300 of the 350 errors. Commenting out "CharTraits.h" and the template operators fixed the rest.

Great!

Now the only thing it's left complaining about are the undefined .value or ->value members, which makes sense. I don't understand -- why do we have to make explicit reference to 'value'? Can't we just overload the operator[] (or operator*) to reference it?

Sadly, no. The character and char_traits structures has to be POD (Plain Old Data), which means no non-static methods, not even constructors and destructors. I tried getting around that by defining an operator*, actually, and it compiled and linked, but exited without any errors when run. After some testing i found out the reason was that the operator*'s i used to assign values to the WCHARs didn't work at all, even though i used them correctly (i double-checked) - they just didn't assign anything.

I think a read-only operator* would work, but you still have the problem with the rest.

Specifically, I'm thinking of something along the lines of:
template<typename T>
inline value_type& operator[](const char_type* __s, size_t __n)
{ return __s[__n].value; }

That would work, but it would have the same problems as the operator* i tried, so it would have to be read-only. We could still use it read-only, of course, but it would lead to confusion since operator[] (like operator*) is usually read-write. That's why i removed the operator* and didn't just make it read-only. Let me know what you think.

Anyway, we could fix this problem by defining a system-dependent define and use that wherever WCHAR.value is used (and WCHAR->value - should probably change those to (*WCHAR).value to keep it clean):

#ifdef WIN32
#define WCv(x) (x)
#else
#define WCv(x) (x).value
#endif

(WCv is short for WideChar value). Could use something else, of course, this is just an example.

typedef wchar_t WCHAR;
..
typedef unsigned short wchar_t;

Ah. That explains a lot. I still don't understand why VC complained about the redefined char_traits, tho. Oh well.

Anyway, let me know what you think.

- Gerry

mrimer
Level: Legendary Smitemaster

Rank Points: 5058
Registered: 02-04-2003
IP: Logged

I don't understand -- why when we return a reference with & is it read-only? It's not just returning a copy of the data or anything...is it?

Anyway, we could fix this problem by defining a system-dependent define and use that wherever WCHAR.value is used (and WCHAR->value - should probably change those to (*WCHAR).value to keep it clean):

#ifdef WIN32
#define WCv(x) (x)
#else
#define WCv(x) (x).value
#endif

Ah, this looks nice. So you mean instead of putting

wszFilepath[nLength-2].value

we would write

WCv(wszFilepath[nLength-2])

right? I'll try that.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.