Solved Efficiency and my plugin...

Hi,

So the past 2 days I have completed a fully working plugin that puts tnt in dispensers in a specified radius, here is the pastebin,http://pastebin.com/hEqANt2T

What you will notice is I have spent alot of time going over almost every scenario, there are still 2 I have left out... so everything returns true but it uses a hell of alot of ifs and elses, and what im focused most on is to reduce the amount of for statements because I tried the max radius on my server and well it took 2 seconds to complete the block checking on a flatworld, so im trying to imagine what it would be like on a server with 10+ people on always, thanks to anyone who helps me in advanced, remember there is nothing wrong with the code I just want to make it more efficient

 

First things first:
- Follow java naming conventions
- Use Encapsulation not directly public fields

So, If's and else's are completely fine once used properly, so having an if-else if- else statement is efficient (especially compared to simply a bunch of if's one next to another)
Can you please point the for loop(s) (Line numbers) so we can have a look without reading all 500 lines of code?
For the loops i have seen so far, i have to say that you can make a method instead of running the same loop in each senario, Also if you want you can tell us the function you want these loops to perform and mabe there is a Build in method in bukkit maintained properly & efficiently.

 

@mine-care

I do follow Java naming conventions, it's just my school IT teachers are confusing me by making me name almost everything opposite to the Java naming conventions and I just had exams getting back into things, hope not too many things are wrong there

I plan on using the integers in different classes once I get this 'base' done

81, 127, 163, 209, 250, 293, 349, 393, 418, 463

The above is the line numbers of all the for loops, the for loops with NOD++ in them are counting how many dispensers are in the radius, the others are adding the correct amount of tnt to each based on the calculations done before it, just wanted to know if there is a more efficient way or doing this instead of looping through all the blocks in that radius 2 times to 1st check how many dispensers are, then check if the player has enough tnt, then fill the dispensers with the amount of tnt if the player has the tnt etc... And lol I completely went past creating a method for the for loops

 

@LeePMC Well i mentioned naming conventions bcause your class name does not follow camel capitalization.

Now about your issue, i saw all these loops. You may want to turn them all to a method and use it instead of writing them over and over, as i imagine it by not, you loop through all the blocks arround the player and if the block is dispenser you increase the count. Firstly instead of counting the dispensers, then the tnt and then adding it you can have them all in 1 loop, in the one tht counts the dispensers arround, instead of just counting add the tnt and when you reach the point where player has no more tnt on him, have a count to report to the user that "X dispensers where not filled! you dont have enough tnt!"

 

@LeePMC

You can remove your loops which check every block in a certain radius if it is a dispenser and replace them with only checking the tile entities in the nearby/affected chunks (dispensers are special tileentity blocks): https://github.com/Bukkit/Bukkit-JavaDoc/blob/master/src/main/java/org/bukkit/Chunk.java#L75

 

@mine-care this section is for you:

Class name doesn't follow because my IT exams required all html documents to be lowercase then capital, so I will make a new Java project and fix all the errors there

I like your idea and I can definitely do that... But let me give you a scenario, two actually

1. The guy your making the plugin for wants all dispensers filled with that amount of tnt in each, and if the player doesn't have enough tnt tell them how many dispensers are in the radius (I am making this for someone, but I'm not sure if he wants what you have explained, I will explain to him it will reduce server lag)

2. This one is true and is currently active in the class, I have /tntfill auto <radius> Which first counts how much tnt your have, then calculates how much tnt can go in each dispenser equally, leaving the remainder of tnt in the players inventory. Is there a way I can change it to one for loop for that?

And now for @blablubbabc

I would love to do what you have said, but by the way you have explained it it sounds like your checking for tile entities in a chunk not in a radius, if I'm wrong I would really love to know, I'll test it in a seperate plugin in a few hours to see if I can squeeze a radius type thing out of it but I'm not exactly a very good programmer and as you can see by the class above, I still have lots to learn.

- LeePMC

 

That's why I said 'nearby/affected chunks': int chunkRadius = ((int) (radius / 16)) + 1;

Then you loop through the chunks in this chunk radius, and loop through the tile entities in those chunks, check if they are dispensers and if their location is in the exact radius (because the chunkRadius is usually slightly greater than the original radius).

 

@LeePMC that explains it a bit :/ i didnt know tht your plugin is requested by someone... :/ Sorri.
Btw does your name have to do anything with PMC (Planet Minecraft)?

 

@mine-care i wish my name had something to do with planet minecraft... But no it's just LeePlaysMC wasn't avalible so I took LeePMC, but now atleast I can prank servers xD