Skip to content

Commit d809648

Browse files
xezonxezon
committed
refactor(namekey): Reduce code duplication in NameKeyGenerator functions, misc formatting tweaks (#1959)
1 parent 3209a5b commit d809648

File tree

12 files changed

+104
-168
lines changed

12 files changed

+104
-168
lines changed

‎Generals/Code/GameEngine/Include/Common/NameKeyGenerator.h‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,20 @@
3636
#include "Common/AsciiString.h"
3737

3838
//-------------------------------------------------------------------------------------------------
39-
/**
40-
Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
41-
fact that NameKeys are really magic cookies, and aren't really interchangeable
42-
with ints. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
43-
determined at runtime. (The generated code is basically identical, of course.)
44-
*/
39+
// Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
40+
// fact that NameKeys are really magic cookies, and aren't really interchangeable
41+
// with integers. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
42+
// determined at runtime. (The generated code is basically identical, of course.)
4543
//-------------------------------------------------------------------------------------------------
4644
enum NameKeyType CPP_11(: Int)
4745
{
4846
NAMEKEY_INVALID = 0,
49-
NAMEKEY_MAX = 1<<23, // max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
50-
FORCE_NAMEKEYTYPE_LONG = 0x7fffffff // a trick to ensure the NameKeyType is a 32-bit int
47+
NAMEKEY_MAX = 1<<23, // max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
48+
FORCE_NAMEKEYTYPE_LONG = 0x7fffffff, // a trick to ensure the NameKeyType is a 32-bit int
5149
};
5250

5351
//-------------------------------------------------------------------------------------------------
54-
/** A bucket entry for the name key generator */
52+
// A bucket entry for the name key generator
5553
//-------------------------------------------------------------------------------------------------
5654
class Bucket : public MemoryPoolObject
5755
{
@@ -72,12 +70,12 @@ inline Bucket::Bucket() : m_nextInSocket(NULL), m_key(NAMEKEY_INVALID) { }
7270
inline Bucket::~Bucket() { }
7371

7472
//-------------------------------------------------------------------------------------------------
75-
/** This class implements the conversion of an arbitrary string into a unique
76-
* integer "key". Calling the nameToKey() method with the same string is
77-
* guaranteed to return the same key. Also, all keys generated by an
78-
* instance of this class are guaranteed to be unique with respect to that
79-
* instance's catalog of names. Multiple instances of this class can be
80-
* created to service multiple namespaces. */
73+
// This class implements the conversion of an arbitrary string into a unique
74+
// integer "key". Calling the nameToKey() method with the same string is
75+
// guaranteed to return the same key. Also, all keys generated by an
76+
// instance of this class are guaranteed to be unique with respect to that
77+
// instance's catalog of names. Multiple instances of this class can be
78+
// created to service multiple namespaces.
8179
//-------------------------------------------------------------------------------------------------
8280
class NameKeyGenerator : public SubsystemInterface
8381
{
@@ -99,16 +97,14 @@ class NameKeyGenerator : public SubsystemInterface
9997
NameKeyType nameToKey(const char* name);
10098
NameKeyType nameToLowercaseKey(const char *name);
10199

102-
/**
103-
given a key, return the name. this is almost never needed,
104-
except for a few rare cases like object serialization. also
105-
note that it's not particularly fast; it does a dumb linear
106-
search for the key.
107-
*/
100+
// given a key, return the name. this is almost never needed,
101+
// except for a few rare cases like object serialization. also
102+
// note that it's not particularly fast; it does a dumb linear
103+
// search for the key.
108104
AsciiString keyToName(NameKeyType key);
109105

110-
// Get a string out of the INI. Store it into a NameKeyType
111-
static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
106+
// Get a string out of the INI. Store it into a NameKeyType
107+
static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
112108

113109
private:
114110

@@ -126,6 +122,7 @@ class NameKeyGenerator : public SubsystemInterface
126122

127123
NameKeyType nameToKeyImpl(const char* name);
128124
NameKeyType nameToLowercaseKeyImpl(const char *name);
125+
NameKeyType createNameKey(UnsignedInt hash, const AsciiString& name);
129126

130127
void freeSockets();
131128

‎Generals/Code/GameEngine/Include/Common/Upgrade.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,10 @@ class UpgradeCenter : public SubsystemInterface
232232
void reset( void ); ///< subsystem interface
233233
void update( void ) { } ///< subsystem interface
234234

235-
UpgradeTemplate *firstUpgradeTemplate( void ); ///< return the first upgrade template
236-
const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const; ///< find upgrade by name key
237-
const UpgradeTemplate *findUpgrade( const AsciiString& name ) const; ///< find and return upgrade by name
238-
const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const; ///< find and return upgrade by name
235+
UpgradeTemplate *firstUpgradeTemplate( void ); ///< return the first upgrade template
236+
const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const; ///< find upgrade by name key
237+
const UpgradeTemplate *findUpgrade( const AsciiString& name ) const; ///< find and return upgrade by name
238+
const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const; ///< find and return upgrade by veterancy level
239239

240240
UpgradeTemplate *newUpgrade( const AsciiString& name ); ///< allocate, link, and return new upgrade
241241

‎Generals/Code/GameEngine/Include/GameClient/Image.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class ImageCollection : public SubsystemInterface
128128

129129
void load( Int textureSize ); ///< load images
130130

131-
const Image *findImageByName( const AsciiString& name ); ///< find image based on name
131+
const Image *findImageByName( const AsciiString& name ) const; ///< find image based on name
132132

133133
/// adds the given image to the collection, transfers ownership to this object
134134
void addImage(Image *image);

‎Generals/Code/GameEngine/Source/Common/NameKeyGenerator.cpp‎

Lines changed: 21 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ void NameKeyGenerator::freeSockets()
8888

8989
}
9090

91-
/* ------------------------------------------------------------------------ */
91+
//-------------------------------------------------------------------------------------------------
9292
inline UnsignedInt calcHashForString(const char* p)
9393
{
9494
UnsignedInt result = 0;
@@ -98,7 +98,7 @@ inline UnsignedInt calcHashForString(const char* p)
9898
return result;
9999
}
100100

101-
/* ------------------------------------------------------------------------ */
101+
//-------------------------------------------------------------------------------------------------
102102
inline UnsignedInt calcHashForLowercaseString(const char* p)
103103
{
104104
UnsignedInt result = 0;
@@ -164,71 +164,45 @@ NameKeyType NameKeyGenerator::nameToLowercaseKey(const char *name)
164164
}
165165

166166
//-------------------------------------------------------------------------------------------------
167-
NameKeyType NameKeyGenerator::nameToKeyImpl(const char* nameString)
167+
NameKeyType NameKeyGenerator::nameToKeyImpl(const char* name)
168168
{
169-
Bucket *b;
170-
171-
UnsignedInt hash = calcHashForString(nameString) % SOCKET_COUNT;
169+
const UnsignedInt hash = calcHashForString(name) % SOCKET_COUNT;
172170

173-
// hmm, do we have it already?
171+
// do we have it already?
172+
const Bucket *b;
174173
for (b = m_sockets[hash]; b; b = b->m_nextInSocket)
175174
{
176-
if (strcmp(nameString, b->m_nameString.str()) == 0)
175+
if (strcmp(name, b->m_nameString.str()) == 0)
177176
return b->m_key;
178177
}
179178

180179
// nope, guess not. let's allocate it.
181-
b = newInstance(Bucket);
182-
b->m_key = (NameKeyType)m_nextID++;
183-
b->m_nameString = nameString;
184-
b->m_nextInSocket = m_sockets[hash];
185-
m_sockets[hash] = b;
186-
187-
NameKeyType result = b->m_key;
188-
189-
#if defined(RTS_DEBUG)
190-
// reality-check to be sure our hasher isn't going bad.
191-
const Int maxThresh = 3;
192-
Int numOverThresh = 0;
193-
for (Int i = 0; i < SOCKET_COUNT; ++i)
194-
{
195-
Int numInThisSocket = 0;
196-
for (b = m_sockets[i]; b; b = b->m_nextInSocket)
197-
++numInThisSocket;
198-
199-
if (numInThisSocket > maxThresh)
200-
++numOverThresh;
201-
}
202-
203-
// if more than a small percent of the sockets are getting deep, probably want to increase the socket count.
204-
if (numOverThresh > SOCKET_COUNT/20)
205-
{
206-
DEBUG_CRASH(("hmm, might need to increase the number of bucket-sockets for NameKeyGenerator (numOverThresh %d = %f%%)",numOverThresh,(Real)numOverThresh/(Real)(SOCKET_COUNT/20)));
207-
}
208-
#endif
209-
210-
return result;
211-
180+
return createNameKey(hash, name);
212181
}
213182

214183
//-------------------------------------------------------------------------------------------------
215-
NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* nameString)
184+
NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* name)
216185
{
217-
Bucket *b;
186+
const UnsignedInt hash = calcHashForLowercaseString(name) % SOCKET_COUNT;
218187

219-
UnsignedInt hash = calcHashForLowercaseString(nameString) % SOCKET_COUNT;
220-
221-
// hmm, do we have it already?
188+
// do we have it already?
189+
const Bucket *b;
222190
for (b = m_sockets[hash]; b; b = b->m_nextInSocket)
223191
{
224-
if (_stricmp(nameString, b->m_nameString.str()) == 0)
192+
if (_stricmp(name, b->m_nameString.str()) == 0)
225193
return b->m_key;
226194
}
227195

228196
// nope, guess not. let's allocate it.
229-
b = newInstance(Bucket);
197+
return createNameKey(hash, name);
198+
}
199+
200+
//-------------------------------------------------------------------------------------------------
201+
NameKeyType NameKeyGenerator::createNameKey(UnsignedInt hash, const AsciiString& name)
202+
{
203+
Bucket *b = newInstance(Bucket);
230204
b->m_key = (NameKeyType)m_nextID++;
231-
b->m_nameString = nameString;
205+
b->m_nameString = name;
232206
b->m_nextInSocket = m_sockets[hash];
233207
m_sockets[hash] = b;
234208

@@ -256,7 +230,6 @@ NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* nameString)
256230
#endif
257231

