@
xiaospike @
Joshwa534
As I continue to encounter your incredibly shit scripts, I've decided I'll take the opportunity to point out the flaws of this particular script. They are listed below. If you care to do something about them, that's your decision, but for the love of god stop distributing this shit.
Code:
print([[
Yalahar Minotaurs or Elves by Xiaospike & Joshwa534]])
wait(5000)
Why do you insert a 5 second wait after starting the script? This raises a couple of issues, not least being that if in your first couple of waypoints you happen to have a label, it will be ignored. This poses a potential risk to the end user, and is a complete waste of a thread.
Code:
local ESTMana = (MaxMana-20)
Walker.ConditionalGoto((Self.ItemCount(268) < ESTMana) or (Self.ItemCount(266) < MaxHealth) or (Self.ItemCount(7378) < MaxAmmo), "resupply", "tohunt")
Here's an interesting one... Why are you wasting time declaring a variable? Why not just do Walker.ConditionalGoto(Self.ItemCount(268) < MaxMana - 20~~~~~. If you really wanted to use a variable in this instance it would be far more convenient to use a variable to store the variability of the function (the number 20), that way you could allow the end user to set it to a value, so that if they will only use 2 or 3 manas at most on refilling (most likely) they will refill every time (even if they only deposited because of capacity...)
Code:
local withdrawManas = math.max(MaxMana - Self.ItemCount(268), 0)*50
local withdrawHealths = math.max(MaxHealth - Self.ItemCount(266), 0)*45
local withdrawAmmo = math.max(MaxAmmo - Self.ItemCount(7378), 0)*15
local totalmoneyneeded = (withdrawManas + withdrawHealths + withdrawAmmo)
local MATHCEIL = (math.ceil((totalmoneyneeded/1000)))*1000
OK, so this is a large chunk to comment on. Firstly, it's ugly as hell. A much more organised way to do this would be to make a function and name it CalculateItemCost, pass in a variable for amount to buy, item ID, and cost per item, then perform these calculations and return the value. It looks shit, and is difficult to debug as a result. Secondly, why would you attempt to truncate a number in such a way? As you are simply rounding, you could use the function math.round. Sure, math.ceil will do the same job, but it once again makes code unreadable and difficult to debug. Last comment about this point, how many people wrote this code? I've noticed elsewhere as well, that your naming convention is to name it whatever the hell you want (e.g totalmoneyneeded all lower case, MATHCEIL very non-descriptive and all upper case, then the camel convention for withdrawHealths etc...) This is incredibly bad practise and anyone in the programming or scripting industry would mock you for it. It is laughable that you have actually managed to use 3 different conventions in 3 consecutive lines. Oh, I almost forgot, why exactly did you put so many brackets on the math.ceil line? Or wait, are you trying to make your code impossible to read? That does seem like the sole intention of this particular script.
Why bother? I mean, seriously, what is the point? If you really wanted to make something here which would ensure the money was withdrawn, just add a balance check at the end of the withdrawing lines. The bot does not take 2 seconds to do what it is supposed to be doing here. Never has done, never will do. Exactly 2 seconds, as well? Hmmmm...
Code:
if (LogoutStamina) and (Self.Stamina() < 960) then
It seems very unintuitive to assume that an end user wants to log out with 2 hours remaining. A round of most hunts for me is much less than an hour, another thing which would be beneficial to have as a variable. On top of that, here's your fourth naming convention. Well done. Include a fifth next time for a free gift voucher!
Code:
Self.SayToNpc({"hi", "flasks", "yes", "yes", "yes", "yes", "yes", "yes", "trade"}, 65)
What the shit? So someone's just started up your script... It goes to the bank, withdraws 0 money, then goes to a mana shop... I know what it should do next! It should spam the word yes in an attempt to sell flasks which it doesn't have any of! Yes! Perfect way to get banned, fucking marvellous! You know what's even better than that? Every time I go to that shop, why not make it do the same thing? Ohh, you already did. On top of that, why not make a script which is obviously targeting players < level 50 try to sell 600 potion flasks! Great! Good job, well done. Learn to use a while true statement, and you can simple say yes until you have no flasks left. If you really want to be random about it, use some randomisation (say yes once for each stack of flasks, plus zero, one, two, or three times depending on the state of a randomly generated number). Don't do this shit, you're just making it fucking obvious who is botting.
Code:
wait(2000)
if (Self.ItemCount(268) < MaxMana) then
BuyItems(268, MaxMana)
wait(500)
end
if (Self.ItemCount(266) < MaxHealth) then
BuyItems(266, MaxHealth)
wait(500)
end
wait(200, 500)
Again with these fucking waits? Seriously, why?! I know... Let's walk up to an NPC, and wait exactly 2 seconds. Next, we'll buy some mana potions and wait exactly half a second. Then, we'll buy some health potions and I tell you what, why not wait exactly another half a second. After that, maybe we can wait another 200-500 milliseconds, cause that will totally be amazingly fun! What the fuck?
Code:
elseif (labelName == "buyammo") then
Walker.Stop()
Self.SayToNpc({"hi", "trade"}, 65)
wait(1000)
if (Self.ItemCount(7378) < (MaxAmmo)) then
BuyItems(7378, MaxAmmo)
wait(500)
end
Walker.Start()
Ok, so we're now saying trade to NPCs before trading? It's not "bad", as I guess that the function (not made by yourselves, of course, because it actually works as it should) to buy items will check that the trade window is open, but this is a waste of calculations. I see you had another great idea here, to say trade, wait exactly a second, then buy some potions, then wait exactly half a second and walk off. Inconspicuous.
Code:
elseif (labelName == "resetbp") then
Walker.Stop()
Container.Close(GoldBP)
wait(1000)
Container.GetFirst():OpenChildren(GoldBP)
wait(1000)
Container.GetByName(GoldBP):Minimize()
if (HideEquipment) then
Client.HideEquipment()
wait(1000)
end
Walker.Start()
end
Should I even bitch about the sleeps here? Do I need to? What this is going to do:
Close the gold BP.
Wait exactly 1 second.
Open the gold BP.
Wait exactly 1 second.
Minimise the gold BP.
Hide the equipment.
Wait exactly 1 second.
Walk off.
You don't think this is becoming a pattern?
Ok, so let's move on to the functions you use:
Code:
function SellItems(item) -- item = item ID
wait(300, 1700)
Self.ShopSellItem(item, Self.ShopGetItemSaleCount(item))
wait(900, 1200)
end
Wait, buy, wait? Why not use a convention? Why waste time waiting before and (sometimes) after every function, when you could just be waiting before every function? I think I know the answer you're going to give (if any), and I suspect you will say something like "we did that incase someone else tries to copy this code, so they won't risk getting banned by doing shit too quickly", but then again there is a large part of me that suspects that you didn't even write this code, so meh.
Code:
function BuyItems(item, count) -- item = item id, count = how many you want to buy up to
wait(900, 1200)
if (Self.ItemCount(item) < count) then
Self.ShopBuyItem(item, (count-Self.ItemCount(item)))
wait(200, 500)
end
end
This one is quite strange... Why would your first wait be outside of the if statement, and your second inside? It makes no sense. Then again, based on the code I've reviewed so far, it seems that your primary "company principle" is something like "If there is nothing else to do, wait." Good one. After reviewing this post, it's also just come to my attention that in fact you are waiting a bit, then this function is called, then it's waiting some more, then it's doing something, then waiting some more, then waiting some more again, then it's gonna do something else. Yeah, it's semi-random (primarily because this piece of code is not designed to have wait calls before and after it), but fuck me it's wasteful.
I won't comment on the rest of the functions, as I do not believe you wrote them. Feel free to correct me if I'm wrong, but I doubt you have the capacity to write anything capable of doing anything productive.
@
inadequate - For the reasons listed above, I won't debug this code for you. If you want it fixed, I'm sure JXScripts will happily do so. Personally, I don't have the time to even begin repairing this shoddy work. Sorry.
Originally Posted by XtrmJosh
