JAD
2009-07-27 20:44:58
From: marpis
I'd have to say No.
Not advanced enough. But you seem to be a fast learner, keep on writing an you'll be a member in a couple of months :)
It's not good to call FindObjTPA multiple times in a single function, because it makes the script mess up easily.
This function is not perfect either, but here's some ideas to improve your TreeTime procedure.
I also suggest you to make it a function that returns boolean.
PS. Read about TPAs!
procedure TreeTime;
var
Failed: integer;
begin
if (not LoggedIn) then Exit;
for i := 0 to 3 do
repeat
if FindObjTPA(x, y, TreeColors[i], 30, 2, 15, 25, 10, TreeTxT) then
begin
GetMousePos(x, y); // FindObjTPA already hovers the tree
case Random(2) of
0: begin
Mouse(x, y, 0, 0, False);
WaitOption('hop', 500);
end;
1:Mouse(x, y, 0, 0, True);
end;
Wait(TreeWait + Random(200));
AntiBan;
end else
begin
Inc(Failed);
if (Failed mod 2 = 0) then Break; // Try to find the tree 2 times with
MakeCompass(Random(360)); // each color
wait(300+random(50));
end;
until(Invfull);
if (Failed > 7) then
begin
writeln('Tree Finding Failed :(');
NextPlayer(false);
end;
end;
Im sorry, it's a No.
1) Not because of Reflection, but because you don't have any failsafes!
Your fishing procedure will be messed up if the fishing spot isn't there yet.
2) If one step of your walking fails, it just keeps on going which leads to no good.
3) You should use parameters on your functions, like this
Code:
MyFindFishingspot(var msx, msy: integer): boolean;
so you wouldn't have to call FindNPC a million times in your fishing procedures.
It's slow and inefficient.
4)
Code:
if(TileOnMS(Fp.tile, 10))then
begin
result := true;
end else
begin
result := false;
end;
can be shortened to
Code:
result := TileOnMS(Fp.tile, 10);
5)
Code:
1: begin
rm; // what is this?
gameTab(rm);
w(500, 500);
gameTab(4);
end;
Why do you call RM; there?
More more more failsafes!
Yes from me.
* Form
You should make player setup there. Check out Naumans script as example (using SRL Player form)
* WalkingDDTM
I suggest you to declare 2 new variables: RCol and WCol. In the beginning of the function, put this:
Code:
RCol := FindRockColor;
WCol := FindWaterColor;
So the autocoloring functions would be called only once to make the function faster.
* Line 866
If(Players[CurrentPlayer].Integers[5] = 1 or 2) then exit;
"1 or 2" means actually 3. You can try this with SCAR writeln(IntToStr(1 or 2))
* DeathWalk is not actually needed, because there are no fighting randoms anymore
* AntiRandom fight detection is not needed anymore.
* SMARTSetupEx is already called in SetupSRL; if /misc/smart.scar is included (new in rev #35)
* MainLoop
Things between lines 945 - 953 will never occur, because former on line 926 you already have checked if AllPlayersInactive
* Line 696
Burns my eyes! Use cases! For 1 line If - Then statements you don't need begin - end. You can do it like this:
Code:
If(Players[CurrentPlayer].Integers[5] = 2) Then
Players[CurrentPlayer].Integers[2]:= Players[CurrentPlayer].Integers[2]+ 28;
If(Players[CurrentPlayer].Integers[5] = 1) Then
Players[CurrentPlayer].Integers[2]:= Players[CurrentPlayer].Integers[2]+ 27;
* Walking
Always use FindDTMRotated() !! because you will never find anything with FindDTM. The MM is never perfectly aligned.
* ChopWillow
You find the willow, and then immediately check uptext. What if the willow is not right next to you?
+ The form was cool, but it needs Player setup.
+ Screenshot saving.
+ Standards, except the nesting problem i mentioned above.
Help to your cutting problem:
1. Find a willow, click it
2. Wait till we have stopped
3. Find the same willow again
4. Save the willow co-cords to (x, y) and do this
Code:
FindColorsSpiralTolerance(x, y, TPA, WillowColor, x-25, y-25, x+25, y+25, 5);
if (Length(TPA) < 5) then
writeln('The Willow is gone!')
else
writeln('The Willow is still there!');
5. Make it randomly every now and then (like every 10+random(3) seconds) MMouse Willow and check uptext 'illo'. If found, make it click the tree to make sure you're still chopping it.
you should put the new script to the 1st post, so people would see it before they vote.
I still have to say No.
Your tree finding function never gets its Attempts to 50, cause it exits after the 1st failure.
Code:
if (Length(WillowATPA) = 0) then
begin
Wait(100 + random(100));
Attempts := Attempts + 1;
Writeln('Willows could not be found. Switching Players….');
Exit; // it will exit even if Attempts is 1.
end;
In your mapwalking you use DTM, not DDTM. DDTM = Dynamic DTM, which means that it is a DTM of which points color tolerance may vary. DDTMs are made with functions, that look somewhat like this:
Code:
function LoadDDTM(tolerance: integer): integer;
begin
MyRoadColor := FindRoadColor;
DTMMainPoint.x := 876;
DTMMainPoint.y := 84;
DTMMainPoint.AreaSize := 1;
DTMMainPoint.AreaShape := 0;
DTMMainPoint.Color := MyRoadColor;
DTMMainPoint.Tolerance := tolerance; // this is set in function parameters, see?
DTMSubPoints[0].x := 764;
DTMSubPoints[0].y := 234;
DTMSubPoints[0].Are….
…
DTMSubPoints[1].x := 654;
DTMSubPoints[1].y := 32;
…
DTMSubPoints[2].x
// and so on, read tuts about this
To the banking thing: you should make it a function like this:
Take these couple of functions from here:
http://www.villavu.com/forum/showthread.php?t=46310 (FindBankEx, OpenBankBooth)
Code:
function DepositLogs: boolean;
var
x, y: integer;
begin
if not FindBankEx('db', x, y, 'move') then
if not YourOwnFindBank then exit;
wait(3000+random(1000));
if not OpenBankBooth('db') then
if not YourOwnOpenBank then exit;
wait(1000+random(1000));
DepositAll;
result := not InvFull;
end;
end;
To-Do-List:
- DDTM to mapwalking
- use functions! you only have procedures
- failsafes, you have barely any.
The last 3 would look much more pretty if I could have hit the quote button, but the threads were closed :p
I just searched the applications forum and copy + pasted the first four posts that came up there. Every single one of his posts in the applications section that I have seen is extremely constructive and not just a vote.
I wasn't going to bother copying and pasting the other 26 votes he's had in there, but if you want to look, just search the appliance section.