SRL Archive - OSR Help

Help with standards

Cage

2013-10-05 12:21:44

Hello! I am trying to get my standards correct in my script before I apply for membership.
I am having a little trouble with this procedure as its quite large and contains many different statements.

If someone could please look over it and tell me why it is not correct so I can improve. :)

Thanks!



procedure BankCustom;
var
  TPAA: T2DPointArray;
  CTS, L, I, X, Y, Tries, BankFound: Integer;
  TPA: TPointArray;

begin
    repeat
      PlayerProtection;
      CTS := GetColorToleranceSpeed;
      ColorToleranceSpeed(2);
      SetColorSpeed2Modifiers(0.11, 2.93);
      FindColorsSpiralTolerance(MSCX, MSCY, TPA, 2051676, MSX1, MSY1, MSX2, MSY2, 4);

      TPAA := TPAToATPAEx(TPA, 200, 100);
      DebugATPA(TPAA,'');
      L := High(TPAA);
      ColorToleranceSpeed(CTS);
      for I := 0 to L do
      begin
        MiddleTPAEx(TPAA[i], X, Y);
        MMouse(X, Y, 3, 3);
        if WaitUptext('th', 300) then
        begin
          GetMousePos(X, Y);
          Mouse(X, Y, 0, 0, True);
          Wait(2500 + Random(500));
          BankFound := 1;
          Break;
        end;
      end;

      if not BankScreen then
      begin
        if not WaitUptext('th', 300) then
        begin
          MakeCompass(randomrange(-20, 20));
          blindwalk(Point(3254 + random(1), 3420 + random(1)));
          PlayerProtection;
          inc(Tries)
          WriteLn('Attempt number: ' + IntToStr(Tries));
        end;
     end;

     if Tries > 5 then
     begin
        WriteLn('Failed finding the bank too many times. Exiting Script.');
        Logout;
        TerminateScript;
     end;

  until(BankFound = 1);


   if PinScreen then
   begin
     InPin(BANKPIN);
   end;

  repeat
    if BankScreen then
    begin
      if finddtm(EssDTM, X, Y, MSX1, MSY1, MSX2, MSY2) then
      begin
        QuickDeposit('inv');
        WriteLn('Commencing Banking');
        MMouse(X, Y, 3, 3);
        wait(randomrange(10, 30));
        GetMousePos(X, Y);
        Mouse(X, Y, 0, 0, mouse_right);
        wait(randomrange(10, 30));
        ChooseOption('Withdraw-All')
        wait(randomrange(10, 30));
        CloseBank;
        Writeln('Banking Complete');
        PlayerProtection;
        Exit;
      end else
      begin
        WriteLn('Could not find the Essence DTM. Consider making another one.');
        Logout;
        TerminateScript;
      end;
   end;
 until (False);
end;

riwu

2013-10-05 20:51:29

From: Cage
Hello! I am trying to get my standards correct in my script before I apply for membership.
I am having a little trouble with this procedure as its quite large and contains many different statements.

If someone could please look over it and tell me why it is not correct so I can improve. :)

Thanks!



procedure BankCustom;
var
  TPAA: T2DPointArray;
  CTS, L, I, X, Y, Tries, BankFound: Integer;
  TPA: TPointArray;

begin
    repeat
      PlayerProtection;
      CTS := GetColorToleranceSpeed;
      ColorToleranceSpeed(2);
      SetColorSpeed2Modifiers(0.11, 2.93);
      FindColorsSpiralTolerance(MSCX, MSCY, TPA, 2051676, MSX1, MSY1, MSX2, MSY2, 4);

      TPAA := TPAToATPAEx(TPA, 200, 100);
      DebugATPA(TPAA,'');
      L := High(TPAA);
      ColorToleranceSpeed(CTS);
      for I := 0 to L do
      begin
        MiddleTPAEx(TPAA[i], X, Y);
        MMouse(X, Y, 3, 3);
        if WaitUptext('th', 300) then
        begin
          GetMousePos(X, Y);
          Mouse(X, Y, 0, 0, True);
          Wait(2500 + Random(500));
          BankFound := 1;
          Break;
        end;
      end;

      if not BankScreen then
      begin
        if not WaitUptext('th', 300) then
        begin
          MakeCompass(randomrange(-20, 20));
          blindwalk(Point(3254 + random(1), 3420 + random(1)));
          PlayerProtection;
          inc(Tries)
          WriteLn('Attempt number: ' + IntToStr(Tries));
        end;
     end;

     if Tries > 5 then
     begin
        WriteLn('Failed finding the bank too many times. Exiting Script.');
        Logout;
        TerminateScript;
     end;

  until(BankFound = 1);


   if PinScreen then
   begin
     InPin(BANKPIN);
   end;

  repeat
    if BankScreen then
    begin
      if finddtm(EssDTM, X, Y, MSX1, MSY1, MSX2, MSY2) then
      begin
        QuickDeposit('inv');
        WriteLn('Commencing Banking');
        MMouse(X, Y, 3, 3);
        wait(randomrange(10, 30));
        GetMousePos(X, Y);
        Mouse(X, Y, 0, 0, mouse_right);
        wait(randomrange(10, 30));
        ChooseOption('Withdraw-All')
        wait(randomrange(10, 30));
        CloseBank;
        Writeln('Banking Complete');
        PlayerProtection;
        Exit;
      end else
      begin
        WriteLn('Could not find the Essence DTM. Consider making another one.');
        Logout;
        TerminateScript;
      end;
   end;
 until (False);
