0

Ok so, I have a method here that when the Entity hits the ground it saves the Location and the Block inside of a HashMap defined like so:

public static HashMap<Location, Material> blocks = new HashMap<Location, Material>();

Then a few ticks later on the server it sets the block back to original type, rather than doing this at the moment its just turning into a null block.

@EventHandler
public void onProjecitle(ProjectileHitEvent event){
    if(event.getEntity().getType() == EntityType.ENDER_PEARL){
        Block block = event.getEntity().getLocation().getBlock();
        for (int z = -1; z <= 1; z++) {
            for (int x = -1; x <= 1; x++) {
                for (int y = -1; y <= 1; y++) {
                    final Location loc = block.getRelative(x, y, z).getLocation();
                    if(loc.getBlock().getType() != Material.AIR){
                        Aure.blocks.put(loc.getBlock().getLocation(), block.getType());
                        loc.getBlock().setType(Material.WOOL);
                        Bukkit.getScheduler().runTaskLater(plugin, new Runnable(){
                            @Override
                            public void run() {
                                loc.getBlock().setType(Aure.blocks.get(loc.getBlock().getLocation()));
                            }
                        }, 40L);
                    }
                }
            }
        }
    }
}
John
  • 131
  • 2
  • 15
  • shorter code != better code. – Jared Apr 10 '14 at 00:57
  • 1
    @Jared This code isn't short... It's what's needed, no more, no less... – Jojodmo Apr 10 '14 at 01:30
  • @jojodmo the code shouldn't be this nested. That's ugly code. It should be broken into methods. You can argue performance but modern compilers make that argument largely moot. – Jared Apr 10 '14 at 01:31
  • @Jared Yeah, I could probably sort into methods i just wanted to get something that works first over and done with and then head over onto getting them into methods. – John Apr 10 '14 at 01:33
  • 1
    @Jared Personally, this is probably the last thing I would put in a method, because you don't really need to use this anywhere else... Also, I think it makes it easier to read when it's nested. I would agree, though, about splitting this into methods if you needed to use this code elsewhere, but it looks like the OP wouldn't need to... I guess it's just a matter of programming preference... – Jojodmo Apr 10 '14 at 01:38
  • @jojodmo I'll be awaiting an answer to the question then (if it's so easy to read). What I see is an extremely nested method with no explanation of either what the method is supposed to do or what the nesting is supposedly accomplishing (not to say that I couldn't statically parse the method to figure out what is going on--I just don't want to do that). – Jared Apr 10 '14 at 01:41
  • @Jared There you go :) Again, it's just programming preference. – Jojodmo Apr 10 '14 at 01:53

2 Answers2

1

Try this:

Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable(){

instead of this:

Bukkit.getScheduler().runTaskLater(plugin, new Runnable(){

runTaskLater is actually scheduling an Asynchronous task, you should try to always use Synchronous tasks.

The problem with using Asynchronous tasks with Bukkit is your task will be scheduled on another thread, not being in sync with the normal server. Making it so that any Bukkit code that you use is VERY unsafe, so, as a general rule of thumb, whenever you're dealing with Bukkit, you should always use Synchronous tasks, which will be run on the same thread as your Main file. So, here's what your scheduled task should look like:

Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable(){
    public void run(){
        loc.getBlock().setType(Aure.blocks.get(loc)); //no need to use loc.getBlock().getLocation(), same thing as just using loc
    }
}, 40L);

Same rule applies when trying to create a repeating task, you should use scheduleSyncRepeatingTask.

Jojodmo
  • 23,357
  • 13
  • 65
  • 107
  • Its not saving the type of the block, I broadcast its stype and it always is being saved as AIR – John Apr 10 '14 at 03:02
  • @Twipply Are you trying the code I suggested? Because if not it probably has to do with you using an `ASync` task, which is **VERY** unstable if you're calling any Bukkit methods, so that's probably why it's being set to the null block. – Jojodmo Apr 10 '14 at 03:51
1

The problem about your code is that you get the block the player's feet are in, so its Material is obviously Material.AIR. You need to get the block below the player, so you should use

Location theRealBlock = event.getEntity().getLocation().substract(0.0D, 1.0D, 0.0D);

instead of

event.getEntity().getLocation();
Jojodmo
  • 23,357
  • 13
  • 65
  • 107
mezzodrinker
  • 998
  • 10
  • 28