A fix for seeding the rng under older win GCC #11305

bkentel · Feb 18, 2015

fix seeding as the title says.
eliminate djb2_hash.
fix a likely old bug where even if a seed was used, the rng would be seeded again anyway.

KA101 · Feb 19, 2015

KA101 reviewed Feb 19, 2015

View changes

KA101 · Feb 19, 2015

src/rng.cpp

-        static_cast<unsigned>(djb2_hash(reinterpret_cast<unsigned char const*>(seed))));
+    if (!seed) {
+        // std::random_device returns the same sequence on older version of GCC on windows.
+        random_generator.seed(time(nullptr) ^ 0xDEADBEEF);


time not declared in scope, sorry. :-/

Sorry, should be ok now.

Don't get cute, just init with the value time() returns.

It was @DavidKeaton -- I would probably have just used time() by itself otherwise. But no worries.

KA101 · Feb 19, 2015

Can't test this whilst GCC refuses to compile, sorry I didn't make that clear.

kevingranade · Feb 20, 2015

kevingranade reviewed Feb 20, 2015

View changes

kevingranade · Feb 20, 2015

src/main.cpp

@@ -39,6 +39,7 @@ int main(int argc, char *argv[])
 #endif
    bool verifyexit = false;
    bool check_all_mods = false;
+    bool explicit_seed = false;


The old flow where there was a seed string that was set and then passed to the seeding function was preferable.

kevingranade · Feb 20, 2015

kevingranade reviewed Feb 20, 2015

View changes

kevingranade · Feb 20, 2015

src/rng.cpp

+        // std::random_device returns the same sequence on older version of GCC on windows.
+        random_generator.seed(time(nullptr) ^ 0xDEADBEEF);
+    } else {
+        random_generator.seed(std::hash<std::string> {}(seed));


This is an unrelated change, it has no business being here.

kevingranade · Feb 20, 2015

kevingranade reviewed Feb 20, 2015

View changes

kevingranade · Feb 20, 2015

src/text_snippets.cpp

-    categories.insert( std::pair<std::string, int>(category, hash) );
+    auto const hash = static_cast<int>(std::hash<std::string> {}(text));
+    snippets.insert( make_pair(hash, text) );
+    categories.insert( make_pair(category, hash) );


I don't even know what you're thinking, the build is broken and you're screwing around with random code that works fine already.

@bkentel Just as a note: the hash of the snippet is stored in the saves along the item data, so a player always sees the same snippet when they look at the item description. Changing the hash function will have a negative effect on save compatibility.

@BevapDin Good information to have. A comment should probably be added to djb2_hash to indicate that its removal is basically only prevented by this.

A note in the snippets header perhaps, but there's no reason for the hash function to reference things that use it, that's just asking to get out of sync.

DavidKeaton · Feb 20, 2015

DavidKeaton reviewed Feb 20, 2015

View changes

DavidKeaton · Feb 20, 2015

src/rng.cpp

-        static_cast<unsigned>(djb2_hash(reinterpret_cast<unsigned char const*>(seed))));
+    if (!seed) {
+        // std::random_device returns the same sequence on older version of GCC on windows.
+        random_generator.seed(time(nullptr) ^ 0xDEADBEEF);


I think this was in reference to my xor comment on seed initialization.

It was worth a try, I liked it. :-)

A fix for seeding the rng under older win GCC

Loading status checks…

f54a94f

KA101 self-assigned this Feb 18, 2015

KA101 removed their assignment Feb 19, 2015

missing include

Loading status checks…

d43f191

KA101 self-assigned this Feb 19, 2015

KA101 assigned KA101 and unassigned KA101 Feb 19, 2015

KA101 closed this Feb 20, 2015

CleverRaven/Cataclysm-DDA

A fix for seeding the rng under older win GCC #11305

A fix for seeding the rng under older win GCC #11305

bkentel commented Feb 18, 2015

KA101 self-assigned this Feb 18, 2015

KA101 reviewed Feb 19, 2015

View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

KA101 removed their assignment Feb 19, 2015

KA101 self-assigned this Feb 19, 2015

This comment has been minimized.

KA101 commented Feb 19, 2015

KA101 assigned KA101 and unassigned KA101 Feb 19, 2015

kevingranade reviewed Feb 20, 2015

View changes

This comment has been minimized.

kevingranade reviewed Feb 20, 2015

View changes

This comment has been minimized.

kevingranade reviewed Feb 20, 2015

View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

KA101 closed this Feb 20, 2015

DavidKeaton reviewed Feb 20, 2015

View changes

This comment has been minimized.

This comment has been minimized.

CleverRaven/Cataclysm-DDA

Join GitHub today

A fix for seeding the rng under older win GCC #11305

Conversation

bkentel commented Feb 18, 2015

KA101 self-assigned this Feb 18, 2015

KA101 reviewed Feb 19, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

KA101 removed their assignment Feb 19, 2015

KA101 self-assigned this Feb 19, 2015

This comment has been minimized.

KA101 commented Feb 19, 2015

KA101 assigned KA101 and unassigned KA101 Feb 19, 2015

kevingranade reviewed Feb 20, 2015 View changes

This comment has been minimized.

kevingranade reviewed Feb 20, 2015 View changes

This comment has been minimized.

kevingranade reviewed Feb 20, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kevingranade via email Feb 20, 2015

KA101 closed this Feb 20, 2015

DavidKeaton reviewed Feb 20, 2015 View changes

This comment has been minimized.

This comment has been minimized.

KA101 reviewed Feb 19, 2015

View changes

kevingranade reviewed Feb 20, 2015

View changes

kevingranade reviewed Feb 20, 2015

View changes

kevingranade reviewed Feb 20, 2015

View changes

DavidKeaton reviewed Feb 20, 2015

View changes