12
\$\begingroup\$

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 < 1) then
      dist := 1;
    D := (round((round(TDist) * 0.3)) / 7);
    if (D > 25) then
      D := 25;
    if (D < 5) then
      D := 5;
    rCnc := random(6);
    if (rCnc = 1) then
      D := randomRange(2, 3);
    if (D <= round(dist)) then
      maxStep := D
    else
      maxStep := round(dist);
    if dist >= 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 < 5) then
      W := 5;
    W := round(W * 0.9);
    wait(W);
    lastdist := dist;
  until (hypot(xs - xe, ys - ye) < 1) if (round(xe) <> round(xs)) or (round(ye) <> round(ys)) then
    moveMouse(round(xe), round(ye));
  mouseSpeed := MSP;
end;

C# code:

private static double Distance(double x1, double y1, double x2, double y2)
{
    return Math.Sqrt(Math.Pow(x2 - x1, 2) + Math.Pow(y2 - y1, 2));
}

public static double Hypot(double x, double y)
{
    return Math.Sqrt(Math.Pow(x, 2) + Math.Pow(y, 2));
}

private async void HumanWindMouse(double xs, double ys, double xe, double ye, double gravity, double wind,
    double minWait, double maxWait, double targetArea)
{
    double veloX = 0,
        veloY = 0,
        windX = 0,
        windY = 0;

    var msp = _mouseSpeed;
    var sqrt2 = Math.Sqrt(2);
    var sqrt3 = Math.Sqrt(3);
    var sqrt5 = Math.Sqrt(5);

    var tDist = (int)Distance(Math.Round(xs), Math.Round(ys), Math.Round(xe), Math.Round(ye));
    var t = (uint)(Environment.TickCount + 10000);

    do
    {
        if (Environment.TickCount > t)
            break;

        var dist = Hypot(xs - xe, ys - ye);
        wind = Math.Min(wind, dist);

        if (dist < 1)
            dist = 1;

        var d = (Math.Round(Math.Round((double)tDist) * 0.3) / 7);

        if (d > 25)
            d = 25;

        if (d < 5)
            d = 5;

        double rCnc = rnd.Next(6);

        if (rCnc == 1)
            d = 2;

        double maxStep;

        if (d <= Math.Round(dist))
            maxStep = d;
        else
            maxStep = Math.Round(dist);

        if (dist >= targetArea)
        {
            windX = windX / sqrt3 + (rnd.Next((int)(Math.Round(wind) * 2 + 1)) - wind) / sqrt5;
            windY = windY / sqrt3 + (rnd.Next((int)(Math.Round(wind) * 2 + 1)) - wind) / sqrt5;
        }
        else
        {
            windX = windX / sqrt2;
            windY = windY / sqrt2;
        }

        veloX = veloX + windX;
        veloY = veloY + windY;
        veloX = veloX + gravity * (xe - xs) / dist;
        veloY = veloY + gravity * (ye - ys) / dist;

        if (Hypot(veloX, veloY) > maxStep)
        {
            var randomDist = maxStep / 2.0 + rnd.Next((int)(Math.Round(maxStep) / 2));
            var veloMag = Math.Sqrt(veloX * veloX + veloY * veloY);
            veloX = (veloX / veloMag) * randomDist;
            veloY = (veloY / veloMag) * randomDist;
        }

        var lastX = (int)Math.Round(xs);
        var lastY = (int)Math.Round(ys);
        xs = xs + veloX;
        ys = ys + veloY;

        if (lastX != Math.Round(xs) || (lastY != Math.Round(ys)))
            SetCursorPosition(new PointF((float)Math.Round(xs), (float)Math.Round(ys)));

        var w = (rnd.Next((int)(Math.Round((double)(100 / msp)))) * 6);

        if (w < 5)
            w = 5;

        w = (int)Math.Round(w * 0.9);
        await Task.Delay(w);
    } while (!(Hypot(xs - xe, ys - ye) < 1));

    if (Math.Round(xe) != Math.Round(xs) || (Math.Round(ye) != Math.Round(ys)))
        SetCursorPosition(new PointF((float)Math.Round(xe), (float)Math.Round(ye)));

    _mouseSpeed = msp;
}

Showcase #1

Showcase #1

Showcase #2

Showcase #2

Showcase #3

Showcase #3

\$\endgroup\$
10
\$\begingroup\$

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

  1. Use OOP and custom types to better encapsulate values
  2. Use more descriptive names
  3. 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>(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);

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ I would recommend using System.Windows.Point and System.Windows.Vector which already support some common math operations out of the box, instead of creating custom implementations and basically reinventing the wheel. \$\endgroup\$ – Nikita B Aug 15 '16 at 8:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.