Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Moving ACE.Server.Physics.Common.Random to ACE.Common.Random #1162

Closed
wants to merge 5 commits into from
Closed

Moving ACE.Server.Physics.Common.Random to ACE.Common.Random #1162

wants to merge 5 commits into from

Conversation

fartwhif
Copy link
Collaborator

@fartwhif fartwhif commented Dec 21, 2018 •
edited

replaced the old thread discriminator with a lock
renamed to ThreadSafeRandom and functions to the familiar Next

@fartwhif
Moved ACE.Server.Physics.Common.Random to ACE.Common.Random
7abd049 
Changed Random wrapper to instantiate a new Random when needed and removed the heavyweight wrapping
@fartwhif fartwhif changed the title Moved ACE.Server.Physics.Common.Random to ACE.Common.Random Moving ACE.Server.Physics.Common.Random to ACE.Common.Random Dec 21, 2018
gmriggs and others added 2 commits December 20, 2018 20:10
@gmriggs @fartwhif
Adding sticky melee distance to charge attack
d965bb7
@fartwhif
added Lock
5f5b491 
changed class name to ThreadSafeRandom
changed function names to Next
@fartwhif
Copy link
Collaborator Author

fartwhif commented Dec 21, 2018 •
edited

OldRandom.RollDice(int min, int max): 00:00:10.0000056, iterations: 134002807 OldRandom.RollDice(float min, float max): 00:00:10.0000075, iterations: 137180787 OldRandom.RollDice(uint min, uint max): 00:00:10.0000001, iterations: 136066119 ThreadSafeRandom.Next(int min, int max): 00:00:10.0000001, iterations: 225691575 ThreadSafeRandom.Next(float min, float max): 00:00:10.0000003, iterations: 231988872 ThreadSafeRandom.Next(uint min, uint max): 00:00:10.0000002, iterations: 228039807

gmriggs
gmriggs requested changes Dec 21, 2018
View reviewed changes
Copy link
Collaborator

@gmriggs gmriggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are referencing ACE.Common, that namespace should be included in the usings for those classes

getting a random # should be ThreadSafeRandom, instead of Common.ThreadSafeRandom

@fartwhif
Copy link
Collaborator Author

fartwhif commented Dec 21, 2018 •
edited

No problem. Actually, I just noticed that I accidentally included one of your branches. I'll submit a new PR tonight after I remove your branch concerning melee combat and refactor ACE.Common.ThreadSafeRandom to just ACE.ThreadSafeRandom

Mag-nus
Mag-nus reviewed Dec 21, 2018
View reviewed changes
Source/ACE.Common/Random.cs
namespace ACE.Common
{
// important class, ensure unit tests pass for this
public static class ThreadSafeRandom
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be named ThreadSafeRandom.cs

@fartwhif fartwhif closed this Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mag-nus Mag-nus Mag-nus left review comments

@gmriggs gmriggs gmriggs requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fartwhif @gmriggs @Mag-nus