HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Human-like mouse movement

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
movementhumanlikemouse

Problem

The code will move the mouse from a start point (xs, ys) to finish point (xe, ye) like a human would.

I'd like some help possibly shortening the code (by optimizing it) and making it more readable.

Also, if you spot any mistakes I could have made while converting the Pascal code to C# code, please let me know!

Original Pascal code:

`procedure _humanWindMouse(xs, ys, xe, ye, gravity, wind, minWait, maxWait, targetArea: extended);
var
veloX, veloY, windX, windY, veloMag, dist, randomDist, lastDist, D: extended;
lastX, lastY, MSP, W, TDist: integer;
T: LongWord;
sqrt2, sqrt3, sqrt5, maxStep, rCnc: extended;
begin
MSP := mouseSpeed;
sqrt2 := sqrt(2);
sqrt3 := sqrt(3);
sqrt5 := sqrt(5);
TDist := distance(round(xs), round(ys), round(xe), round(ye));
t := getSystemTime() + 10000;
repeat
if (getSystemTime() > t) then
break;
dist := hypot(xs - xe, ys - ye);
wind := minE(wind, dist);
if (dist 25) then
D := 25;
if (D = targetArea then
begin
windX := windX / sqrt3 + (random(round(wind) * 2 + 1) - wind) / sqrt5;
windY := windY / sqrt3 + (random(round(wind) * 2 + 1) - wind) / sqrt5;
end
else
begin
windX := windX / sqrt2;
windY := windY / sqrt2;
end;
veloX := veloX + windX;
veloY := veloY + windY;
veloX := veloX + gravity * (xe - xs) / dist;
veloY := veloY + gravity * (ye - ys) / dist;
if (hypot(veloX, veloY) > maxStep) then
begin
randomDist := maxStep / 2.0 + random(round(maxStep) div 2);
veloMag := sqrt(veloX veloX + veloY veloY);
veloX := (veloX / veloMag) * randomDist;
veloY := (veloY / veloMag) * randomDist;
end;
lastX := round(xs);
lastY := round(ys);
xs := xs + veloX;
ys := ys + veloY;
if (lastX <> round(xs)) or (lastY <> round(ys)) then
moveMouse(round(xs), round(ys));
W := (random((round(100 / MSP))) * 6);
if (W round(xs)) or (round(ye) <> round(ys)) then
moveMouse(round(xe), r

Solution

You can get much cleaner code and more readable code if you do 3 things:

  • Use OOP and custom types to better encapsulate values



  • Use more descriptive names



  • Write more helper methods that encapsulate values.



My first big problem is with HumanWindMouse - taking 9 doubles as parameters (and especially with not ideal names) is a recipe for disaster; what happens when someone refactors the code and accidentally switches the order in which they should be passed, or the order in which they actually are passed? Taking that many parameters is asking for trouble, and especially if they're all of the same type. Looking at it, I would say that you have three different custom types available to you - a 2D point, a 2D force, and a range of allowable values. I made these classes

// Define typical math operations
public class Point2D
{
    public double X;
    public double Y;
}

// This isn't necessarily a great equivalence, although
// there is certainly an argument that could be made for it.
// Done here for the sake of simplicity as the two implementations
// end up being identical.  A better implementation of your algorithm
// would treat forces as vectors, as seen at the bottom.
using Force2D = Point2D;

private class Range
{
    public double Minimum;
    public double Maximum;
}


Now our signature looks like this

private async void HumanWindMouse(Point2D start, Point2D end, Force2D push, Range wait, double targetArea)


Still a lot of parameters, but more manageable, and Intellisense (or ReSharper, or any IDE) should be better at catching errors for you than before.

Next I looked through your method and pulled out some common code and some constants (there is still a lot of low-hanging fruit, however. Left as an exercise for the reader).

private static int Distance(Point2D start, Point2D end)
{
    var xDiff = Math.Round(end.X) - Math.Round(start.X);
    var yDiff = Math.Round(end.Y) - Math.Round(start.Y);

    return Convert.ToInt32(Math.Sqrt(xDiff * xDiff + yDiff * yDiff));
}

public static double Hypotenuse(Point2D sides)
{ 
    return Math.Sqrt(Math.Pow(sides.X, 2) + Math.Pow(sides.X, 2));
}

private static T Clamp(T value, T min, T max) where T : IComparable
{
    return value.CompareTo(min)  0
            ? max
            : value;
}

private double CalculateWind(double original, double current)
{
    return current / sqrt3 + (rnd.Next((int)(Math.Round(original) * 2 + 1)) - original) / sqrt5;
}

private Force2D CalculateWind(double dist, double targetArea, Force2D wind, Force2D push)
{
    return dist >= targetArea
        ? new Force2D { x = CalculateWind(wind.X, push.X), y = CalculateWind(wind.Y, push.X) }
        : new Force2D { x = wind.X / sqrt2, y = wind.Y / sqrt2 };
}

private static readonly double sqrt2 = Math.Sqrt(2);
private static readonly double sqrt3 = Math.Sqrt(3);
private static readonly double sqrt5 = Math.Sqrt(5);


Some of these values can be computed once and then forgotten about, and the functions need to be updated to use the new values. Also, you never use your Distance function to get the double distance, just an integer distance, so I moved all of that logic into the function. You also then reference that distance (double to an int) later by... casting it back into a double and then rounding it? That won't do anything meaningful, and it makes me wonder what the intended behavior is.

You should always use curly braces in your loops and conditionals - it'll save you a lot of trouble later. It's even better if you can use a clean, readable ternary (or a function) to replace conditionals though.

double maxStep = Clamp(d, double.MinValue, Math.Round(dist));
...

// assumes wind and velocity are Force2D that replaced windX, windY, veloX, veloY
wind = CalculateWind(dist, targetArea, wind, push);

// assumes appropriate math operations exist
velocity += wind + push.Y * (end - start) / dist;


etc, etc.

My solution still isn't perfectly clean - the force and point class have no meaningful difference between them (besides semantics). Overall, however, by moving more things into helper functions you can really clean up the code. That and renaming things - I was hesitant to move as many things into helper functions or structs than I would have liked.

Final note, in passing. Right now you're taking gravity and wind and doing some kind of weird stuff - they both seem to apply to both directions, as if there were an equal force horizontally and vertically from the two of them. A force is much more accurately represented as an angle and a magnitude. Then you can just use trig to figure out its contribution in a given direction, i.e.

velocity.X += Math.cos(gravity.X) * gravity.Magnitude


or something like that.

Code Snippets

// Define typical math operations
public class Point2D
{
    public double X;
    public double Y;
}

// This isn't necessarily a great equivalence, although
// there is certainly an argument that could be made for it.
// Done here for the sake of simplicity as the two implementations
// end up being identical.  A better implementation of your algorithm
// would treat forces as vectors, as seen at the bottom.
using Force2D = Point2D;

private class Range
{
    public double Minimum;
    public double Maximum;
}
private async void HumanWindMouse(Point2D start, Point2D end, Force2D push, Range wait, double targetArea)
private static int Distance(Point2D start, Point2D end)
{
    var xDiff = Math.Round(end.X) - Math.Round(start.X);
    var yDiff = Math.Round(end.Y) - Math.Round(start.Y);

    return Convert.ToInt32(Math.Sqrt(xDiff * xDiff + yDiff * yDiff));
}

public static double Hypotenuse(Point2D sides)
{ 
    return Math.Sqrt(Math.Pow(sides.X, 2) + Math.Pow(sides.X, 2));
}

private static T Clamp<T>(T value, T min, T max) where T : IComparable
{
    return value.CompareTo(min) < 0
        ? min
        : value.CompareTo(max) > 0
            ? max
            : value;
}

private double CalculateWind(double original, double current)
{
    return current / sqrt3 + (rnd.Next((int)(Math.Round(original) * 2 + 1)) - original) / sqrt5;
}

private Force2D CalculateWind(double dist, double targetArea, Force2D wind, Force2D push)
{
    return dist >= targetArea
        ? new Force2D { x = CalculateWind(wind.X, push.X), y = CalculateWind(wind.Y, push.X) }
        : new Force2D { x = wind.X / sqrt2, y = wind.Y / sqrt2 };
}

private static readonly double sqrt2 = Math.Sqrt(2);
private static readonly double sqrt3 = Math.Sqrt(3);
private static readonly double sqrt5 = Math.Sqrt(5);
double maxStep = Clamp(d, double.MinValue, Math.Round(dist));
...

// assumes wind and velocity are Force2D that replaced windX, windY, veloX, veloY
wind = CalculateWind(dist, targetArea, wind, push);

// assumes appropriate math operations exist
velocity += wind + push.Y * (end - start) / dist;
velocity.X += Math.cos(gravity.X) * gravity.Magnitude

Context

StackExchange Code Review Q#138654, answer score: 10

Revisions (0)

No revisions yet.