DRD Team

Graf Zahl wrote:That fix gets undone each time I update from ZDoom - because it's not in there. v_video is one of the files that I have marked as having no difference so it just gets copied over again.

If you can verify that your fix has no side effects on ZDoom itself I'll include it there, too.

Graf Zahl wrote:Can you please explain what you did in zstrformat? Your fix does not make much sense so unless you give a good explanation for it I'll revert it before committing.

Index: src/zstrformat.cpp
===================================================================
--- src/zstrformat.cpp	(revisione 405)
+++ src/zstrformat.cpp	(copia locale)
@@ -690,7 +690,7 @@
 				precision = DEFPREC;
 			}
 			dblarg = va_arg(arglist, double);
-			obuff = dtoaresult = dtoa(dblarg, expchar ? 2 : 3, precision, &expt, &signflag, &dtoaend);
+			obuff = dtoaresult = dtoa(dblarg, expchar, precision, &expt, &signflag, &dtoaend);
 //fp_common:
 			decimal_point = localeconv()->decimal_point;
 			flags |= F_SIGNED;

GuntherDW wrote:

Graf Zahl wrote:That fix gets undone each time I update from ZDoom - because it's not in there. v_video is one of the files that I have marked as having no difference so it just gets copied over again.

If you can verify that your fix has no side effects on ZDoom itself I'll include it there, too.

I've just tested a clean r1737 ZDoom with the #ifdef __GNUC__ part.
It doesn't seem to affect anyting in a negative way. I've played through a map without any problem, the video system also initialised without any problems whatsoever.

Daniele C. wrote:Also, I am not sure that this fix is for non-Windows systems only: somebody should check that SDL is not initialized twice also on Windows systems for example. I am sure it is.

Daniele C. wrote:

GuntherDW wrote:

Graf Zahl wrote:That fix gets undone each time I update from ZDoom - because it's not in there. v_video is one of the files that I have marked as having no difference so it just gets copied over again.

If you can verify that your fix has no side effects on ZDoom itself I'll include it there, too.

I've just tested a clean r1737 ZDoom with the #ifdef __GNUC__ part.
It doesn't seem to affect anyting in a negative way. I've played through a map without any problem, the video system also initialised without any problems whatsoever.

#ifdef __GNUC__ is incorrect: it means that you don't want that code when building without GCC.
One can also use GCC on Windows, so it you are targeting non-Windows systems it's better to use #ifndef WIN32; I am actually using it that way.

Also, I am not sure that this fix is for non-Windows systems only: somebody should check that SDL is not initialized twice also on Windows systems for example. I am sure it is.

Daniele C. wrote:

Graf Zahl wrote:Can you please explain what you did in zstrformat? Your fix does not make much sense so unless you give a good explanation for it I'll revert it before committing.

Note: explanation was already given, e.g. it fixes a crash ALWAYS happening on Linux when compiling with gcc 4.4

gcc (Gentoo 4.4.0 p1.1) 4.4.0

Gez wrote:

Daniele C. wrote:Also, I am not sure that this fix is for non-Windows systems only: somebody should check that SDL is not initialized twice also on Windows systems for example. I am sure it is.

I think SDL is initialized not thrice, not twice, not once, but "zeroce" on Windows. Randy made the smart decision of not using SDL at all for Windows, instead relying on DirectX.

SDL is nice for Linux, but its Windows version is crap. It doesn't support non-US keyboards, it doesn't support mouse buttons past the thrice or fourth, it doesn't support USB keypads, it doesn't support a lot of other things; and that's just for input. I care less about the audio part but I've been led to understand that Win-SDL is spotty in that domain too.

Daniele C. wrote: I found out that my patch was a lucky strike (it was probably selecting some debug algorithm for dtoa()), since the bug is actually in dtoa() itself, see http://patrakov.blogspot.com/2009/03/do ... dtoac.html.

ZDoom has the same behaviour so this should be also fixed upstream.

Daniele C. wrote: It's sad to hear such bad experiences with SDL, I remember it was a really bright star when it apperead in the cross-platform world

GuntherDW wrote:That was my first one, but i didn't know that it wasn't the cause of graf not implementing it. (#ifndef _WIN32, or should i use it without the _ in front of it?)
And what about a 64bit of windows? Or does WIN32 still apply on it?

Graf Zahl wrote:

Daniele C. wrote: It's sad to hear such bad experiences with SDL, I remember it was a really bright star when it apperead in the cross-platform world

SDL has so many problems and shortcomings that it's really no surprise.

You want to use SDL's sound system? Prepare yourself for crude and limited sound support where you have to do all the dirty stuff yourself and have to live with severe limitations (AFAIK it only can handle stereo but no surround.)

You want to use the graphics subsystem? Ok, but it's so tightly integrated with the input code that you can't use one without the other. And here's where things break down because SDL's input subsystem under Windows is an abysmal piece of utter crap.

Keyboard handling? As Gez said, there's no decent international support under Windows? Why? The SDL developers made the incredibly stupid decision to use DirectInput exclusively when a Windows app is run fullscreen. DirectInput doesn't even know the concept of keyboard mappings so SDL does it itself - and all it provides is a en-us layout. No chance to change it and no chance to switch back to Windows messaging for receiving text input. This blunder is the biggest show stopper for most international users. Sadly SDL is not the only software failing here.

There's not much to say about the rest of the input system, except that the code is very old and hasn't been maintained well.

Graf Zahl wrote:My past experience with SDL began when I started working on PrBoom's GL renderer - something that ultimately ended up being GZDoom's renderer. The bad keyboard support had been bothering me before so I tried to remedy it. The only way I could do it was to take SDL and rip out half of its guts and then add ZDoom's keyboard system into the mix. Since SDL was already mutilated to bits and shreds at that point I didn't stop and replaced the video code, too, with something that worked better. And after doing that the program worked so much better that I never gave SDL another chance.

Graf Zahl wrote:

Daniele C. wrote: I found out that my patch was a lucky strike (it was probably selecting some debug algorithm for dtoa()), since the bug is actually in dtoa() itself, see http://patrakov.blogspot.com/2009/03/do ... dtoac.html.

ZDoom has the same behaviour so this should be also fixed upstream.

Coincidentally, that code in ZDoom was changed recently so I won't apply your patch. Please run a test with the latest ZDoom to see if it's better now.

Index: src/v_video.cpp
===================================================================
--- src/v_video.cpp	(revisione 413)
+++ src/v_video.cpp	(copia locale)
@@ -1499,6 +1499,9 @@
 	M_InitVideoModesMenu();
 	BorderNeedRefresh = screen->GetPageCount ();
 	setsizeneeded = true;
+#ifndef	_WIN32
+	setmodeneeded = false;
+#endif
 }
 
 void V_Shutdown()
