Salty as fuck Posted February 21, 2016 Author Share Posted February 21, 2016 Maybe use ID's instead of item names? Id's can change depending on updates while names don't. 1 Quote Link to comment Share on other sites More sharing options...
crico Posted February 21, 2016 Share Posted February 21, 2016 Looks good! Quote Link to comment Share on other sites More sharing options...
Extreme Scripts Posted February 21, 2016 Share Posted February 21, 2016 (edited) Really good attempt for a first script! My suggestions would be to clean up your code a lil cos you could simplify a lot of things e.g. When checking for a Glory in your bank you can just use: getBank().getItems().stream().filter(item -> item != null && item.getName().contains("Amulet of glory")).collect(Collectors.toList()); That will return all the glories in your bank. Great start though Edited February 21, 2016 by Extreme Scripts 2 Quote Link to comment Share on other sites More sharing options...
ProPheT1x Posted February 21, 2016 Share Posted February 21, 2016 Looks swell. Quote Link to comment Share on other sites More sharing options...
Salty as fuck Posted February 21, 2016 Author Share Posted February 21, 2016 Really good attempt for a first script! My suggestions would be to clean up your code a lil cos you could simplify a lot of things e.g. When checking for a Glory in your bank you can just use: getBank().getItems().stream().filter(item -> item != null && item.getName().contains("Amulet of glory")).collect(Collectors.toList()); That will return all the glories in your bank. Great start though Literally a life saver Writing everything out was so annoying. Thanks :3 Quote Link to comment Share on other sites More sharing options...
Explv Posted February 21, 2016 Share Posted February 21, 2016 (edited) Really good attempt for a first script! My suggestions would be to clean up your code a lil cos you could simplify a lot of things e.g. When checking for a Glory in your bank you can just use: getBank().getItems().stream().filter(item -> item != null && item.getName().contains("Amulet of glory")).collect(Collectors.toList()); That will return all the glories in your bank. Great start though Arrays.stream(getBank().getItems()).filter(item -> item.getName().contains("Amulet of glory")).collect(Collectors.toList()); FTFY. getBank().getItems() returns an Item[] Edited February 21, 2016 by Explv Quote Link to comment Share on other sites More sharing options...
Botre Posted February 21, 2016 Share Posted February 21, 2016 Arrays.stream(getBank().getItems()).filter(item -> item.getName().contains("Amulet of glory")).collect(Collectors.toList()); FTFY. getBank().getItems() returns an Item[] Optional<Item> glory = Arrays.stream(getBank().getItems()).filter(i -> i.getName().startsWith("Amulet of glory")).findFirst(); FTFY. 1 Quote Link to comment Share on other sites More sharing options...
Explv Posted February 21, 2016 Share Posted February 21, 2016 (edited) Optional<Item> glory = Arrays.stream(getBank().getItems()).filter(i -> i.getName().startsWith("Amulet of glory")).findFirst(); FTFY. Item glory = getBank().getItem(item -> item.getName().startsWith("Amulet of glory")); FTFY. Edited February 21, 2016 by Explv Quote Link to comment Share on other sites More sharing options...
Botre Posted February 21, 2016 Share Posted February 21, 2016 (edited) Item glory = getBank().getItem(item -> item.getName().startsWith("Amulet of glory")); FTFY. Nope. Mine is still faster, less expensive and safer. Thank you for playing, please try again! Edited February 21, 2016 by Botre Quote Link to comment Share on other sites More sharing options...
Explv Posted February 22, 2016 Share Posted February 22, 2016 (edited) Nope. Mine is still faster, less expensive and safer. Thank you for playing, please try again! Your opinion. You should learn what premature optimisation is. My solution is better. Thank you for playing, please try again! Edited February 22, 2016 by Explv Quote Link to comment Share on other sites More sharing options...
Botre Posted February 22, 2016 Share Posted February 22, 2016 (edited) Your opinion. You should learn what premature optimisation is. My solution is better. Thank you for playing, please try again! Premature optimization is only an anti-pattern if the cost of the optimization outweighs the benefit. Me picking the best solution over the worse solution in this case and at this point doesn't cost me anything extra because.... ... I already know the best solution and can implement it as fast if not faster than the lesser solution. Since the optimization didn't cost me anything, the cost can't outweigh the benefit and this is there not a case of antipattern premature optimization but efficient optimization tout-court. *tokens only, push to reject* I'm just playing lel, let's stop this cockfight. Edited February 22, 2016 by Botre Quote Link to comment Share on other sites More sharing options...
lisabe96 Posted February 22, 2016 Share Posted February 22, 2016 (edited) for (Item item : script.getInventory().getItems()) { if (item != null && item.getName().startsWith("Amulet of glory")) { return item; } } I win for readable oldschool code Edited February 22, 2016 by lisabe96 Quote Link to comment Share on other sites More sharing options...
Flamezzz Posted February 22, 2016 Share Posted February 22, 2016 (edited) I would still prefer the most readable solution of all: bank.getItem(new ContainsNameFilter<>("Amulet of glory")); It reads nicely like a sentence and is equal in performance when rounded to nanoseconds.@OP Nice script Edited February 22, 2016 by Flamezzz 2 Quote Link to comment Share on other sites More sharing options...
Explv Posted February 22, 2016 Share Posted February 22, 2016 (edited) I would still prefer the most readable solution of all: bank.getItem(new ContainsNameFilter<>("Amulet of glory")); It reads nicely like a sentence and is equal in performance when rounded to nanoseconds. @OP Nice script But didn't you know you should maximise performance, and minimise readability Edited February 22, 2016 by Explv Quote Link to comment Share on other sites More sharing options...
Kemdawg Posted March 3, 2016 Share Posted March 3, 2016 Using the string ("Amulet of glory") will also give you uncharged glory's so wouldn't it be safer to just use ("Amulet of glory (") to at least make sure that there's at least a single charge left in the glory? Not sure if the script doesn't need uncharged glorys but my assumption was that they're used for teleporting. 2 Quote Link to comment Share on other sites More sharing options...