Transporter Posted July 21, 2016 Share Posted July 21, 2016 (edited) Say I had 100 possible nodes in my onStart method: Collections.addAll( nodes, new nodeOne(this), new NodeTwo(this), new nodeThree(this), new nodeFour(this), etc... up to 100 Won't it take a long time to go through all these nodes? Is there a better way to sort the nodes? E,g say 5 of the nodes were involved in separate stages of a banking method, would it be better to have one Banking class and then putting the others into states? Edited July 21, 2016 by Transporter Quote Link to comment Share on other sites More sharing options...
AresScripts Posted July 21, 2016 Share Posted July 21, 2016 Say I had 100 possible nodes in my onStart method: Collections.addAll( nodes, new nodeOne(this), new NodeTwo(this), new nodeThree(this), new nodeFour(this), etc... up to 100 Won't it take a long time to go through all these nodes? Is there a better way to sort the nodes? E,g say 5 of the nodes were involved in separate stages of a banking method, would it be better to have one Banking class and then putting the others into states? Classes are supposed to break programs down into smaller parts to make it less complicated. Having 100 different classes, all being nodes, would overcomplicate things. Quote Link to comment Share on other sites More sharing options...
Botre Posted July 21, 2016 Share Posted July 21, 2016 Please don't use "nodes". You bring up one great reason not to use them (among many others): inefficient and/or unnecessarily convoluted control flow. Quote Link to comment Share on other sites More sharing options...
Lemons Posted July 21, 2016 Share Posted July 21, 2016 Won't it take a long time to go through all these nodes? Depends on what the checks are. If the check is "if the inventory is full" that is a pretty quick check. If it involves a lot of computations/loops, its probably going to Is there a better way to sort the nodes? Yes, do something as you mentioned with the Banking class. You can also make a new type of "Node" which allows nesting of other nodes into it, so you can have more of a tree of nodes instead of a straight line. I typically try to make nodes reusable, so I boil them down to the most basic part I need (like Banking), and make it configurable on instantiation. You can see how I do it here (it's messy, sorry): https://github.com/Lem0ns/OSBotAPI/tree/master/tasks 1 Quote Link to comment Share on other sites More sharing options...
Explv Posted July 21, 2016 Share Posted July 21, 2016 Say I had 100 possible nodes in my onStart method: Collections.addAll( nodes, new nodeOne(this), new NodeTwo(this), new nodeThree(this), new nodeFour(this), etc... up to 100 Won't it take a long time to go through all these nodes? Is there a better way to sort the nodes? E,g say 5 of the nodes were involved in separate stages of a banking method, would it be better to have one Banking class and then putting the others into states? 100 nodes?!?! You're doing something very, very wrong.. Quote Link to comment Share on other sites More sharing options...
liverare Posted July 21, 2016 Share Posted July 21, 2016 (edited) Won't it take a long time to go through all these nodes? Depends on what the checks are. If the check is "if the inventory is full" that is a pretty quick check. If it involves a lot of computations/loops, its probably going to Is there a better way to sort the nodes? Yes, do something as you mentioned with the Banking class. You can also make a new type of "Node" which allows nesting of other nodes into it, so you can have more of a tree of nodes instead of a straight line. I typically try to make nodes reusable, so I boil them down to the most basic part I need (like Banking), and make it configurable on instantiation. You can see how I do it here (it's messy, sorry): https://github.com/Lem0ns/OSBotAPI/tree/master/tasks This. 100 nodes?!?! You're doing something very, very wrong.. Or something very, very incredible. Don't underestimate anyone. If you're finding you have a lot of nodes and they're all necessary, then you could consider using reflection. You can use reflection to initiate new node instances, instead of doing it yourself. It's a little more work behind the scenes, but it keeps things looking nice in the foreground. This is one example for my law rune crafter: @Override public void onStart() throws InterruptedException { super.onStart(Wrapper.class); super.getContextContainer().registerTasks( /* DepositLawRunes.class, */ RepairPouches.class, WithdrawLogs.class, WithdrawRingOfDueling.class, EquipRingOfDueling.class, FillPouches.class, WithdrawPureEssence.class, FlyToEntrana.class, EnterLawRift.class, CraftLawRunes.class, WalkToBalloon.class, WalkToRuins.class, WalkToAltar.class); . . . } Although things can get very, very, very, very convoluted, with things becoming limited (because each node had to have uniform constructors), generic and...honestly bad. But it's nice in its own ways, for instance, each class had reference to a wrapper, and that was provided automatically by my framework. I made this because I started writing a shit load of random scripts and I wanted uniformity to simplify writing new scripts. However, I've found that some of my scripts failed to benefit in any way from the framework, or had to require some crowbarring of code to work properly with it. I'm moving away from all kinds of dependency (even of my own stuff) so that everything's self-reliant, even if it's more hassle when it comes to updating scripts. I ironically named the project and packages "better.scripts". xD Edited July 21, 2016 by liverare Quote Link to comment Share on other sites More sharing options...
Transporter Posted July 21, 2016 Author Share Posted July 21, 2016 100 nodes?!?! You're doing something very, very wrong.. I'm writing a very, very big script at the moment and my script doesn't have 100 nodes was just an example of a script with a lot of nodes. Quote Link to comment Share on other sites More sharing options...
Botre Posted July 21, 2016 Share Posted July 21, 2016 @Override public void onStart() throws InterruptedException { super.onStart(Wrapper.class); super.getContextContainer().registerTasks( /* DepositLawRunes.class, */ RepairPouches.class, WithdrawLogs.class, WithdrawRingOfDueling.class, EquipRingOfDueling.class, FillPouches.class, WithdrawPureEssence.class, FlyToEntrana.class, EnterLawRift.class, CraftLawRunes.class, WalkToBalloon.class, WalkToRuins.class, WalkToAltar.class); . . . } Dear god please no. You took a bad design pattern and somehow made it worse. @OP allow me to recommend you to stay away from the "node" approach. You should isolate and OO-ify conditions and behaviors in a decoupled fashion instead. And then just use classical good old-fashioned control flow mechanisms instead of iterating over a fucking collection of "if blocks". 1 Quote Link to comment Share on other sites More sharing options...
Auron Posted July 23, 2016 Share Posted July 23, 2016 (edited) I have a method in my main script called changeStateNodes(), and it assesses the current situation and adds only the necessary nodes.When it's obvious that the current active nodes have done their job, I call changeStateNodes(). I'm not sure what your situation is, but perhaps this could be useful? Edited July 23, 2016 by Auron Quote Link to comment Share on other sites More sharing options...