So I wanted to code a simple PRNG for generating terrain in a game. It has to have the following properties:
Computationally cheap
Generate semi random numbers between 0 and an positive integer.
I need to be able to go back and teleport anywhere in the world so random.Next doesn't cut it.
I basically want a noise function. One output for each input with repeatable results.
No seed will be used. If I need another series I'll just shift x value by a couple of millions
These are some outputs produced by my program:
1 as input (x) gives 2 as a pseudo random number
2 as input (x) gives 2 as a pseudo random number
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Threading;
using System.Security.Cryptography;
namespace Hashing_Functions
{
class Program
{
static void Main(string[] args)
{
int i0 = 0;
while (true)
{
i0++;
Console.WriteLine(RandomHashFunction(i0, 10));
//returns random number between 0 and max based on input
}
}
static int RandomHashFunction(int input, int max)
{
var SHA1 = new SHA1CryptoServiceProvider();
StringBuilder sb = new StringBuilder();
string hash2string;
double output;
byte[] hash;
SHA1.ComputeHash(ASCIIEncoding.ASCII.GetBytes(input.ToString()));
hash = SHA1.Hash;
sb.Clear();
foreach (byte b in hash)
{
sb.Append(b.ToString());
}
hash2string = sb.ToString();
output = hash2string.GetHashCode();
output = Math.IEEERemainder(output, max);
output = Math.Abs(output);
return Convert.ToInt16(output);
}
}
}
-
4\$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . \$\endgroup\$Peilonrayz– Peilonrayz ♦2017年04月06日 10:12:07 +00:00Commented Apr 6, 2017 at 10:12
-
3\$\begingroup\$ I have rolled back the last edit but kept the context edit. Please see what you may and may not do after receiving answers . \$\endgroup\$Heslacher– Heslacher2017年04月06日 10:39:35 +00:00Commented Apr 6, 2017 at 10:39
-
\$\begingroup\$ Sorry I didn't know that. Thanks for helping. \$\endgroup\$Wagacca– Wagacca2017年04月06日 13:38:59 +00:00Commented Apr 6, 2017 at 13:38
-
\$\begingroup\$ Most random number generators have ways of skipping forward and backward within their streams. For example, here is one webpage describing how to skip forward in a LCG. \$\endgroup\$JS1– JS12017年04月12日 08:36:29 +00:00Commented Apr 12, 2017 at 8:36
1 Answer 1
Clean up
I don't know why it had to be so long and to have so many temporary variables. So I shortened it to make sense out of it. Here's what I got.
static int RandomHashFunction(int input, int max)
{
using (var sha1 = new SHA1CryptoServiceProvider()) // Disposing things is a good habit
{
var ascii = ASCIIEncoding.ASCII;
var hashAsString = ascii.GetChars(
sha1.ComputeHash(
ascii.GetBytes(input.ToString())
)
);
return Convert.ToInt32(
Math.Abs(
Math.IEEERemainder(hashAsString.GetHashCode(), max)
)
);
}
}
WAT?
It is still not obvious to me, what are you attempting to achieve. Could you provide:
- a few samples of input and output.
- declare a variable that you think works as a seed.
- a reference to a particular algorithm you're trying to implement or at least are inspired by.
- OR at least a mathematical formula that you think will give you the desired result.
To me, there are a lot of unnecessary steps and manipulations.
Convert.ToInt16
was used to return andint
(which isInt32
) -- this was losing some meaning units of the number...Math.Abs
. Why bother with it if theMath.IEEERemainder
already gives you a number.- Relying on
String::GetHashCode
may also be a bad thing sinceThe behavior of GetHashCode is dependent on its implementation, which might change from one version of the common language runtime to another: MSDN
All these things seem very excessive and result in a messy code IMO.
Update 1
Updated the code to use symmetric GetChars
and GetBytes
based on the same encoding.
Update 2
Frankly, I am not finding the updated question very explanatory. The signature of the original function does not declare meaningful parameters. So I had to guess what their meaning is.
I also don't understand why a regular Random.Next*()
won't work for you. It's trivial!
static int RandomHashFunction1(int seed = 1, int max)
{
var randomDouble = new Random(seed).NextDouble();
return (int)Math.Round(randomDouble * max);
}
If you really want to stay away from System.Random
, you can use SHA1CryptoServiceProvider
as you do in the original code, but I don't see why it is necessary.
static int RandomHashFunction2(int seed = 1)
{
using (var sha1 = new SHA1CryptoServiceProvider())
return BitConverter.ToInt32(sha1.ComputeHash(BitConverter.GetBytes(seed)), startIndex: 0);
}
Lastly, I don't see why do you need to use any string representation at all and go through unnecessary steps with Math.*
which make your function computationally heavier.
Please consider accepting the answer as it does the job, OR carefully describe the acceptance criteria. Your problem statement is unclear!
Update 3
Okay, you updated the question with the important info, which I quote here
need to generate terrain so say we have a line that consists of dots with an X and a Y value. The X values start at 0 and can go to positive and negative infinity depending on the player position. Depending on the X I need a random Y. System.Random gives a random number and then another random number based on the previous one. If I would only move to the right I could keep on generating terrain based on the previous dot using random.Next () But if I wanted to go back I would need the previous number with you can't do with Random.Next().
This is the new version of the function that should meet your more precise problem definition.
static int ConsistentRandom(
int maxRandomInt,
int seedInGame = 1,
int specificLocationInGame = 0
)
{
var actualSeedForRandom = seedInGame + specificLocationInGame;
var randomDouble = new Random(actualSeedForRandom).NextDouble();
return (int)Math.Round(randomDouble * maxRandomInt);
}
Notice that var actualSeedForRandom = seedInGame + specificLocationInGame;
may be implemented in many various ways, I just used the simplest formula that came to my mind.
Here's a test console app:
public static void Main(string[] args)
{
for (var iteration = 1; iteration <= 2; iteration++)
{
Console.WriteLine("TEST ITERATION " + iteration);
for (var seed = 0; seed < 2; seed++)
{
Console.WriteLine("SEED " + seed);
for (var location = 0; location < 5; location++)
{
Console.Write(ConsistentRandom(10, seed, location));
if (location != 4) Console.Write(" - ");
}
Console.WriteLine();
}
Console.WriteLine();
}
Console.ReadKey();
}
And here's what it produces:
As you can see, as long as you know the seedInGame
, you control the sequence of randoms. And if you know both the seedInGame
and specificLocationInGame
, you can easily get the specific value of random for this combination.
In other words, your Game should pick a new seedInGame
on every new map it's trying to generate. The value of specificLocationInGame
will let you "teleport" anywhere within this particular map.
Update 4 - To Address Concerns About Randomness
Here's the exact code I used to generate some consistent but random number sequence. The resulting sequence can be downloaded from here.
static void Main(string[] args)
{
var path = @"C:\temp\generatedRandom.txt";
if (File.Exists(path))
File.Delete(path);
using (var fileStream = File.Create(path))
using (var textWriter = new BinaryWriter(fileStream, Encoding.ASCII))
{
var seed = 42;
for (var location = 0; location < 3 * 3000 * 3000; location++)
{
var randomByte = ConsistentRandom(Int64.MaxValue, seed, location);
textWriter.Write(randomByte);
Console.WriteLine(randomByte);
}
}
}
static Int64 ConsistentRandom(
Int64 maxRandomInt,
int seedInGame = 1,
int specificLocationInGame = 0
)
{
var actualSeedForRandom = seedInGame + specificLocationInGame;
var randomDouble = new Random(actualSeedForRandom).NextDouble();
return (Int64)Math.Round(randomDouble * maxRandomInt);
}
I then used ent - pseudorandom number sequence test for checking how well the solution performs in terms of randomness.
I think, only the author can judge how far or how close this result is located from the desired one.
Update 5 - Visualizing the Solution (or How I Screwed Up)
Okay, I built a little tool to visualize the generated "random" sequence via the proposed algorithm (since I didn't find one online). The following picture demonstrates "little regularities" in the sequence... Well, the "random" sequence has very distinguishable regularities, so the proposed approach does not guarantee good results. I think, it's valid to suppose that approach in fact guarantees bad results. :(
@Wagacca, you were right in our conversation under my answer!
-
\$\begingroup\$ Alright I improved it. I hope it's clear now what I want to achieve. \$\endgroup\$Wagacca– Wagacca2017年04月06日 10:03:14 +00:00Commented Apr 6, 2017 at 10:03
-
\$\begingroup\$ @Wagacca please see the Update 2 \$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年04月06日 16:49:53 +00:00Commented Apr 6, 2017 at 16:49
-
1\$\begingroup\$ Alright. I need to generate terrain so say we have a line that consists of dots with an X and a Y value. The X values start at 0 and can go to positive and negative infinity depending on the player position. Depending on the X I need a random Y. System.Random gives a random number and then another random number based on the previous one. If I would only move to the right I could keep on generating terrain based on the previous dot using random.Next () But if I wanted to go back I would need the previous number with you can't do with Random.Next(). That's why I need a hashing function. \$\endgroup\$Wagacca– Wagacca2017年04月06日 19:55:56 +00:00Commented Apr 6, 2017 at 19:55
-
\$\begingroup\$ @Wagacca now it makes WAY MORE SENSE! \$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年04月06日 20:00:25 +00:00Commented Apr 6, 2017 at 20:00
-
\$\begingroup\$ @Wagacca I have a solution in mind. Will update the answer within the next 2-6 hours... \$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年04月06日 20:14:57 +00:00Commented Apr 6, 2017 at 20:14