258232
return result;
259-
260233
}
261234

262235
//-------------------------------------------------------------------------------------------------

‎Generals/Code/GameEngine/Source/Common/System/Upgrade.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ void UpgradeCenter::reset( void )
292292
}
293293

294294
//-------------------------------------------------------------------------------------------------
295-
/** Find upgrade matching name key */
295+
/** Find upgrade by veterancy level */
296296
//-------------------------------------------------------------------------------------------------
297297
const UpgradeTemplate *UpgradeCenter::findVeterancyUpgrade( VeterancyLevel level ) const
298298
{
@@ -301,7 +301,7 @@ const UpgradeTemplate *UpgradeCenter::findVeterancyUpgrade( VeterancyLevel level
301301
}
302302

303303
//-------------------------------------------------------------------------------------------------
304-
/** Find upgrade matching name key */
304+
/** Find upgrade by name key */
305305
//-------------------------------------------------------------------------------------------------
306306
UpgradeTemplate *UpgradeCenter::findNonConstUpgradeByKey( NameKeyType key )
307307
{
@@ -328,7 +328,7 @@ UpgradeTemplate *UpgradeCenter::firstUpgradeTemplate( void )
328328
}
329329

330330
//-------------------------------------------------------------------------------------------------
331-
/** Find upgrade matching name key */
331+
/** Find upgrade by name key */
332332
//-------------------------------------------------------------------------------------------------
333333
const UpgradeTemplate *UpgradeCenter::findUpgradeByKey( NameKeyType key ) const
334334
{
@@ -344,13 +344,11 @@ const UpgradeTemplate *UpgradeCenter::findUpgradeByKey( NameKeyType key ) const
344344
}
345345

346346
//-------------------------------------------------------------------------------------------------
347-
/** Find upgrade matching name */
347+
/** Find upgrade by name */
348348
//-------------------------------------------------------------------------------------------------
349349
const UpgradeTemplate *UpgradeCenter::findUpgrade( const AsciiString& name ) const
350350
{
351-
352351
return findUpgradeByKey( TheNameKeyGenerator->nameToKey( name ) );
353-
354352
}
355353

356354
//-------------------------------------------------------------------------------------------------

‎Generals/Code/GameEngine/Source/GameClient/System/Image.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,9 @@ void ImageCollection::addImage( Image *image )
219219
//-------------------------------------------------------------------------------------------------
220220
/** Find an image given the image name */
221221
//-------------------------------------------------------------------------------------------------
222-
const Image *ImageCollection::findImageByName( const AsciiString& name )
222+
const Image *ImageCollection::findImageByName( const AsciiString& name ) const
223223
{
224-
std::map<unsigned,Image *>::iterator i=m_imageMap.find(TheNameKeyGenerator->nameToLowercaseKey(name));
224+
std::map<unsigned,Image *>::const_iterator i=m_imageMap.find(TheNameKeyGenerator->nameToLowercaseKey(name));
225225
return i==m_imageMap.end()?NULL:i->second;
226226
}
227227

‎GeneralsMD/Code/GameEngine/Include/Common/NameKeyGenerator.h‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,20 @@
3636
#include "Common/AsciiString.h"
3737

3838
//-------------------------------------------------------------------------------------------------
39-
/**
40-
Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
41-
fact that NameKeys are really magic cookies, and aren't really interchangeable
42-
with ints. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
43-
determined at runtime. (The generated code is basically identical, of course.)
44-
*/
39+
// Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
40+
// fact that NameKeys are really magic cookies, and aren't really interchangeable
41+
// with integers. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
42+
// determined at runtime. (The generated code is basically identical, of course.)
4543
//-------------------------------------------------------------------------------------------------
4644
enum NameKeyType CPP_11(: Int)
4745
{
4846
NAMEKEY_INVALID = 0,
49-
NAMEKEY_MAX = 1<<23, // max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
50-
FORCE_NAMEKEYTYPE_LONG = 0x7fffffff // a trick to ensure the NameKeyType is a 32-bit int
47+
NAMEKEY_MAX = 1<<23, // max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
48+
FORCE_NAMEKEYTYPE_LONG = 0x7fffffff, // a trick to ensure the NameKeyType is a 32-bit int
5149
};
5250

5351
//-------------------------------------------------------------------------------------------------
54-
/** A bucket entry for the name key generator */
52+
// A bucket entry for the name key generator
5553
//-------------------------------------------------------------------------------------------------
5654
class Bucket : public MemoryPoolObject
5755
{
@@ -72,12 +70,12 @@ inline Bucket::Bucket() : m_nextInSocket(NULL), m_key(NAMEKEY_INVALID) { }
7270
inline Bucket::~Bucket() { }
7371

7472
//-------------------------------------------------------------------------------------------------
75-
/** This class implements the conversion of an arbitrary string into a unique
76-
* integer "key". Calling the nameToKey() method with the same string is
77-
* guaranteed to return the same key. Also, all keys generated by an
78-
* instance of this class are guaranteed to be unique with respect to that
79-
* instance's catalog of names. Multiple instances of this class can be
80-
* created to service multiple namespaces. */
73+
// This class implements the conversion of an arbitrary string into a unique
74+
// integer "key". Calling the nameToKey() method with the same string is
75+
// guaranteed to return the same key. Also, all keys generated by an
76+
// instance of this class are guaranteed to be unique with respect to that
77+
// instance's catalog of names. Multiple instances of this class can be
78+
// created to service multiple namespaces.
8179
//-------------------------------------------------------------------------------------------------
8280
class NameKeyGenerator : public SubsystemInterface
8381
{
@@ -99,16 +97,14 @@ class NameKeyGenerator : public SubsystemInterface
9997
NameKeyType nameToKey(const char* name);
10098
NameKeyType nameToLowercaseKey(const char *name);
10199

102-
/**
103-
given a key, return the name. this is almost never needed,
104-
except for a few rare cases like object serialization. also
105-
note that it's not particularly fast; it does a dumb linear
106-
search for the key.
107-
*/
100+
// given a key, return the name. this is almost never needed,
101+
// except for a few rare cases like object serialization. also
102+
// note that it's not particularly fast; it does a dumb linear
103+
// search for the key.
108104
AsciiString keyToName(NameKeyType key);
109105

110-
// Get a string out of the INI. Store it into a NameKeyType
111-
static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
106+
// Get a string out of the INI. Store it into a NameKeyType
107+
static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
112108

113109
private:
114110

@@ -124,6 +120,7 @@ class NameKeyGenerator : public SubsystemInterface
124120

125121
NameKeyType nameToKeyImpl(const char* name);
126122
NameKeyType nameToLowercaseKeyImpl(const char *name);
123+
NameKeyType createNameKey(UnsignedInt hash, const AsciiString& name);
127124

128125
void freeSockets();
129126

‎GeneralsMD/Code/GameEngine/Include/Common/Upgrade.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,10 @@ class UpgradeCenter : public SubsystemInterface
235235
void reset( void ); ///< subsystem interface
236236
void update( void ) { } ///< subsystem interface
237237

238-
UpgradeTemplate *firstUpgradeTemplate( void ); ///< return the first upgrade template
239-
const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const; ///< find upgrade by name key
240-
const UpgradeTemplate *findUpgrade( const AsciiString& name ) const; ///< find and return upgrade by name
241-
const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const; ///< find and return upgrade by name
238+
UpgradeTemplate *firstUpgradeTemplate( void ); ///< return the first upgrade template
239+
const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const; ///< find upgrade by name key
240+
const UpgradeTemplate *findUpgrade( const AsciiString& name ) const; ///< find and return upgrade by name
241+
const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const; ///< find and return upgrade by veterancy level
242242

243243
UpgradeTemplate *newUpgrade( const AsciiString& name ); ///< allocate, link, and return new upgrade
244244

‎GeneralsMD/Code/GameEngine/Include/GameClient/Image.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class ImageCollection : public SubsystemInterface
128128

129129
void load( Int textureSize ); ///< load images
130130

131-
const Image *findImageByName( const AsciiString& name ); ///< find image based on name
131+
const Image *findImageByName( const AsciiString& name ) const; ///< find image based on name
132132

133133
/// adds the given image to the collection, transfers ownership to this object
134134
void addImage(Image *image);

0 commit comments

Comments
 (0)