search

idkclub / dicebot

🎲 /roll support for Slack
https://dicebot.idk.club
The Unlicense
53 stars 20 forks source link

roll.go is missing rand.Seed initialization #50

Closed sztrzask closed 5 years ago

sztrzask commented 5 years ago

Hi,

I've noticed that rolls doesn't seem random.

I've checked roll.go file and I've noticed that you're missing Seed call? Apparently you should seed rand to minimize sequence and pattern occurrences .

Reference https://stackoverflow.com/questions/39529364/go-rand-intn-same-number-value

arkie commented 5 years ago

It's currently Seed with the time during each call, I believe -https://github.com/arkie/dicebot/blob/master/roll.go#L187

Let me know if it seems like that's not actually taking affect due to being outside the package, however.

sztrzask commented 5 years ago

Cool. Do you happen to know if slack provides correct time.Now() for go or is it "frozen" like in go playground?

edit: nah, nevermind, it must work

arkie commented 5 years ago

time.Now is actually evaluated on the server running the bot (i.e. in App Engine), Slack just triggers the request.