procedure TheFormOnClose(Sender : TObject; var Action : TCloseAction);
begin
if (not(MainForm.ModalResult = 1)) then
EndTheForm := True;
end;
Could be:
procedure TheFormOnClose(Sender : TObject; var Action : TCloseAction);
begin
EndTheForm := (not(MainForm.ModalResult = 1));
end;
In GetScriptData:
'versiondebug':
begin
ScriptLatestVersionL.caption := TheVersion;
if (Version = TheVersion) then
begin
writeln('You have the Latest Version: ' + Version);
ScriptDescriptionL.Caption := 'Script is up to date!';
ScriptDescriptionL.Font.Color := clGreen;
Exit;
end else
Writeln('You are OUTDATED, click the update button to get it.');
ScriptDescriptionL.Caption := 'Script is Outdated!';
ScriptDescriptionL.Font.Color := clRed;
end;
Should have a begin/end after the else?
'versiondebug':
begin
ScriptLatestVersionL.caption := TheVersion;
if (Version = TheVersion) then
begin
writeln('You have the Latest Version: ' + Version);
ScriptDescriptionL.Caption := 'Script is up to date!';
ScriptDescriptionL.Font.Color := clGreen;
Exit;
end else
begin //HERE
Writeln('You are OUTDATED, click the update button to get it.');
ScriptDescriptionL.Caption := 'Script is Outdated!';
ScriptDescriptionL.Font.Color := clRed;
end; //HERE
end;
Could be wrong, but I'm pretty sure. It would also make sense based on how the standards were.
In Write:
case LowerCase(WhatToWrite) of
'help':
begin
ShowMessage('Help:' + Chr(13) +
'Declare your players before trying' + Chr(13) +
'to press the start script button!' + Chr(13) +
'' + Chr(13) +
'Hope this helped.' + Chr(13));
end;
end;
Could just be:
if LowerCase(WhatToWrite) = 'help' then
begin
ShowMessage('Help:' + Chr(13) +
'Declare your players before trying' + Chr(13) +
'to press the start script button!' + Chr(13) +
'' + Chr(13) +
'Hope this helped.' + Chr(13));
end;
No case of statements when there's only 1 case.
Personally, I think with do's with things like:
with Lines do
begin
Add('Progress Reports');
end;
Looks more ugly. It would look better to just do:
Lines.Add('Progress Reports');
Doesn't look bad to me at least.
In StatsCheckOutput:
begin
sa := Explode(#10, s);
for i := 0 to High(sa) do
if (Trim(sa[i]) <> '') then
if StartsWith('ERROR(1)', sa[i]) then
begin
WriteLn('STATS: "' + sa[i] + '"');
Stats_UserID := '';
Stats_Password := '';
end
else if StartsWith('ERROR(2)', sa[i]) then
begin
WriteLn('STATS: "' + sa[i] + '"');
Stats_ScriptID := '';
end
else
WriteLn('STATS: "' + sa[i] + '"');
end;
That could be cleaned up with a case of statement:
begin
sa := Explode(#10, s);
for i := 0 to High(sa) do
begin
if not(Trim(sa[i]) <> '') then continue;
WriteLn('STATS: "' + sa[i] + '"');
if StartsWith('ERROR(1)', sa[i]) then
begin
Stats_UserID := '';
Stats_Password := '';
end else
if StartsWith('ERROR(2)', sa[i]) then
Stats_ScriptID := '' else
WriteLn('STATS: "' + sa[i] + '"');
end;
end;
Not that much shorter, but overall looks cleaner to me.
In StatsSendReport:
else
begin
AddPostVariable (Client, 'sID', '-1');
end;
Should just be:
else
AddPostVariable (Client, 'sID', '-1');
ExpGained:
procedure ExpGained; //Coh3n
begin
SetArrayLength(XpGain, 5)
XpGain := [25, 37.5, 67.5, 175, 250];
case Lowercase(Players[CurrentPlayer].Strings[0])of
'tree' : g := 0;
'oak' : g := 1;
'willow' : g := 2;
'yew' : g := 3;
'magic' : g := 4;
end;
end;
Could be:
procedure ExpGained; //Coh3n
var S: TStringArray;
i: Integer;
begin
SetArrayLength(XpGain, 5)
XpGain := [25, 37.5, 67.5, 175, 250];
S := ['tree', 'oak', 'willow', 'yew', 'magic'];
for i := 0 to 4 do
if(S[i] = Lowercase(Players[CurrentPlayer].Strings[0]) then
g := i;
end;
Yay 1 line shorter :p
*tries hard to shorten Erik's code*
Hmm..
5: begin
PickUpMouse;
Wait(500 + Random(500));
end;
6: begin
GetClientDimensions(X, Y);
MMouse(Random(X) + 1, Random(Y) + 1, 0, 0);
Wait(500 + random(500));
end;
7: begin
GameTab(RandomRange(tab_Combat, tab_Notes));
Wait(500 + Random(500));
GameTab(tab_Inv);
end;
8: begin
SleepAndMoveMouse(5000 + Random(1000));
end;
Could be:
5: PickUpMouse;
6: begin
GetClientDimensions(X, Y);
MMouse(Random(X) + 1, Random(Y) + 1, 0, 0);
end;
5, 6: wait(500 + Random(500));
7: begin
GameTab(RandomRange(tab_Combat, tab_Notes));
Wait(500 + Random(500));
GameTab(tab_Inv);
end;
8: SleepAndMoveMouse(5000 + Random(1000));
Unless my logic fails :p
Here:
procedure AntiRandoms;
begin
FindNormalRandoms;
end;
Maybe you should check for an InFight, just to be safe?
In AtYewTree:
Case Lowercase(WhichDirection) of
'bottom': If FindSymbolDirection(P.x, P.y, 'tree', 'top') then
Result := True;
'top': If FindSymbolDirection(P.x, P.y, 'tree', 'bottom') then
Result := True;
end;
Could be:
Case Lowercase(WhichDirection) of
'bottom': Result := FindSymbolDirection(P.x, P.y, 'tree', 'top') then
'top': Result := FindSymbolDirection(P.x, P.y, 'tree', 'bottom') then
end;
In SwitchTree:
if FindSymbolDirection(P.x, P.y, 'tree', 'bottom') then
begin
mouse(P.x, P.y - 9, 3, 3, true);
Result := true;
end;
if Not (Result) then
begin
if FindSymbolDirection(P.x, P.y, 'tree', 'top') then
begin
mouse(P.x, P.y + 9, 3, 3, true);
end;
end;
Could be:
Result := FindSymbolDirection(P.x, P.y, 'tree', 'bottom');
if(Result)then
mouse(P.x, P.y - 9, 3, 3, true) else
if FindSymbolDirection(P.x, P.y, 'tree', 'top') then
mouse(P.x, P.y + 9, 3, 3, true);
In WalkToDWillows:
if FindDTMRotated(BankDTM, P.x, P.y , 604, 63, 647, 99, -Pi, Pi, 0.3, WalkAngle) then
begin
if MFNFEx(P.x+20, P.y -33,5,5) then
Result := true;
end else Writeln('Couldnt use BankDTM');
Could be:
if FindDTMRotated(BankDTM, P.x, P.y , 604, 63, 647, 99, -Pi, Pi, 0.3, WalkAngle) then
Result := MFNFEx(P.x+20, P.y -33,5,5) else
Writeln('Couldnt use BankDTM');
The same could be done on the next 6 lines below that.
In WalkToDWillowsBank:
if FindDTMRotated(BankDTM, P.x, P.y, MMX1, MMY1, MMX2, MMY2, -Pi, Pi, 0.3, WalkAngle) then
begin
if MFNFEx(P.x-8, P.y,2,6) then
Result := true;
end else Writeln('Couldnt find BankDTM');
If Not (Result) then
if FindSymbol(P.x, P.y, 'bank') then
begin
if MFNFEx(P.x, P.y,2,2) then
Result := true;
That's really rough on the eyes
if FindDTMRotated(BankDTM, P.x, P.y, MMX1, MMY1, MMX2, MMY2, -Pi, Pi, 0.3, WalkAngle) then
Result := MFNFEx(P.x-8, P.y,2,6) else
Writeln('Couldnt find BankDTM');
If Not (Result) and FindSymbol(P.x, P.y, 'bank') then
if MFNFEx(P.x, P.y,2,2) then
Result := true;
Then below that you have:
If Not (Result) then
if FindSymbol(P.x, P.y, 'bank') then
begin
if MFNFEx(P.x, P.y,2,2) then
Result := true;
end else Writeln('Couldnt find Bank Symbol');
if FindSymbol(P.x, P.y, 'fish') then
begin
If Not (Result) then
if MFNFEx(P.x, P.y + 20, 6, 6) then
if FindDTMRotated(BankDTM, P.x, P.y, MMX1, MMY1, MMX2, MMY2, -Pi, Pi, 0.3, WalkAngle) then
begin
if MFNFEx(P.x-7, P.y,3,6) then
Result := true;
end else Writeln('Couldnt find BankDTM');
end;
Change to:
If Not (Result) and FindSymbol(P.x, P.y, 'bank') and MFNFEx(P.x, P.y,2,2) then
Result := true else
Writeln('Couldnt find Bank Symbol');
if FindSymbol(P.x, P.y, 'fish') then Not (Result) and MFNFEx(P.x, P.y + 20, 6, 6) and
FindDTMRotated(BankDTM, P.x, P.y, MMX1, MMY1, MMX2, MMY2, -Pi, Pi, 0.3, WalkAngle) and
MFNFEx(P.x-7, P.y,3,6) then
Result := true else
Writeln('Couldnt find BankDTM');
Much shorter.
Use same concepts for other walking procedures below that.
In WalkToEdgeYews:
if not(Length(DirtColourTPA) > 1)then
Exit
else
DirtColourATPA:= SplitTPAEx(DirtColourTPA, 4, 4);
SortATPASize(DirtColourATPA, True);
P := MiddleTPA(DirtColourATPA[0]);
//...etc.
Shouldn't it be an else begin/end?
In GEYewWalk:
Case Random(2) Of
0: begin
if walktotile(point(3168, 3489), 2, 8) then
Writeln('Made it to the first step ToYews');
if walktotile(point(3181, 3490), 2, 8) then
Writeln('Made it to the second step ToYews');
if walktotile(point(3195, 3494), 2, 8) then
Writeln('Nearly there ToYews');
if walktotile(point(3207, 3503), 2, 8) then
Writeln('Woop we made it ToYews');
end;
1: begin
if walktotile(point(3169, 3489), 2, 8) then
Writeln('Made it to the first step ToYews');
if walktotile(point(3184, 3490), 2, 8) then
Writeln('Made it to the second step ToYews');
if walktotile(point(3196, 3497), 2, 8) then
Writeln('Nearly there ToYews');
if walktotile(point(3206, 3502), 2, 8) then
Writeln('Woop we made it ToYews');
end;
2: begin
if walktotile(point(3169, 3491), 2, 8) then
Writeln('Made it to the first step ToYews');
if walktotile(point(3187, 3489), 2, 8) then
Writeln('Made it to the second step ToYews');
if walktotile(point(3200, 3502), 2, 8) then
Writeln('Nearly there ToYews');
if walktotile(point(3207, 3504), 2, 8) then
Writeln('Woop we made it ToYews');
end;
end;
That random(2) should be a random(3) if you want it to ever do the last path. Random(2) would only pick 0 or 1, never 2.
Same for GEYewBank.
I really think you should take out all of those writeln's after the if(WalkToTile)'s. No point really unless you are going to put in a fail safe.
In WaitInvCount:
Result := false;
T := GetSystemTime + Maxtime;
while GetSystemTime < T do
begin
wait(10);
if InvCount = Count then
begin
Result := true;
break;
end;
end;
Change to:
T := GetSystemTime + Maxtime;
while GetSystemTime < T do
begin
wait(10);
Result := InvCount = Count;
if(Result)then break;
end;
In LoadTree, take out all of the
Name :=
In each different case, and just put at the top of the function:
Result.Name := LowerCase(which);
On line 2231:
wait(9000);
Randomness is probably necessary.
Sorry, I started to get extremely bored so I just slightly skimmed over the last part of the script but didn't see anything significant.
Try to get rid of some of the redundancies in your walking by making a function. You repeat a lot of the exact same stuff several times in walking procedures, so making that into 1 function then calling it could really cut down on code + make the script more readable (and easier to add new locations).
Well I'm off to bed. Hope you didn't get offended by all that I pointed out, it's a nice script :) Some of the stuff I pointed out was just very small code shortenings anyway, although some did highly improve readability IMO. You had some messy standards in some places that you may want to take a look at.
Cheers,
~JAD
like the pic showing it just stand there with out moving