end;

Don't leave a white line after your var declaration, otherwise the method looks fragmented.
Your first repeat (along with some of the codes below it) has 2 white tabs, just need 1.
Other than that it's okay, but i advise you not to have any potential infinite loop ;)

Cage

2013-10-05 21:01:51

From: riwu
Don't leave a white line after your var declaration, otherwise the method looks fragmented.
Your first repeat (along with some of the codes below it) has 2 white tabs, just need 1.
Other than that it's okay, but i advise you not to have any potential infinite loop ;)

Thanks for the info. :)

Justin

2013-10-05 22:53:02



procedure BankCustom;
var
  TPAA: T2DPointArray;
  CTS, L, I, X, Y, Tries, BankFound: Integer;
  TPA: TPointArray;

begin
    repeat
      PlayerProtection;
      CTS := GetColorToleranceSpeed;
      ColorToleranceSpeed(2);
      SetColorSpeed2Modifiers(0.11, 2.93);
      FindColorsSpiralTolerance(MSCX, MSCY, TPA, 2051676, MSX1, MSY1, MSX2, MSY2, 4);

      TPAA := TPAToATPAEx(TPA, 200, 100);
      DebugATPA(TPAA,'');
      L := High(TPAA);
      ColorToleranceSpeed(CTS);
      for I := 0 to L do
      begin
        MiddleTPAEx(TPAA[i], X, Y);
        MMouse(X, Y, 3, 3);
        if WaitUptext('th', 300) then
        begin
          GetMousePos(X, Y);
          Mouse(X, Y, 0, 0, True);
          Wait(2500 + Random(500));
          BankFound := 1;
          Break;
        end;
      end;

      if not BankScreen then
      begin
        if not WaitUptext('th', 300) then
        begin
          MakeCompass(randomrange(-20, 20));
          blindwalk(Point(3254 + random(1), 3420 + random(1)));
          PlayerProtection;
          inc(Tries)
          WriteLn('Attempt number: ' + IntToStr(Tries));
        end;
     end;

     if Tries > 5 then
     begin
        WriteLn('Failed finding the bank too many times. Exiting Script.');
        Logout;
        TerminateScript;
     end;

  until(BankFound = 1);


   if PinScreen then
   begin
     InPin(BANKPIN);
   end;

  repeat
    if BankScreen then
    begin
      if finddtm(EssDTM, X, Y, MSX1, MSY1, MSX2, MSY2) then
      begin
        QuickDeposit('inv');
        WriteLn('Commencing Banking');
        MMouse(X, Y, 3, 3);
        wait(randomrange(10, 30));
        GetMousePos(X, Y);
        Mouse(X, Y, 0, 0, mouse_right);
        wait(randomrange(10, 30));
        ChooseOption('Withdraw-All')
        wait(randomrange(10, 30));
        CloseBank;
        Writeln('Banking Complete');
        PlayerProtection;
        Exit;
      end else
      begin
        WriteLn('Could not find the Essence DTM. Consider making another one.');
        Logout;
        TerminateScript;
      end;
   end;
 until (False);
end;

http://www.diffchecker.com/ynff7y0h

Not sure why diffchecker throws the 'ChooseOption' off but in Simba it's correct.

Sjoe

2013-10-06 06:55:24

standards are good :)


   if PinScreen then
   begin
     InPin(BANKPIN); //only one statement for if..then
   end;

Can be shortened since there's only one followup action after if.. then
the begin..end; is redundant.
if there were multiple followup actions then a begin..end; was necessary


   if PinScreen then InPin(BANKPIN); //no need for a new begin/end;

euphemism

2013-10-06 10:11:33

From: Sjoe


   if PinScreen then InPin(BANKPIN); //no need for a new begin/end;

While we are at it, it's generally accepted to be poor, standards-wise, to throw the body of an if statement after the then. Should be:


if (PinScreen) then
  InPin();

Also note the parentheses around the clause of the if statement.

Cage

2013-10-06 11:14:42

Thanks for everybody's input. Ill be sure to implement your suggestions before I submit my application!