Here you go. I'm sorry to nitpick but you wanted someone to look over it and I've found nothing that would cause an error. But I've given you some advice and fixed your standards to give you an example.
Simba Code:
program Willow;
{$DEFINE SMART} //COMMENT OUT FOR NO SMART
{$i SRL\SRL.simba}
{$i SPS/SPS.simba}
var
objo:Integer;
myPath:TPointArray; (* This should be camel capped. *)
(*
I added a () at the end of the procedure "DeclarePlayers".
*)
procedure DeclarePlayers();
begin
HowManyPlayers := 1;
NumberOfPlayers(HowManyPlayers);
CurrentPlayer := 0;
(*
I have changed your player declaration here to be more compact and easy to
read, add more players to if one wishes and to keep track of. It works
the same way that you had it but it is much easier to read in shorter
code.
*)
with Players[0] do
begin
Name := '';
Pass := '';
Pin := '';
Active := True;
end;
end;
(*
Bold words should not be capped and procedure/function names should never be
all capped. They should be capped properly like AntiBan or Antiban.
*)
procedure Antiban();
begin
(*
You need to practice proper indentation and spacing between lines of code.
There is also no real need to add constant begin .. ends to each of the
cases if there is only one line of code for a case.
*)
case Random(30) of
0: RandomRClick();
1: BoredHuman();
2: PickUpMouse();
3: ExamineInv();
4: HoverSkill('woodcutting', False); (* I've capped 'False'. *)
(* You should be using SRL_ANGLE_HIGH or SRL_ANGLE_LOW. *)
5: RandomAngle(SRL_ANGLE_HIGH);
end;
end;
procedure Chop();
var
(*
Variable names for words should be camel capped or properly capped if one
word. Integers like x, y, z, etc should be lower cased, as you have them
already.
*)
x, y, po, Timer: Integer;
begin
(*
I've removed the +1 from the InvCount. I liked your idea but I feel like it
could be used better if we declare it as just the initial inventory count
and then later in the procedure we will use it in a better way.
*)
po := InvCount;
(*
You should not be declaring x and y as these static positions, there is no
point to it. Unless I'm misunderstanding FindObjTPA you don't have to and
should not be passing these variables as MSCX and MSCY. Though if you
need to for the function for some reason, then you should just use MSCX
and MSCY as the position variables instead of creating memory load
by declaring x and y.
I've removed this part of the code and changed your FindObjTPA.
*)
if (FindObjTPA(x,y,3164747,10,1,15,60,450,['hop Will'])) then
begin
Mouse(x,y,1,1,False); (* False needs to be capped *)
WaitOption('Chop',1000);
Flag(); (* This should be capped. *)
MarkTime(Timer);
repeat
Wait(1000+random(300)); (* This should be capped. *)
Antiban;
(*
You shouldn't be passing static variables for the timer. I fixed it for you.
I've also changed your InvCount = po to InvCount > po because of the
earlier change that I made. This will be the same exact thing but it
looks cleaner, in my opinion.
*)
until((InvCount > po) or (TimeFromMark(Timer) >= RandomRange(30000, 40000)));
(*
This isn't so much a standards issue as it is more a scripting logic
nitpicking of mine. You increase the global variable objo by 1 if the
inventory count is greater than the initial count. Why don't you change
this to be something like this:
Declare a new local variable for this procedure called newCount.
newCount := InvCount;
newCount := newCount - po;
if (InvCount <> po) then
IncEx(objo, newCount);
Or something like that, anyway. I'm rusty but I believe that's how you'd
do it to make sure you count every single log chopped.
*)
if(InvCount > po) then
Inc(objo);
end;
end;
procedure Proggy();
begin
Writeln('*******************************');
Writeln('Blackstabs Chopper. Enjoy!!');
Writeln('You have been running for: ' + TimeRunning);
(* This can be ToStr instead of IntToStr. *)
Writeln('You have Chopped: ' + ToStr(objo));
Writeln('*******************************');
end;
(*
Why do you not return for this function? It should be a procedure if you
do not plan to return a value.
I also cleaned this function up a 'bit to be neater.
*)
function Bankr() : Boolean;
begin
OpenBankFast('db');
if(BankScreen) then
DepositAll();
CloseBank;
end;
begin
SetupSRL;
DeclarePlayers;
LoginPlayer;
SPS_Setup(RUNESCAPE_SURFACE,['10_8','11_9','10_9']);
myPath := [Point(4185, 3729), Point(4177, 3695), Point(4207, 3687)];
FindNormalRandoms;
repeat
SPS_WalkToPos(Point(4190, 3739));
Chop;
FindNormalRandoms;
wait(1250); (* Remember that you sohuldn't be using static wait times. *)
if (InvFull) then
begin
//Drop/bank and/or walking procedure goes here...
SPS_WalkPath(myPath);
wait(4000); (* No static wait times. *)
Bankr;
ClearDebug;
Proggy;
end;
(* You should change false to be less fail-prone and to be more safe. *)
until(False);
end.