Jump to content


Photo

My patches and contribution


  • Please log in to reply
95 replies to this topic

#76 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 17 October 2014 - 11:55 AM

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 :)
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#77 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 21 November 2014 - 06:32 AM

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 by Tels, 21 November 2014 - 06:33 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#78 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3669 posts

Posted 21 November 2014 - 07:19 AM

Good forward planning there :)

#79 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 23 November 2014 - 04:16 AM

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

http://swift-mazes.c..._2014-11-23.txt

I believe the following patches have not yet been applied:

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

What can I do to make this go faster?

Edited by Tels, 23 November 2014 - 04:33 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#80 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3669 posts

Posted 23 November 2014 - 06:06 AM

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.

#81 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 23 November 2014 - 11:04 AM

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 by Tels, 23 November 2014 - 11:07 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#82 Bikerdude

Bikerdude

    Mod hero

  • Member
  • PipPipPipPipPip
  • 18538 posts

Posted 23 November 2014 - 11:19 AM

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..?

#83 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3669 posts

Posted 23 November 2014 - 11:19 AM

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.

#84 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 05 January 2015 - 10:42 AM

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


"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#85 SeriousToni

SeriousToni

    Beginner Contest 2012 Winner

  • Member
  • PipPipPipPip
  • 2196 posts

Posted 05 January 2015 - 11:14 AM

Less crashes, nice! :D
"Einen giftigen Trank aus Kräutern und Wurzeln für die närrischen Städter wollen wir brauen." - Text aus einem verlassenen Heidenlager

#86 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 17 August 2015 - 08:04 AM

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

 

http://swift-mazes.c...unused_vars.txt


  • SeriousToni likes this
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#87 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3669 posts

Posted 17 August 2015 - 09:04 AM

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!

#88 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 18 August 2015 - 10:58 AM

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


"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#89 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 19 August 2015 - 07:37 AM

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 by Tels, 19 August 2015 - 07:39 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#90 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 19 August 2015 - 07:51 AM

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 by Tels, 19 August 2015 - 07:55 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#91 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 19 August 2015 - 07:54 AM

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;

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#92 SeriousToni

SeriousToni

    Beginner Contest 2012 Winner

  • Member
  • PipPipPipPip
  • 2196 posts

Posted 19 August 2015 - 04:39 PM

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

Keep up the great efforts! Seriously!! :)


Edited by SeriousToni, 19 August 2015 - 04:39 PM.

"Einen giftigen Trank aus Kräutern und Wurzeln für die närrischen Städter wollen wir brauen." - Text aus einem verlassenen Heidenlager

#93 NagaHuntress

NagaHuntress

    Member

  • Member
  • PipPip
  • 39 posts

Posted 19 August 2015 - 05:29 PM

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.
 



#94 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3669 posts

Posted 20 August 2015 - 01:21 AM

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.



#95 NagaHuntress

NagaHuntress

    Member

  • Member
  • PipPip
  • 39 posts

Posted 20 August 2015 - 06:38 AM

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.



#96 nbohr1more

nbohr1more

    Darkmod PR, Wordsmith

  • Development Role
  • PipPipPipPipPip
  • 7648 posts

Posted 08 September 2016 - 04:04 PM

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...


  • Bikerdude likes this
Please visit TDM's IndieDB site and help promote the mod:

http://www.indiedb.c...ds/the-dark-mod

(Yeah, shameless promotion... but traffic is traffic folks...)




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users