Using knownledge of the SRL includes to remove duplicate code and improve logic

[Thomas]

function banking(preset: integer): Boolean;
begin
if (not isLoggedIn()) then
exit(False);
FailTimer.start();
begin
repeat
if bankscreen.open(BANK_GE) then
break();
wait(gaussRangeInt(100, 500));
minimap.clickCompass();
if (FailTimer.getTime() > TIMEOUT) or (not isLoggedIn()) then
begin
writeLn('Could not find banker, shutting down');
players[currentPlayer].logout();
terminateScript();
end;
until bankScreen.isOpen(5000) or pinscreen.isOpen();
if pinScreen.isOpen() then
pinScreen.enter(players[currentPlayer].bankPin);
repeat
if bankScreen.clickButton(button) then
break;
if (timer.getTime > timeOut) or (not isLoggedIn) then
exit;
until (not bankScreen.isOpen);
end;
result := true;
end;

This might seem like a good code, but has allot of duplicate code. You might not notice this because most of the checks are 'behind the scenes' in the SRL includes.

Let's start with the 'bankscreen.open(BANK_GE)' boolean.

function TRSBankScreen.open(bankType: integer): boolean;
begin
if (bankScreen.isOpen()) or (pinScreen.isOpen()) then
exit(true);

case bankType of
BANK_NPC_BLUE..BANK_NPC_DRAYNOR: result := self.__openNPC(bankType);
BANK_BOOTH: result := self.__openBankBooth();
BANK_GE: Result := Self.__openGE();
BANK_CHEST_SW..BANK_CHEST_BARB_OUTPOST: result := self.__openChest(bankType);
end;
end;
As you can see the result of the 'bankscreen.open(BANK_GE)' boolean is true once the bankScreen or pinScreen is open.
So repeating it until that result is true is a waste of code and makes the script check for bankScreen.isOpen (or pinScreen.isOpen) more then needed.

Then one might try to repeat the bankScreen.clickButton(BANK_BUTTON_1/2) until (not bankScreen.isOpen). Let's have a look at those functions in the SRL includes!
function TRSBankScreen.clickButton(button: integer; useKey: boolean = true): boolean;
const
BUTTON_OFFSET: TPoint = [-662, -25];
var
b: TBox;
timeOut: LongWord;
begin
if (not self.isOpen()) then
begin
print('bankScreen.clickButton(): Unable to click button since bankscreen isn''t open', TDebug.ERROR);
exit(false);
end;

b := gridBox(button, 6, 1, 30, 21, 39, 0,
point(self.x2 + BUTTON_OFFSET.x,
self.y2 + BUTTON_OFFSET.y));

// try ESC key first
if (useKey) and ((button = BANK_BUTTON_PRESET_1) or (button = BANK_BUTTON_PRESET_2)) then
begin
timeOut := (getSystemTime() + randomRange(1500, 2000));
repeat
multiType(toStr(button - 2), randomRange(30, 50), 2, false);
wait(100 + random(50));
result := (not self.isOpen());
until((getSystemTime() > timeOut) or result);

if (result) then
exit();
end;

mouseBox(b, MOUSE_MOVE);

if (self.isOpen()) then
begin
fastClick(MOUSE_LEFT);
wait(50 + random(50));
end else
exit(true);

wait(100 + random(150));
result := true;
end;
We can see that the result will become true is the bankscreen (self) is not open:
result := (not self.isOpen());
Now we know that, it's unneeded to put a check wether the bankscreen is actually closed, since it'll already return true if it is close.

Now let's try to remove the duplicate login checks and failtimers.
By putting logincheck at the begin of a loop or repeat you don't have to call it before the loop itself.
Then by combining both repeats you can even eliminate the third check.

Now for the pinscreen.
function TRSPinScreen.enter(pin: string): boolean;
var
i, tries: integer;
b: TBox;
begin
result := false;
print('pinScreen.enter()', TDebug.HEADER);

if (not self.isPinVaild(pin)) then
begin
print('Cannot enter pin because the pin isnt vaild!', TDebug.ERROR);
print('pinScreen.enter(): result = false', TDebug.FOOTER);
exit();
end;

if (not self.isOpen()) then
begin
print('Cannot enter pin because the pinscreen is not open', TDebug.ERROR);
print('pinScreen.enter(): result = false', TDebug.FOOTER);
exit();
end;

repeat
inc(tries);

for i := 1 to 4 do
begin
if (self.isOpen()) then
begin
if (self.__findNumber(pin[i], b)) then
begin
print('Succesfully clicked number ' + pin[i]);
mouseBox(b, MOUSE_LEFT);

wait(350 + random(400));
end else
exit(self.__exit('Failed to find number ' + pin[i], TDebug.ERROR, false));
end else
exit(self.__exit('Pinscreen isn''t open anymore', TDebug.ERROR, false));
end;

// now we've entered
print('Waiting for the outcome of the pin...');
wait(250 + random(200));

// wait upto 4 seconds for the pinscreen not to be open
if (self.waitNotOpen(4000)) then
begin
if (not conversationBox.isOpen()) then // no convo box? = entered pin succesfully
exit(self.__exit('Succesfully entered pin', TDebug.DEBUG, true));

if (not conversationBox.findChat(['Please try again', 'ease try aga'])) then // didnt find bad
exit(self.__exit('Conversation box is open, but didn''t find the text to retry.', TDebug.ERROR, false))
else
conversationBox.continue(true, true); // if we did found the text, lets skip it.
end else
exit(self.__exit('Pinscreen is still open, something went wrong', TDebug.ERROR, false)); // else pinscreen is still open, something messed up

print('Going to try and enter our pin again..');
until (tries >= 2);

exit(self.__exit('Failed to enter our pin after two tries', TDebug.ERROR, false));
end;

Notice this part:
if (not self.isOpen()) then
begin
print('Cannot enter pin because the pinscreen is not open', TDebug.ERROR);
print('pinScreen.enter(): result = false', TDebug.FOOTER);
exit();
end;

This means it would just exit the 'function TRSPinScreen.enter(pin: string): boolean' if the pinscreen is not open. This making it unneeded to check for it in the first place before entering it.

Final result:
function banking(preset: integer): Boolean;
begin
FailTimer.start();
repeat
if (FailTimer.getTime > TIMEOUT) or (not isLoggedIn()) then
exit();
if (not bankscreen.open(BANK_GE)) then
minimap.clickCompass();
pinScreen.enter(players[currentPlayer].bankPin);
until bankScreen.clickButton(preset);
result := true;
end;

I will try to find another usefull snippet/example later. As this guide is just a work in progress, feel free to feedback/correct me.


Regards,
Thomas