Index: src/m_options.cpp
===================================================================
--- src/m_options.cpp	(revisione 413)
+++ src/m_options.cpp	(copia locale)
@@ -3211,7 +3211,7 @@
 
 void UpdateJoystickMenu(IJoystickConfig *selected)
 {
-	int i;
+	unsigned int i;
 	menuitem_t item = { whitetext };
 	int itemnum = -1;
 
Index: src/gl/old_renderer/gl1_flats.cpp
===================================================================
--- src/gl/old_renderer/gl1_flats.cpp	(revisione 413)
+++ src/gl/old_renderer/gl1_flats.cpp	(copia locale)
@@ -575,7 +575,7 @@
 			fixed_t lastceilingheight=sector->CenterCeiling();	// render only in the range of the
 			fixed_t lastfloorheight=sector->CenterFloor();		// current sector part (if applicable)
 			F3DFloor * rover;	
-			int k;
+			unsigned int k;
 			
 			// floors are ordered now top to bottom so scanning the list for the best match
 			// is no longer necessary.

Graf Zahl wrote:... reintroducing one serious crash bug again!

I won't bother with Linux warning removal patches anymore. Yours was not the first that did some careless things.

Index: src/v_video.cpp
===================================================================
--- src/v_video.cpp   (revisione 413)
+++ src/v_video.cpp   (copia locale)
@@ -1499,6 +1499,9 @@
   M_InitVideoModesMenu();
   BorderNeedRefresh = screen->GetPageCount ();
   setsizeneeded = true;
+#ifndef   _WIN32
+   setmodeneeded = false;
+#endif
}

void V_Shutdown()
Index: src/m_options.cpp
===================================================================
--- src/m_options.cpp	(revisione 413)
+++ src/m_options.cpp	(copia locale)
@@ -3211,7 +3211,7 @@
 
 void UpdateJoystickMenu(IJoystickConfig *selected)
 {
-	int i;
+	unsigned int i;
 	menuitem_t item = { whitetext };
 	int itemnum = -1;
 
@@ -3223,7 +3223,7 @@
 	}
 	if (selected != NULL)
 	{
-		for (i = 0; (unsigned)i < Joysticks.Size(); ++i)
+		for (i = 0; i < Joysticks.Size(); ++i)
 		{
 			if (Joysticks[i] == selected)
 			{
@@ -3281,10 +3281,10 @@
 
 		item.type = joymore;
 		item.e.mfunc = StartJoystickConfigMenu;
-		for (int i = 0; i < (int)Joysticks.Size(); ++i)
+		for (i = 0; i < Joysticks.Size(); ++i)
 		{
 			item.a.joyselection = i;
-			if (i == itemnum)
+			if (i == (unsigned int)itemnum)
 			{
 				JoystickMenu.lastOn = JoystickItems.Size(); 
 			}
@@ -3308,14 +3308,14 @@
 
 	// If the joystick config menu is open, close it if the device it's
 	// open for is gone.
-	for (i = 0; (unsigned)i < Joysticks.Size(); ++i)
+	for (i = 0; i < Joysticks.Size(); ++i)
 	{
 		if (Joysticks[i] == SELECTED_JOYSTICK)
 		{
 			break;
 		}
 	}
-	if (i == (int)Joysticks.Size())
+	if (i == Joysticks.Size())
 	{
 		SELECTED_JOYSTICK = NULL;
 		if (CurrentMenu == &JoystickConfigMenu)
Index: src/gl/old_renderer/gl1_flats.cpp
===================================================================
--- src/gl/old_renderer/gl1_flats.cpp	(revisione 413)
+++ src/gl/old_renderer/gl1_flats.cpp	(copia locale)
@@ -575,13 +575,13 @@
 			fixed_t lastceilingheight=sector->CenterCeiling();	// render only in the range of the
 			fixed_t lastfloorheight=sector->CenterFloor();		// current sector part (if applicable)
 			F3DFloor * rover;	
-			int k;
+			unsigned int k;
 			
 			// floors are ordered now top to bottom so scanning the list for the best match
 			// is no longer necessary.
 
 			ceiling=true;
-			for(k=0;k<(int)x.ffloors.Size();k++)
+			for(k=0;k<x.ffloors.Size();k++)
 			{
 				rover=x.ffloors[k];
 				
@@ -638,7 +638,7 @@
 			}
 				  
 			ceiling=false;
-			for(k=x.ffloors.Size()-1;k>=0;k--)
+			for(int k=(int)x.ffloors.Size()-1;k>=0;k--)
 			{
 				rover=x.ffloors[k];
 

Graf Zahl wrote:You replaced some signed variables with unsigned ones - including one that needed to be signed. The result was a crash with 3D floors.