Ericthecmh Posted October 8, 2015 Share Posted October 8, 2015 Another reason I wrote this API is easier cross-bot compatibility. Means I rewrite less code, and therefore I have the same code base in case a bug breaks out. Within this context, a null is not good. A null means that code that should work perfectly fine will not work. A null means additional checking on the users end, which can cause more problems if they forget. In my case, I pass a omniapi.data.DefaultNPC instance, which simply extends omniapi.data.NPC and is constructed to, simply, not exist. This means there is no NPE when trying to interact (my above post), but it still actually represents nothing. A new scripter is going to try the exact code I highlighted in my above post, realise it doesn't work, and panic because they wrote incorrect code (that is actually correct). I personally don't believe that the API should ever return null, especially in contexts like this. Lastly, I wrote this API as this is personally what I believe the OSBot API should be like. cross-bot? HERESY. BURN HIMMM 3 Quote Link to comment Share on other sites More sharing options...
Alek Posted October 9, 2015 Share Posted October 9, 2015 Alright, here is a reasonable example of why you want to have control: Monster X is found in two regions, Region A and Region B. NPC monsterX = getNPCs().closest("Monster X"); if(monsterX != null)monsterX.interact("Attack"); else if (regionB.contains(myPlayer())walk(new Path(regionA)) Not using null checks:getNPCs().closest("Monster X").interact("Attack"); //Does nothing, doesn't go to a different regionNot using null checks with region switching:NPC monsterX = getNPCs().closest("Monster X"); if(!monsterX.getPosition.equals(new Position(-1, -1, 0)) //See where I'm going with this?monsterX.interact("Attack")else if (regionB.contains(myPlayer()) walk(new Path(regionA)); You could do some hard spaghetti crap and make booleans for new NPC instances, but that's a hundred times worse than returning null. It's totally fine for scripters to do this because you guys know exactly what to expect from the behavior of your script. From our standpoint, this would be a nightmare. Quote Link to comment Share on other sites More sharing options...
fixthissite Posted October 9, 2015 Share Posted October 9, 2015 (edited) Alright, here is a reasonable example of why you want to have control: Monster X is found in two regions, Region A and Region B. NPC monsterX = getNPCs().closest("Monster X"); if(monsterX != null) monsterX.interact("Attack"); else if (regionB.contains(myPlayer()) walk(new Path(regionA)) Not using null checks: getNPCs().closest("Monster X").interact("Attack"); //Does nothing, doesn't go to a different region Not using null checks with region switching: NPC monsterX = getNPCs().closest("Monster X"); if(!monsterX.getPosition.equals(new Position(-1, -1, 0)) //See where I'm going with this? monsterX.interact("Attack") else if (regionB.contains(myPlayer()) walk(new Path(regionA)); You could do some hard spaghetti crap and make booleans for new NPC instances, but that's a hundred times worse than returning null. It's totally fine for scripters to do this because you guys know exactly what to expect from the behavior of your script. From our standpoint, this would be a nightmare. It should be optional. Check out my suggestion thread.With a null object, you can still optionally check the reference. The point is it's common to "do nothing" if there is no object to work with, so you should not force the check. The branching should only occur if you want an else (like your example). NPC npc = ...; if(npc == NPC.NONE) { //move } else { npc.interact(...); } The player might not want to go to a new location. There are times where you're waiting for something to spawn, which you don't want to do anything at that exact moment (such as waiting for a tree to spawn). It's common to "do nothing" if null occurs. You should not force this. There are seriously no downfalls. Null pointers were a mistake. Some languages only support it for compatibility with other languages. Edited October 9, 2015 by fixthissite 1 Quote Link to comment Share on other sites More sharing options...
Alek Posted October 9, 2015 Share Posted October 9, 2015 It should be optional. Check out my suggestion thread. With a null object, you can still optionally check the reference. The point is it's common to "do nothing" if there is no object to work with, so you should not force the check. The branching should only occur if you want an else (like your example). NPC npc = ...; if(npc == NPC.NONE) { //move } else { npc.interact(...); } The player might not want to go to a new location. There are times where you're waiting for something to spawn, which you don't want to do anything at that exact moment (such as waiting for a tree to spawn). It's common to "do nothing" if null occurs. Respect your opinion, but I don't agree with it. You are then comparing two NPCs, which is more of an expensive check than null (not by very much). The only situation where I can see this being "alright" is sitting in a single area waiting for something with a fast respawn rate. If you are sitting at KBD (long respawn), you could be doing other tasks (maybe some anti-ban crap). Above all this would break way too many scripts because you are talking about all entities that this would affect (widgets, ground items, inventory, objects, npcs, etc). Quote Link to comment Share on other sites More sharing options...
fixthissite Posted October 9, 2015 Share Posted October 9, 2015 (edited) Respect your opinion, but I don't agree with it. You are then comparing two NPCs, which is more of an expensive check than null (not by very much). The only situation where I can see this being "alright" is sitting in a single area waiting for something with a fast respawn rate. If you are sitting at KBD (long respawn), you could be doing other tasks (maybe some anti-ban crap). Above all this would break way too many scripts because you are talking about all entities that this would affect (widgets, ground items, inventory, objects, npcs, etc). That's micro-optimization and should be avoided. It is true that you COULD be doing other tasks, but you do not need to force it. Most scripters don't. There are situations where you would want to do nothing on null, and that's what the design is aiming for. Look through scripts, and I'm sure you'll see that most of the null checks are there primarily to avoid the NPE. I've seen scripters perform null checks when it isn't needed for the sake of "better safe than sorry".Sleeping will slow the bot down. If a rock/npc appears while the bot is sleeping, someone else could snatch it. It's still possible to do so aswell. It's helping the situations that don't need to branch if a reference is null Edited October 9, 2015 by fixthissite Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted October 9, 2015 Author Share Posted October 9, 2015 Alright, here is a reasonable example of why you want to have control: Monster X is found in two regions, Region A and Region B. NPC monsterX = getNPCs().closest("Monster X"); if(monsterX != null) monsterX.interact("Attack"); else if (regionB.contains(myPlayer()) walk(new Path(regionA)) Not using null checks: getNPCs().closest("Monster X").interact("Attack"); //Does nothing, doesn't go to a different region Not using null checks with region switching: NPC monsterX = getNPCs().closest("Monster X"); if(!monsterX.getPosition.equals(new Position(-1, -1, 0)) //See where I'm going with this? monsterX.interact("Attack") else if (regionB.contains(myPlayer()) walk(new Path(regionA)); You could do some hard spaghetti crap and make booleans for new NPC instances, but that's a hundred times worse than returning null. It's totally fine for scripters to do this because you guys know exactly what to expect from the behavior of your script. From our standpoint, this would be a nightmare. if (regionB.contains(myPlayer()) walk(regionA); else if (getNpcs().closest("Monster X").interact("Attack")) log("attacked"); would work fine as an alternative. Quote Link to comment Share on other sites More sharing options...
fixthissite Posted October 9, 2015 Share Posted October 9, 2015 (edited) if (regionB.contains(myPlayer()) walk(regionA);else if (getNpcs().closest("Monster X").interact("Attack")) log("attacked");would work fine as an alternative.He was referring to doing something if closest returned null. Your code focuses more on ensuring the player is at the correct area.I got bashed on another forum for this subject, at a time long before I came across the "billion dollar mistake" thing. I expressed it as an opinion, and it was shunned. It's like the time when people were told the world is round. It's like when Sacrotes got sentenced to death for denying belief in gods. But now, there are tons of topics on it, and tons of reason behind it. It can be debated for ages if you'd like, it's not going to change the fact that null pointers were a mistake (as coined by the creator of em). So it's best to leave it at "null grew on some people, so they prefer it" and allow them to code their style Edited October 9, 2015 by fixthissite Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted October 14, 2015 Author Share Posted October 14, 2015 Just wanna let you guys know that I've added webwalking to OmniAPI (courtesy of @Valkyr), enjoy Quote Link to comment Share on other sites More sharing options...
Woody Posted October 18, 2015 Share Posted October 18, 2015 Just wanna let you guys know that I've added webwalking to OmniAPI (courtesy of @Valkyr), enjoy Does it handle objects? Widgets? Dungeons? Teleports? Shortcuts? Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted October 18, 2015 Author Share Posted October 18, 2015 Does it handle objects? Widgets? Dungeons? Teleports? Shortcuts? It handles objects + the enter wilderness widget atm. There was a teleport handler however Valkyr never gave it to me, so I'll write my own at some point. 1 Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted October 20, 2015 Author Share Posted October 20, 2015 Hey guys, just letting you all know that I pushed another update that further improves the interaction system as well as implementing my PaintLib project (gonna make it look nicer soon), enjoy Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted November 1, 2015 Author Share Posted November 1, 2015 Alright, here is a reasonable example of why you want to have control: Monster X is found in two regions, Region A and Region B. NPC monsterX = getNPCs().closest("Monster X"); if(monsterX != null) monsterX.interact("Attack"); else if (regionB.contains(myPlayer()) walk(new Path(regionA)) Not using null checks: getNPCs().closest("Monster X").interact("Attack"); //Does nothing, doesn't go to a different region Not using null checks with region switching: NPC monsterX = getNPCs().closest("Monster X"); if(!monsterX.getPosition.equals(new Position(-1, -1, 0)) //See where I'm going with this? monsterX.interact("Attack") else if (regionB.contains(myPlayer()) walk(new Path(regionA)); You could do some hard spaghetti crap and make booleans for new NPC instances, but that's a hundred times worse than returning null. It's totally fine for scripters to do this because you guys know exactly what to expect from the behavior of your script. From our standpoint, this would be a nightmare. I know it's useless to reply at this point, it's been a long time. I'm not sure why I didn't say this in the first place, but in any data type present in OmniAPI, #exists() will define whether or not the object is both there and available to be interacted with (for example, in widgets it will check if the child isn't null and if the widget is visible). No need for nasty instanceof checks, no need for null checks either ;) if (getNPCFinder().findClosest("Dwarf").exists()) { getNPCFinder().getLastFound().attack(); } else { log("not there :("); } Or, in your case: NPC monsterX = getNPCFinder().findClosest("Monster X"); if (monsterX.exists()) monsterX.attack(); else if (regionB.contains(myPlayer())) walk(new Path(regionA)); Might be something to look in to ;) Quote Link to comment Share on other sites More sharing options...
Cartzlid Posted November 12, 2015 Share Posted November 12, 2015 Can you please use Maven? Quote Link to comment Share on other sites More sharing options...
Bobrocket Posted November 13, 2015 Author Share Posted November 13, 2015 Can you please use Maven? Any benefits for doing so? Quote Link to comment Share on other sites More sharing options...