My patches and contribution

Tels · October 17, 2014

SteveL, Anderson sent me a few missing strings, and I've folded them in. The archive on the server has been updated, if you already downloaded it, please re-download

Tels · November 21, 2014

Here is a small patch that adds one comment and fixes another one:

Index: game/Entity.h
===================================================================
--- game/Entity.h	   (revision 6166)
+++ game/Entity.h	   (working copy)
@@ -144,7 +144,7 @@

//
// Signals
-// make sure to change script/doom_defs.script if you add any, or change their order
+// make sure to change script/tdm_defs.script if you add any, or change their order
//
typedef enum {
    SIG_TOUCH,							  // object was touched
Index: game/Game_local.h
===================================================================
--- game/Game_local.h   (revision 6166)
+++ game/Game_local.h   (working copy)
@@ -91,7 +91,7 @@
// Tels: If you change this value, make sure that LUDICROUS_INDEX
// in renderer/RenderWorld_local.h is higher than MAX_GENTITIES:
#define	    GENTITYNUM_BITS				 13
-#define	    MAX_GENTITIES				   (1<<GENTITYNUM_BITS)
+#define	    MAX_GENTITIES				   (1<<GENTITYNUM_BITS)		    // max entity count
#define	    ENTITYNUM_NONE				  (MAX_GENTITIES-1)
#define	    ENTITYNUM_WORLD				 (MAX_GENTITIES-2)
#define	    ENTITYNUM_MAX_NORMAL    (MAX_GENTITIES-2)
Index: renderer/RenderWorld_local.h
===================================================================
--- renderer/RenderWorld_local.h	    (revision 6166)
+++ renderer/RenderWorld_local.h	    (working copy)
@@ -21,7 +21,7 @@
#define __RENDERWORLDLOCAL_H__

// assume any lightDef or entityDef index above this is an internal error
-#define LUDICROUS_INDEX	    65537		   // (2 ** 16) + 1;
+#define LUDICROUS_INDEX	    65537		   // (2 ** 16) + 1;	   must be higher than MAX_GENTITIES


typedef struct portal_s {

Edited November 21, 2014 by Tels

SteveL · November 21, 2014

Good forward planning there

Tels · November 23, 2014

Here is a small patch quietening a compiler warning and cleaning up some left-over from doom cruft:

http://swift-mazes.com/pub/tdm-patches/tels_patch_o_cleanup_2014-11-23.txt

I believe the following patches have not yet been applied:

http://swift-mazes.c..._2014-10-04.txt

What can I do to make this go faster?

Edited November 23, 2014 by Tels

SteveL · November 23, 2014

What can I do to make this go faster?

Make patches that'll make a difference to players. Tweaks to comments and even quieting compiler warnings are positive things but they're not likely to make it to the top of anyone's to-do list for merging and testing when there's a release upcoming.

I've had the glasswarp patch assigned to me for a month or so, but I haven't touched it because it's a needle in the haystack. There's loads of redundant code for old GPUs that I'd like to see the back of so I don't have to work round it, but removing one bit won't do much to help.

Tels · November 23, 2014

Make patches that'll make a difference to players.

I'm sorry, but my limited time does not permit me to do major work, and since creating patches from a "not being able to commit or branch" SVN tree is easily twice the work, this further hampers my ability to contribute. And since I can't commit even the most simple typo in a comment....

So, you have to take what I produce. Or, if you are not interested in such changes anymore, just tell me, and I'll find other interesting things todo. -_-

Edit: And to expand on that: If not even the simply patches are cross-checked and commited, I'm not convinced any other major changes "making a difference to the players" will be commited either. So I'm very vary to spend time on things that then will be ignored. For instance the language patch, I don't think it has been applied. That work was done weeks ago and I'm just waiting for it to get applied so I can continue with it. Sorry, but I don't want to waste neither my nor your time.

Edited November 23, 2014 by Tels

Bikerdude · November 23, 2014

For instance the language patch, I don't think it has been applied. That work was done weeks ago and I'm just waiting for it to get applied so I can continue with it. Sorry, but I don't want to waste neither my nor your time.

I thoguht l10n was already part of the mod..?

SteveL · November 23, 2014

Edit: And to expand on that: If not even the simply patches are cross-checked and commited, I'm not convinced any other major changes "making a difference to the players" will be commited either. So I'm very vary to spend time on things that then will be ignored. For instance the language patch, I don't think it has been applied. That work was done weeks ago and I'm just waiting for it to get applied so I can continue with it. Sorry, but I don't want to waste neither my nor your time.

The patches you've provided that are important for 2.03 have been committed immediately. Take the Linux compilation bugs for example: those matter to people playing on Linux, so they got prioritised and committed immediately. The language patch *did* get commited, last Monday (rev 14071). It's just that these housekeeping patches will need to wait for 2.04, given that people are tidying up their tracker issues and regretfully bumping stuff they can't finish this cycle to 2.04. I just bumped all my animation issues this morning despite being 90% on 2 of the 3.

Tels · January 5, 2015

I've added a possible patch for bug http://bugs.thedarkmod.com/view.php?id=3968to the tracker.

SeriousToni · January 5, 2015

Less crashes, nice!

Tels · August 17, 2015

Here is a patch that removes a few variables that are computed and set, but never used:

http://swift-mazes.com/pub/tdm-patches/tels_patch_unused_vars.txt

SteveL · August 17, 2015

Ta. I'd love to get the brush and pan out and make a major assault on these compiler warnings some time. This is a start!

Tels · August 18, 2015

Steve, if you want, I can look into cleaning up more of them. Just say a word.

Tels · August 19, 2015

I've looked into producing a better patch, will post this in a minute. But there are indeed some more troubling warnings from the compiler:

sound/snd_emitter.cpp:181:15: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
 openalSource = NULL;
              ^
sound/snd_system.cpp: In member function ‘virtual void idSoundSystemLocal::Shutdown()’:
sound/snd_system.cpp:449:28: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
   openalSources[i].handle = NULL;
                           ^
sound/snd_system.cpp: In member function ‘ALuint idSoundSystemLocal::AllocOpenALSource(idSoundChannel*, bool, bool)’:
sound/snd_system.cpp:1221:10: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
  return NULL;
         ^
sound/snd_system.cpp: In member function ‘void idSoundSystemLocal::FreeOpenALSource(ALuint)’:
sound/snd_system.cpp:1235:41: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
    openalSources[i].chan->openalSource = NULL;
                                        ^
sound/snd_world.cpp: In member function ‘void idSoundWorldLocal::AddChannelContribution(idSoundEmitterLocal*, idSoundChannel*, int, int, float*)’:
sound/snd_world.cpp:1923:53: warning: passing NULL to non-pointer argument 3 of ‘void alSourcei(ALuint, ALenum, ALint)’ [-Wconversion-null]
     alSourcei( chan->openalSource, AL_BUFFER, NULL );

Basically, openalSource is an integer, not a pointer. Setting it to NULL can't be right. Maybe it was supposed to be 0 instead?

And:

sound/snd_emitter.cpp: In member function ‘void idSlowChannel::GenerateSlowChannel(FracTime&, int, float*)’:
sound/snd_emitter.cpp:1180:24: warning: variable ‘orgTime’ set but not used [-Wunused-but-set-variable]
 int i, neededSamples, orgTime, zeroedPos, count = 0;

This patch removes orgTime (zeroedPos fills its role already):


Index: sound/snd_emitter.cpp 
===================================================================
--- sound/snd_emitter.cpp       (revision 6532)
+++ sound/snd_emitter.cpp       (working copy)
@@ -1177,7 +1177,7 @@
void idSlowChannel::GenerateSlowChannel( FracTime& playPos, int sampleCount44k, float* finalBuffer ) {
       idSoundWorldLocal *sw = static_cast<idSoundWorldLocal*>( soundSystemLocal.GetPlayingSoundWorld() );
       float in[MIXBUFFER_SAMPLES+3], out[MIXBUFFER_SAMPLES+3], *src, *spline, slowmoSpeed;
-       int i, neededSamples, orgTime, zeroedPos, count = 0;
+       int i, neededSamples, zeroedPos, count = 0;
 
       src = in + 2;
       spline = out + 2;
@@ -1190,7 +1190,6 @@
       }
 
       neededSamples = sampleCount44k * slowmoSpeed + 4;
-       orgTime = playPos.time;
 
       // get the channel's samples
       chan->GatherChannelSamples( playPos.time * 2, neededSamples, src );

Edited August 19, 2015 by Tels

Tels · August 19, 2015

There are a lot more oddities:

/* 
=============== 
StringCRC 
=============== 
*/ 
ID_INLINE unsigned int StringCRC( const char *str ) { 
       unsigned int i, crc; 
       const unsigned char *ptr; 

       crc = 0; 
       ptr = reinterpret_cast<const unsigned char*>(str); 
       for ( i = 0; str[i]; i++ ) { 
               crc ^= str[i] << (i & 3); 
       }
       return crc; 
}

Should ptr be used instead of str, or just removed? Edited August 19, 2015 by Tels

Tels · August 19, 2015

Here is another case where an int gets assigned NULL instead of 0:

Index: framework/Common.cpp 
===================================================================
--- framework/Common.cpp        (revision 6532)
+++ framework/Common.cpp        (working copy)
@@ -2626,7 +2626,7 @@
       GetGameAPI = (GetGameAPI_t) Sys_DLL_GetProcAddress( gameDLL, "GetGameAPI" );
       if ( !GetGameAPI ) {
               Sys_DLL_Unload( gameDLL );
-               gameDLL = NULL;
+               gameDLL = 0;
               common->FatalError( "couldn't find game DLL API" );
               return;
       }
@@ -2651,7 +2651,7 @@
       // Tels: The game DLL does its own check, and exits, so this check is probably never run.
       if ( gameExport.version != GAME_API_VERSION ) {
               Sys_DLL_Unload( gameDLL );
-               gameDLL = NULL;
+               gameDLL = 0;
               common->FatalError( "wrong game DLL API version" );
               return;
       }
@@ -2683,7 +2683,7 @@
 
       if ( gameDLL ) {
               Sys_DLL_Unload( gameDLL );
-               gameDLL = NULL;
+               gameDLL = 0;
       }
       game = NULL;
       gameEdit = NULL;

SeriousToni · August 19, 2015

Two serious guys doing serious business no one understands, but everyone seriously supports.

Keep up the great efforts! Seriously!!

Edited August 19, 2015 by SeriousToni

NagaHuntress · August 19, 2015

I've looked into producing a better patch, will post this in a minute. But there are indeed some more troubling warnings from the compiler:

sound/snd_emitter.cpp:181:15: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
 openalSource = NULL;
              ^
sound/snd_system.cpp: In member function ‘virtual void idSoundSystemLocal::Shutdown()’:
sound/snd_system.cpp:449:28: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
   openalSources[i].handle = NULL;
                           ^
sound/snd_system.cpp: In member function ‘ALuint idSoundSystemLocal::AllocOpenALSource(idSoundChannel*, bool, bool)’:
sound/snd_system.cpp:1221:10: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
  return NULL;
         ^
sound/snd_system.cpp: In member function ‘void idSoundSystemLocal::FreeOpenALSource(ALuint)’:
sound/snd_system.cpp:1235:41: warning: converting to non-pointer type ‘ALuint {aka unsigned int}’ from NULL [-Wconversion-null]
    openalSources[i].chan->openalSource = NULL;
                                        ^
sound/snd_world.cpp: In member function ‘void idSoundWorldLocal::AddChannelContribution(idSoundEmitterLocal*, idSoundChannel*, int, int, float*)’:
sound/snd_world.cpp:1923:53: warning: passing NULL to non-pointer argument 3 of ‘void alSourcei(ALuint, ALenum, ALint)’ [-Wconversion-null]
     alSourcei( chan->openalSource, AL_BUFFER, NULL );

Basically, openalSource is an integer, not a pointer. Setting it to NULL can't be right. Maybe it was supposed to be 0 instead?

I saw this when doing my 64 bit patch. The problem is that there is documentation out there say that NULL should be used for Open AL's integer handles. In practice people should be using AL_NONE instead.

SteveL · August 20, 2015

I thought that NULL was just a #define 0 in any case, i.e. an integral type already? An I just mistaken or is there a difference between platforms?

Some of the "unused" variables are used in sections of the code hidden by compiler switches, so that's another tricky thing to look out for.

NagaHuntress · August 20, 2015

I thought that NULL was just a #define 0 in any case, i.e. an integral type already? An I just mistaken or is there a difference between platforms?

In plain C, NULL is actually defined as ((void *)0), but in C++ it can be defined as 0, as C++ uses 0 as a null pointer. However, in GCC it defines NULL as "__null", as tested by the following code:

#include <stdio.h>
#define STRINGIFY(x) #x
#define STRINGIT(x) STRINGIFY(x)
main () {
	printf("A NULL is: " STRINGIT(NULL) "\n");
}

$ gcc -g nulltest.c && ./a.out 
A NULL is: ((void *)0)
$ gcc -g nulltest.cpp && ./a.out 
A NULL is: __null

__null is internal to GCC and used to help generate warnings.

Typically NULL is used when you want a null pointer (i.e. a pointer to nothing), so if you're trying to implicitly cast it to an integer, you're either doing pointer black magic wrong, or have a bug on your hands, so GCC's warning is a good thing. Using NULL with OpenAL's handles is just abusing C++'s definition of NULL, and I regard it as bad coding practice.

nbohr1more · September 8, 2016

Well. I might just take up the mantle for moving the bulk of info_location into game code.

Using it for "light probes", fps based "dynamic quality variance", etc looks like it might become painful.

Pending further evaluation...

My patches and contribution

Recommended Posts

Link to comment

Share on other sites

Top Posters In This Topic

Popular Days

Top Posters In This Topic

Popular Days

Popular Posts

Tels

Obsttorte

nbohr1more

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Recent Status Updates