WTH CvEnums?

[ls612]

Before the source code for the new patch was released someone posted complaining about how the NotificationTypes enums were having supposedly nonsense numbers in Lua. It turns out that this was intentional for some reason

//  !!!!!!!!!!!!!!!!!!!!!!! 
//  IMPORTANT - NotificationTypes enum contains hash values, not an index!
//  It is not required to add a new enum for a new notification, you can just use the hash (FString::Hash) of the name
//	as defined in the Notifications.xml or equivalent.  The enum is just a helper so that the has generation is done
//	at compile time.  e.g.  the enum NOTIFICATION_TECH is equivalent to FString::Hash("NOTIFICATION_TECH")
//
//  MODDERS:
//		If you add a new notification, you do NOT have to add it to this helper list, you can just make a define
//		with the desired hash ID on the DLL side and just cast the value to a NotificationTypes value when passing it
//		to the notification system.

//	When adding Notifications, please remember to add an entry in the sound notifications file using the same name.
//  !!!!!!!!!!!!!!!!!!!!!!! 

enum NotificationTypes

This is one of the dumbest things I've seen in a while, why was the normal way of having them sequential not good? Anyways it seems that this is spread out through multiple parts of CvEnums

	BUTTONPOPUP_CHOOSE_FAITH_GREAT_PERSON,	// Do not add any more sequential enums!  Use hash values!

	// Do not add any more sequential enums!  Use hash values!

	PLAYEROPTION_MODDER_3,

	NUM_PLAYEROPTION_TYPES,		// Do not add anymore sequential enums!

	// Do not add any more sequential enums, add explicit key values (hash of text key)

	AUTOMATE_TRADE		= 0x4bdc68d8,	// FString::Hash("AUTOMATE_TRADE");

