Jump to content
×
×
  • Create New...

Dan

Moderators
  • Posts

    356
  • Joined

  • Last visited

  • Days Won

    33

Dan last won the day on September 4

Dan had the most liked content!

Reputation

65 Excellent

5 Followers

About Dan

Designer
Script Writer III
  • Rank
    Moderator

Personal Information

  • Location
    Somewhere
  • Interests
    Stuff
  • Occupation
    Something

Recent Profile Visitors

1482 profile views
  1. Nice to see something slightly more unorthodox in your poll. Not everyone needs to follow the same framework, that's how we expand our knowledge base as a team. I personally don't like private api methods where we ones to achieve the same result. boolean success = Time.sleepUntil(ExInventory::isEmpty, 2000); if (success) /// VS boolean success = Condition.wait(()->ExInventory::isEmpty, 200, 10); if (success) However I'm assuming you just adapted your methods from another client to suit ours without really digging around too much in our API (albeit there's not much documentation). Although for the most part, your script is nice and tidy as well as split into a somewhat modular layout, for an activity such as wintertodt, stuffing everything into 1 class irks me slightly. Manageability is reduced significantly as debugging something in 300 lines of code is vs your 5 main activities split into their own task of ~80 lines each. You've clearly been around the block for a while and know what you're doing. I'll leave this open for @const_ to offer any suggestions and/or complete the application. 👍
  2. Nice structure and broad use of the API. Some feedback from looking through which may be helpful for the future. Firstly, I hate standard sleeps with a passion. https://github.com/Protoprize/PP-Aerial-Fisher/blob/35d4e7b0d0a1c7dc58a191b2daf21e42cffd015f/src/com/proto/aerial/tasks/GetBird.java#L19 Here you wait 1.2-1.8 seconds without any validation whatsoever. I've never done the activity you wrote the script for, so it may be that there's no validation available or whatever but I'd personally stay clear as you've used conditional waits perfectly fine throughout the rest of the script. Also make things easier for yourself and make use of the chat API. Component cont = Components.stream().text("Tap here to continue").first(); if (cont.valid()) { cont.click(); Condition.sleep(Random.nextInt(1200, 1800)); } //////////////////// VS if(Chat.canContinue){ Chat.clickContinue(); Condition.wait(()->!Chat.canContinue(), 150, 10); } I also noticed something else too which I thought could come in handy for you. I wasn't sure if you knew that Condition.wait() returns a boolean, if early exits, true, if times out, false so below, it just saves you doing the same Inventory.selectedItem() check twice. https://github.com/Protoprize/PP-Aerial-Fisher/blob/35d4e7b0d0a1c7dc58a191b2daf21e42cffd015f/src/com/proto/aerial/tasks/CutBait.java#L25 if (knife.interact("Use")) { Condition.wait(() -> Inventory.selectedItem().valid(), 200, 30); if (Inventory.selectedItem().valid()) { int x = Skills.level(Skill.Cooking.getIndex()); cut.interact("Use"); Condition.wait(() -> Inventory.stream().id(AerialData.CUT_ITEMS).isEmpty() || Skills.level(Skill.Cooking.getIndex()) != x, 300, 100); } } //////////////// VS if (knife.interact("Use")) { if (Condition.wait(() -> Inventory.selectedItem().valid(), 200, 30)) { int x = Skills.level(Skill.Cooking.getIndex()); cut.interact("Use"); Condition.wait(() -> Inventory.stream().id(AerialData.CUT_ITEMS).isEmpty() || Skills.level(Skill.Cooking.getIndex()) != x, 300, 100); } } However overall, you'll be a great addition to the team, congrats. @const_
  3. Using an item on another entity is a single click option and will therefore return false. For example if(Inventory.stream().name("Knife").first().interact("Use")){ if(Inventory.stream().name("Log").first().interact("Use")){ System.out.println("This won't return true"); } else { System.out.println("This will always return false"); } } /////////// VS if(Inventory.stream().name("Knife").first().interact("Use")){ if(Inventory.stream().name("Log").first().interact("Use", false)){ System.out.println("This should return true"); } } the second example passes useMenu = false, meaning you're telling interact() that what you're about to do, will have no menu to verify the correct option was selected. Unlike desktop, there's no 'uptext' or whatever it was called upon hovering over and item etc in the top left. Therefore without using the menu, we can't verify what entity was tapped on.
  4. Not at the moment, I don’t know if it will come back seeing as we’re a mobile only bot now.
  5. Pushed today, got me 1-43 prayer using gilded altar on w330, let me know how it goes.
  6. Much better, try and stay active this time! @const_
  7. I know it's tricky to get logs etc atm but one day you'll be thankful, nice work on that. 👍 So a few things to mention here, you can tell it's a game object because you're refering to it in the game object stream using Objects.stream().name("Mysterious ruins").nearest().first(). However, I never saw your code prior to using the tilematrix so I can't comment too much but declaring a game object is also known as caching. Caching something stores the data at it's current state. If the object isn't loaded/valid etc then you're caching a GameObject.Nil and therefore it has no information to pass to your object.interact("Enter") as it's not loaded the object to interact with. Just food for thought but I'd suggest caching an object if you have multiple calls to do with it, for example, if(Objects.stream().name("Mysterious ruins").nearest().first()!=GameObject.getNil()){ if (Objects.stream().name("Mysterious ruins").nearest().first().inViewport()){ Objects.stream().name("Mysterious ruins").nearest().first().interact("Enter"); } else { if(Objects.stream().name("Mysterious ruins").nearest().first().tile().distanceTo(Players.local())>10){ Movement.step(Objects.stream().name("Mysterious ruins").nearest().first()); } else { Camera.turnTo(Objects.stream().name("Mysterious ruins").nearest().first()); } } } //*******VS GameObject ruins = Objects.stream().name("Mysterious ruins").nearest().first(); if(ruins!=GameObject.getNil()){ if (ruins.inViewport()){ ruins.interact("Enter"); } else { if(ruins.tile().distanceTo(Players.local())>10){ Movement.step(ruins); } else { Camera.turnTo(ruins); } } } As you can see, the first example accesses the object stream 6 times, each time is a fresh call filtering out all other objects every time. If each object call is 100ms (example numbers) then to complete this it'll take 600ms alone just to access the objects data. The second example accesses the stream once and stores the data. However it's checking if it's Nil before continuing, if it's nil, it's not found the object and will then proceed to loop the tasks and come back to it, hopefully then, you're in range of the object in order to cache it and therefore accessing the stream once @100ms and then accessing the data from memory near instantly. With that being said, you've shown a real improvement over the last few days and weeks in your learning journey and I'm happy to congratulate you on joining the team. @const_ do your thing!
  8. I don't have access to a lot of your extended methods etc so some of this is going to be trying to assume the desired effect and implementation. PickpocketOrWait.java - Your waitUntil method, I'm assuming it's args are, boolean, looping time in ms, looping tries before exiting. If so, I don't see why you aren't just using the Condition.wait() from the api. However I also remember you saying something in my dm's about passing a timeout feature or something like that. Whatever the case, passing 10,000 and 15,000 seems a little extreme however, I can't really comment any more than that as I don't have access to the method however I would argue it's an unnecessary custom method to have with the existing Condition.wait(). Keeping with the same task, I don't quite follow the logic used. It validates if your thieving has dropped less than 5, I might be out of the loop but as far as I know thieving and failing doesn't drop your thieving level does it? Either way, IF it does activate, the first thing you check is if your hitpoints are <2, if so just idle. That seems rather dangerous logic to lock up the script for an extended period of time at 1 hp without any features to eat or to run to a safe spot included. Just because it makes no sense to do this in full 3rd age and a tbow, it doesn't mean someone won't and then come complaining to you that you lost them their whole bank because something happened and the script locked them out for 15 seconds. I'm not particularly a fan of task validations which are standard set to true. Your StealFromStall validate is always true, no validation in there at all which then forces your execute to run and subsequently unnecessarily wait. I personally don't like home brewed API's, over the years they break and don't get updated. "I don't want to publicly post everything I've implemented." If there's something that the API is missing so badly, as part of the script writer team, you should aim to improve the API and add to it for the better of the group rather than working solo. This is just personal preference but that doesn't take away from your clear capabilities. Overall it's a very well put together script. Overcomplicated extended api methods for example: InventoryExtended.containsAnyOf(mainScript.getThievingStall().getJunkNames()); //feature is already in api Inventory.stream().name(mainScript.getThievingStall().getJunkNames()).isNotEmpty(); However I'm assuming the version I was sent was already amended from Consts feedback as it's been split into a nice neat structure with references to enums etc. The commentary and suggestions provided are just that, food for thought but not required to proceed with the application as I'm happy you've shown enough understanding and beyond to be approved, promoted and accepted by all 😄 @const_ If you can do the honours.
  9. It'll only show the model when you're right next to it otherwise rendering every single model will take too much resource on the client so the model is fine. The update should hopefully be out soon but you should probably try and debug the issue. You don't have any printouts in the script so if anyone provides you with a logcat if it were public, you'd have nothing to see where stuff went wrong. A simple check of the return from the interact() would be better than waiting unecessarily. Interact() returns a boolean as to whether it was successfully interacted with or not, if it's true, enter conditional wait, otherwise, let the poll() loop the task again for another attempt. Something else you might want to try is waiting till you're no longer in motion before trying to interact as you can probably imagine, a static object is much easier to interact with than a moving one. https://github.com/Gamburger1/Runecrafting-spinbot/blob/f6adcad7426a7173822d45a7b239eb13ba63d02b/src/Runecrafting/MudRuneTasks/ClickBalloon.java#L32 I also just realised that in this task, you don't actually have any checks to see which action you should be carrying out. Line 31 will wait 4.5 seconds before continuing if the interaction failed, but I mentioned how to resolve this above. However you don't actually check if the widget on line 32 is even visible before trying to click it, if it's not then you're waiting another 4.5 seconds so if you miss the first interaction, you're waiting 9 seconds each loop. While you've split your tasks down to a very granular level, multiple actions per task should still have if checks to ensure you're ready to complete the next action in the task.
  10. While I can understand the frustrations of some objects not working, using a workaround rather than trying to solve the problem doesn’t help anyone. I know I gave you the example script for my barbarian fishing which used the npc tile matrix to interact with, but that was written back when we had no active devs fixing these issues. Now we do. While having a script that works is great but if there’s something that isn’t working as intended you should be trying to fix it or get it fixed rather than working around the issue. @_RickDo you have issues when loading the mysterious ruins? I vaguely remember there being something that you changed recently so wasn’t sure if that’s now all good. Also, @Kebab King can you check the model of the basket? If the model isn’t correct or is absent then that will need looking at but otherwise you should be using the Object stream to interact with objects and not their tiles. I know there’s been a bit of a bugged update yesterday so testing things now probably won’t be a good idea. Glad to see the Bank api being used and some generifying taking place however.
  11. This is quite nicely written, there's some things to change or look at below. Good structure to your files, nice and modular making it simpler and easier for you to maintain and debug in the future. This script offers the opportunity to show good overall use of the API with a few different aspects covered, moving/walking, objects, inventory, banking, equipment. In terms of feedback, from what I can tell, you have a fear of using objects. in almost every task you use an objects tile matrix and interact with that which isn't really how it should be done. If you want to interact with an object at a specific tile, you can pass at(Tile). For example Objects.stream().at(new Tile(3532, 2452, 0)).name("Door").interact("Open"); Is there a reason why you don't use the Bank API to interact with the bankers and bank booths? https://github.com/Gamburger1/Runecrafting-spinbot/blob/3e63bfda77f1a5911c2c34e80bf0a08db33fc9ca/src/Runecrafting/AirRuneTasks/ClickBank.java#L26 return Objects.stream().name("Bank booth").nearest().first().inViewport() vs return Bank.inViewport() I felt that you have a lot of repeated code and could probably do with generifying it. For example here; https://github.com/Gamburger1/Runecrafting-spinbot/blob/3e63bfda77f1a5911c2c34e80bf0a08db33fc9ca/src/Runecrafting/MudRuneTasks/WithdrawEssence.java#L65 I don't see why you need 4 different methods for the different stamina potions, lots of different options but as an idea; String stam = ""; ... ... ... if (Movement.energyLevel() < RUN_ENERGY) { stam = ""; if(Bank.stream().name("Stamina potion(1)").count()>0) { stam = "Stamina potion(1)"; } else if(Bank.stream().name("Stamina potion(2)").count()>0) { stam = "Stamina potion(2)"; } else if(Bank.stream().name("Stamina potion(3)").count()>0) { stam = "Stamina potion(3)"; } else if(Bank.stream().name("Stamina potion(4)").count()>0) { stam = "Stamina potion(4)"; } if(!stam.equals("")){ drinkStamina(stam); } } ... ... ... private void drinkStamina(String stamina){ System.out.println("Run energy minimum is " + RUN_ENERGY + " Withdrawing stamina potion and drinking it"); Bank.withdraw(stamina, 1); Condition.wait(() -> Inventory.stream().name(stamina).count() == 1, 150, 15); Inventory.stream().namne(stamina).first().interact("Drink"); Condition.wait(() -> Inventory.stream().id(VIAL).count() == 1, 150, 15); Bank.deposit(229, 1); Condition.wait(() -> Inventory.stream().id(VIAL).count() < 1, 150, 15); } Your tasks use a lot of the same stuff, you've got a different clickBank task for each type of rune, a different crafting task for each type of rune when a lot of the actions are the same if not similar. A good start but just some bits to work on. Feel free to @me when you've updated it etc.
  12. I had one on V1, just waiting for some client fixes before I do any more porting, I think there were some botched updates over the last few days but it's on my todo list 🙂
  13. @iZho has released one. Will close this request.