my spawn protection is hit and miss

Discussion in 'Plugin Development' started by d3x, Apr 6, 2011.

Thread Status:
Not open for further replies.
  1. Offline

    d3x

    I wrote a basic plugin to protect 256 blocks around my spawn located at 0,0 and it works pretty good but for some reason people can randomly (rarely) hit someone once or twice, and place or break a block. It's not in any specific area and their ability to do anything is short lived. Could someone look over the code for me and see if they can spot the problem?

    SpawnProtect
    BlockListener
    EntityListener
    PlayerListener
     
  2. Offline

    Sammy

    It may be lag spikes, why don't you just do a return after the each if, instead of doing all the if's every time something happens

    Code:
                if (x > 0 && x < plugin.border) {
                   return;
                }
                if (z > 0 && z < plugin.border) {
                    return;
                }
                if (x < 0 && x > -plugin.border) {
                    i++;
                }
                if (z < 0 && z > -plugin.border) {
                    return;
                }
                else{
                    event.setCancelled(true);
                }
    
    Sorry If I'm misunderstanding your code.
     
  3. Offline

    d3x

    I think you're misunderstanding code in general. :p
     
  4. Offline

    Carnes

    I think Sammy is right. Also, i found your problem.

    Code:
        if (i > 1) {
            if (plugin.allow.containsKey(event.getPlayer().getName())) {
                return;
            }
            event.setCancelled(true);
        }
    
    If they are aligned with an axis when they place a block, i==1 and it will allow them to place a block in the restricted area. You could change it to i>0 or use Sammy's method. This same oversight allows them to build on block 0,0, but Sammy's return method would fix that too.
     
  5. Offline

    Sammy

    You try 4 ifs and if they are true you add i++ on each true one
    On the 5th if you see if there was a addition and cancel the event
    Am I right ?

    The way you are doing it is inefficient imo because you always do 4 checks even if you already have too cancel the event from the 1st if.

    PS: I didn't saw if my way would work, I just changed it on the fly so you would get my idea ;)
     
  6. Offline

    d3x

    @Carnes Good eye!

    Code:
            int x = event.getBlock().getX();
            int z = event.getBlock().getZ();
            int i = 0;
            if (x >= 0 && x < plugin.border) {
                i++;
            }
            if (z >= 0 && z < plugin.border) {
                i++;
            }
            if (x < 0 && x > -plugin.border) {
                i++;
            }
            if (z < 0 && z > -plugin.border) {
                i++;
            }
            if (i > 1) {
                if (plugin.allow.containsKey(event.getPlayer().getName())) {
                    return;
                }
                event.setCancelled(true);
            }
    Seems to work better.

    @Sammy I would like to see you make more efficient code and benchmark it against mine. I mean you could do something silly like

    Code:
            int x = event.getBlock().getX();
            int z = event.getBlock().getZ();
            int i = 0;
            if (x >= 0 && x < plugin.border) {
                i++;
            }
            if (z >= 0 && z < plugin.border) {
                i++;
            }
            if (i < 2 && x < 0 && x > -plugin.border) {
                i++;
            }
            if (i < 2 && z < 0 && z > -plugin.border) {
                i++;
            }
            if (i > 1) {
                if (plugin.allow.containsKey(event.getPlayer().getName())) {
                    return;
                }
                event.setCancelled(true);
            }
    which would save the second ifs from being executed half the time but the other half there would be the extra overhead of the i checks and in the end it would probably not pan out. It's not like the ifs are CPU insensitive it's basic math.
     
  7. Offline

    Sammy

    First of I'm not here to measure code skills I'm here to HELP
    The idea is to "return" every time you get a x,y,z<plugin.border, and not doing all the other if's because they are redundant (if you aren't understanding me never mind just forget it).
    And the "basic math" is almost exponential with a lot of players on the server...
     
  8. Offline

    d3x

    @Sammy

    I wasn't asking you to write it your way and benchmark it to measure skill, I asking it so
    1) I could see what you mean; your code won't even compile so its hard to understand what you mean.
    2) you could back up your claim of mine being inefficient/redundant If there is room for improvement in my code I would love to see it. I would pay to see it. Would you like money to take the time to explain it to me?

    also its not exponential but multiplied. 50 users means the code runs times 50 not to the power of 50. But I do understand your point as the very reason I am writing this plugin is to be as efficient unlike the alternative already available plugins. I have a load average of 40-50 users so most plugins cause me quite a bit of lag.
     
  9. Offline

    Sammy

    The influence of players is EXPONENTIAL because every player multiplies the "onPlace" and "onRemove" events a huge amount of times... but I'm here to help with code not math, I suggest that you go brush up your knowledge on exponential distributions ;)

    But please PLEASE don't patronize me, I'm not obligated to help you
     
  10. Offline

    d3x

    @Sammy

    I don't understand why you would think every user places and breaks twice as many blocks as the last person. Also I am aware you're not obligated to help me that is exactly why I offered you money.

    Edit: I have been trying to think if ways to change it up and all I can come up with is:
    Code:
            if (x >= 0 && x < plugin.border) {
                i++;
            }
            else if (x > -plugin.border) {
                i++;
            }
            if (z >= 0 && z < plugin.border) {
                i++;
            }
            else if (z > -plugin.border) {
                i++;
            }
    now roughly half the time only 2 if's will fire :p
     
  11. Offline

    Sammy

    @d3x
    Thanks but I'm not here for the money :)

    "I don't understand why you would think every user places and breaks twice as many blocks as the last person"
    Well because the more players you have the more interactions tend to occur (players become more active).

    My idea is to try something like this:

    Code:
                if (x > 0 && x < plugin.border) {
                    event.setCancelled(true);
                } else if (z > 0 && z < plugin.border) {
                    event.setCancelled(true);
                } else if (x < 0 && x > -plugin.border) {
                    event.setCancelled(true);
                } else if (z < 0 && z > -plugin.border) {
                    event.setCancelled(true);
                } else {
                   return;
                }
    I'm doing this by heart so it maybe wrong but try to follow me:
    You do this:
    is x out in border? YES/NO
    is -x out in border? YES/NO
    is Z out in border? YES/NO
    is -Z out in border? YES/NO

    Only AFTER doing all the 4 ifs you cancel or not the event, why not:
    Cancel the event if one is of borders

    This way if "is x out in border? YES" you don't need to do the other 3 ifs, you just cancel the event and break the method with a return
     
  12. Offline

    d3x

    Thank you for making me think so hard on this one I just realized all I need is:

    Code:
    if (-border< x && x < border&& -border< z && z < border) {
        event.setCancelled(true);
    }

    Edit: simplified!

    Code:
    if (Math.abs(x) < border && Math.abs(z) < border) {
        event.setCancelled(true);
    }
    Thanks again!
     
  13. Offline

    Sammy

    Sorry if I was to harsh, the internet is so full of cynicism and sometimes is hard to understand a persons motivation :)

    Back to the code:
    Code:
    if (Math.abs(x) < border && Math.abs(z) < border) {
        event.setCancelled(true);
    }
    Shouldn't it be an OR (||) ? because x maybe off border but z not or vice versa
     
  14. Offline

    d3x

    if it was OR they could be at -15000, 10 and would still not be able to build.
     
Thread Status:
Not open for further replies.

Share This Page