In AGetVersion:
if Ver = Version then result := true;
Could save length, and true lines (since code after an if/then should be on a new line):
Result := Ver = version;
Also, it's better to do that because then the function will always result something.
while not FindThis do
begin
for i:= 1 to tol do
begin
if FindColorsSpiralTolerance(MSCX, MSCY, T, color, MSX1, MSY1, MSX2, MSY2, tol) then
break;
end;
k := length(t);
FindThis := k > 0; //thanks to noidea :)
if timefrommark(time) > 5000 then break;
end;
Change to:
while not FindThis and TimeFromMark(time) < 5000+random(1500) do
begin
for i := 1 to tol do
if FindColorsSpiralTolerance(MSCX, MSCY, T, color, MSX1, MSY1, MSX2, MSY2, i) then
break;
end;
k := length(t);
FindThis := k > 0; //thanks to noidea :)
end;
Added randomness to your marktime + put in the while statement. Took out unnecessary begin/end in for loop. Most importantly, your tolerance incrementer was using tol instead of I in the FindColorsSpiralTolerance. I'm assuming you wanted to keep incrementing the tolerance until found, right? In that case, you would want to use i as the tolerance as I changed it to above.
So now we have this:
result:= true;
d:= 0;
if not loggedin then loginplayer;
ColorToleranceSpeed(3);
time := 0;
marktime(time);
while not FindThis and TimeFromMark(time) < 5000+random(1500) do
begin
for i := 1 to tol do
if FindColorsSpiralTolerance(MSCX, MSCY, T, color, MSX1, MSY1, MSX2, MSY2, i) then
break;
end;
k := length(t);
FindThis := k > 0; //thanks to noidea :)
end;
ColorToleranceSpeed(1);
if Length(T) = 0 then
begin
A_Debug('color not found');
result:= false;
exit;
end;
Which could get rid of a few lines by doing a few things with the Result:
d:= 0;
if not loggedin then loginplayer;
ColorToleranceSpeed(3);
time := 0;
marktime(time);
while not FindThis and TimeFromMark(time) < 5000+random(1500) do
begin
for i := 1 to tol do
if FindColorsSpiralTolerance(MSCX, MSCY, T, color, MSX1, MSY1, MSX2, MSY2, i) then
break;
end;
k := length(t);
FindThis := k > 0; //thanks to noidea :)
end;
ColorToleranceSpeed(1);
Result := not Length(T) = 0;
if not Result then
begin
A_Debug('color not found');
exit;
end;
Only 1 line, but looks cleaner to me.
Not sure, but at this part:
if not result then
begin
A_Debug('Something has gone wrong');
players[currentplayer].active := false;
end;
Wouldn't you just want to Logout when things go wrong, have if not loggedin then Exit;'s at the beginning of (almost) all procedures/functions, and then at the end of the main loop have if not loggedin then NextPlayer(false)? That's the way I do it at least.
In Stats Check Output:
for i := 0 to High(sa) do
if (Trim(sa[i]) <> '') then
if StartsWith('ERROR(1)', sa[i]) then
begin
WriteLn('STATS: "' + sa[i] + '"');
end
else if StartsWith('ERROR(2)', sa[i]) then
begin
WriteLn('STATS: "' + sa[i] + '"');
Stats_ScriptID := '5';
end
else
WriteLn('STATS: "' + sa[i] + '"');
That looks messy:
for i := 0 to High(sa) do
if (Trim(sa[i]) <> '') and StartsWith('ERROR(1)', sa[i]) then
WriteLn('STATS: "' + sa[i] + '"')
else if StartsWith('ERROR(2)', sa[i]) then
begin
WriteLn('STATS: "' + sa[i] + '"');
Stats_ScriptID := '5';
end else
WriteLn('STATS: "' + sa[i] + '"');
Saves some lines and looks cleaner.
On line 221:
var
Stats_GlobalVars: array[1..24] of Integer;
Shouldn't that be up top with the other global variables? Would be easier to keep track of everything/change stuff, and is standard.
On line 278-280 get rid of unnecessary begin/end.
In NextPlayerP:
NextPlayer(Players[(CurrentPlayer + 1)].Active);
I don't understand why you do that? NextPlayer switches players, leaving the current players .active to whatever you put in there. Why would you want to set it to what the next players .active is? It would always be true anyways.
In LoadTrees:
case lowercase(players[currentplayer].strings[0]) of
'normal': with TA do
begin
name := 'normal';
uptext := 'ree';
color := 4094573;
tol := 4;
end;
'oak': with TA do
begin
name := 'oak';
uptext := 'Oak';
color := 5739656;
tol := 4;
end;
'willow': with TA do
begin
name := 'willow';
uptext := 'illow';
color := 4282446;
tol := 4;
end;
'maple': with TA do
begin
name := 'maple';
uptext := 'aple';
color := 3956883;
tol := 4;
end;
'teak': with TA do
begin
name := 'teak';
uptext := 'eak';
color := 5410940;
tol := 4;
end;
'mahogany': with TA do
begin
name := 'mahogany';
uptext := 'ahogany';
color := 1525307;
tol := 4;
end;
'yew': with TA do
begin
name := 'yew';
uptext := 'ew';
color := 1259054;
tol := 4;
end;
'magic':with TA do
begin
name := 'magic';
uptext := 'agic';
color := 12836577;
tol := 4;
end;
end;
Could be shortened a fair amount:
TA.name := lowercase(players[currentplayer].strings[0]);
TA.tol := 4;
case lowercase(players[currentplayer].strings[0]) of
'normal': with TA do
begin
uptext := 'ree';
color := 4094573;
end;
'oak': with TA do
begin
uptext := 'Oak';
color := 5739656;
end;
'willow': with TA do
begin
uptext := 'illow';
color := 4282446;
end;
'maple': with TA do
begin
uptext := 'aple';
color := 3956883;
end;
'teak': with TA do
begin
uptext := 'eak';
color := 5410940;
end;
'mahogany': with TA do
begin
uptext := 'ahogany';
color := 1525307;
end;
'yew': with TA do
begin
uptext := 'ew';
color := 1259054;
end;
'magic':with TA do
begin
uptext := 'agic';
color := 12836577;
end;
end;
function findax:boolean;
var
i, a: integer;
tpa: TPointArray;
c: TIntegerArray;
begin
colortolerancespeed(3);
c := [5064763, 593214, 5261885, 869985];
for i:= 0 to high(c) do
if findcolorstolerance(tpa, c[i], mscx - 40, mscy - 40, mscx + 40, mscy + 40, 5) then
inc(a);
result := (a > 0);
colortolerancespeed(1);
end;
Could just be:
function findax:boolean;
var
i: integer;
tpa: TPointArray;
c: TIntegerArray;
begin
colortolerancespeed(3);
c := [5064763, 593214, 5261885, 869985];
for i:= 0 to high(c) do
if findcolorstolerance(tpa, c[i], mscx - 40, mscy - 40, mscx + 40, mscy + 40, 5) then
Result := true;
colortolerancespeed(1);
end;
No need for an incrementing variable.
In ChopTree:
while findax do
begin
antiban;
if invfull then break;
end;
Possible infinite loop. Add a (marktime?) Fail safe.
Also in ChopTree:
if invfull then break;
until invfull or not result;
You can delete that first line there, because it will already break if inv is full because of the until.
In DropLogs:
if not loggedin then exit;
if not invfull then exit;
Could just be:
if not loggedin or not invfull then exit;
In AwkSetup:
if l then
if not loggedin then loginplayer;
Could just be:
if l then and not loggedin then loginplayer;
Very nice Awkward!
Hopefully you were not offended by how much I pointed out, I was really bored.. Most of that stuff was just code shortenings/nothing that matters to anybody else except me :p There were just a possibly problematic things such as the switching players (which maybe you were doing right and I misread), the tolerance changer, an infinite loop or 2, and maybe a couple other small things.
~JAD