It seems that for ALL future enums we modders will add that they want to have hash values (instead of nice sequential enums). This is good to know (they are expecting us to add enums, so we don't need to worry too much about randomly breaking the engine doing so), but IMO a horrible design choice.

 

[Putmalk]

I'm not sure why, they don't explain a reason. But this is good to know (even though I would have seen it anyway at some point). Can someone provide an explanation as to how this helps?
And for example, why those specific hashes? What's the point of having to write ridiculous hex numbers?

 

[ls612]

I'm not sure why, they don't explain a reason. But this is good to know (even though I would have seen it anyway at some point). Can someone provide an explanation as to how this helps?
And for example, why those specific hashes? What's the point of having to write ridiculous hex numbers?

It should look like

enum CvFoo
{
	FOO_BAR = FString::Hash("FOO_BAR");

That hex code is simply the hash.

 

[Putmalk]

It should look like

enum CvFoo
{
	FOO_BAR = FString::Hash("FOO_BAR");

That hex code is simply the hash.

Alright.

Is this supposed to be more economical or something?

 

[ls612]

Alright.

Is this supposed to be more economical or something?

I have no idea. Maybe it is because these are shared between the DLL (which we can mess with) and the EXE (which we can't mess with and don't know a lot about) and they are worried about collisions? I don't know, and it seems useless to me.

 

[whoward69]

What's more, whomever made that design decision clearly understands nothing about the properties of enums and/or hash codes

enum { A=0, B, C, D} is guaranteed to have unique values for A, B, C and D whereas enum { A=hash(A), B=hash(B), C=hash(C), D=hash(D)} does not.

A perfectly valid (although somewhat useless) hashing function is "return 0x0000001 & value" (or even just "return 1")

 

[ls612]

What's more, whomever made that design decision clearly understands nothing about the properties of enums and/or hash codes

enum { A=0, B, C, D} is guaranteed to have unique values for A, B, C and D whereas enum { A=hash(A), B=hash(B), C=hash(C), D=hash(D)} does not.

A perfectly valid (although somewhat useless) hashing function is "return 0x0000001 & value" (or even just "return 1")

I still suspect it has something to do with the fact that we can modify the DLL but we can't modify the game executable, and stock enums are supposedly shared between the two.

 

[Putmalk]

Bumping this thread to ask the question, if I would like to create a new notification, which way would I do this?

// way #1 (current implementation)
#define NOTIFICATION_TEST FString::Hash("NOTIFICATION_TEST");

// way #2 (probably wrong...)
#define NOTIFICATION_TEST = FString::Hash("NOTIFICATION_TEST");

// way #3 (comments guide against this) (this errors)
// enum NotificationTypes
[S]NOTIFICATION_TEST = FString::Hash("NOTIFICATION_TEST"),[/S]

In addition the comments mention something about a Sound Notifications file...which one is this?

This hash stuff is confusing me greatly. Where did they come up with the specific hashes to use and how could we create them?

For example what is so special about 0x0611623D and 0x66774058 in the code below? How can I generate my own hash safely?

#define NOTIFICATION_LEAGUE_PROJECT_COMPLETE ((NotificationTypes)0x0611623D)
#define NOTIFICATION_LEAGUE_PROJECT_PROGRESS ((NotificationTypes)0x66774058)

 

[whoward69]

In addition the comments mention something about a Sound Notifications file...which one is this?

...\assets\Sounds\XML\NotificationSounds.xml

but as there seems to be no way to update that file from a mod, you can probably ignore the comment - you just won't get a sound played as the notification arrives (which IMHO is not a bad thing!)

 

[whoward69]

I would like to create a new notification, which way would I do this?

// way #1 (current implementation)
#define NOTIFICATION_TEST FString::Hash("NOTIFICATION_TEST");

// way #2 (probably wrong...)
#define NOTIFICATION_TEST = FString::Hash("NOTIFICATION_TEST");

// way #3 (comments guide against this) (this errors)
// enum NotificationTypes
[S]NOTIFICATION_TEST = FString::Hash("NOTIFICATION_TEST"),[/S]

Number #2 is obviously wrong as a #define is a "copy/paste" operation within the pre-processor and not an assignment within the compiler.

I'm surprised #3 throws an error as that would have been my first choice (and what comment guides against it, nothing I can see in CvEnums.h)

 

[Putmalk]

Number #2 is obviously wrong as a #define is a "copy/paste" operation within the pre-processor and not an assignment within the compiler.

I'm surprised #3 throws an error as that would have been my first choice (and what comment guides against it, nothing I can see in CvEnums.h)

//
//  MODDERS:
//		If you add a new notification, you do NOT have to add it to this helper list, you can just make a define
//		with the desired hash ID on the DLL side and just cast the value to a NotificationTypes value when passing it
//		to the notification system.

The error that on

NOTIFICATION_TEST = FString::Hash("NOTIFICATION_TEST"),

is "function call is not allowed in a constant expression"

 

[whoward69]

is "function call is not allowed in a constant expression"

Too used to Java

So because of restrictions in the C++ compiler, it looks like the dev's printed out all the values from FString::Hash() and entered them by hand into the enum ... and that didn't set alarm bells ringing for them!!!

 

[Putmalk]

Too used to Java

So because of restrictions in the C++ compiler, it looks like the dev's printed out all the values from FString::Hash() and entered them by hand into the enum ... and that didn't set alarm bells ringing for them!!!

Pretty much, *facepalm*.

Anyway, I'm going to send you a PM, I have a question to ask you on the UI.

 

[ls612]

...\assets\Sounds\XML\NotificationSounds.xml

but as there seems to be no way to update that file from a mod, you can probably ignore the comment - you just won't get a sound played as the notification arrives (which IMHO is not a bad thing!)

That is for the future. Dennis Shirk said on the most recent Polycast that sound editing would be fixed for mods in the Fall Patch (yay!).

 

[Putmalk]

That is for the future. Dennis Shirk said on the most recent Polycast that sound editing would be fixed for mods in the Fall Patch (yay!).

Perfect, did he also say terraforming...?

 

[ls612]

Perfect, did he also say terraforming...?

No, he said nothing about it. It may not hurt to bring it up specifically in the forums, he and other Firaxis people do look at this part.