Solved Function chaining error

Hi.
I'm using ArrayLists to store locations, but when I register a lot of locations very fast, a lot of them are skipped : only the first ones are registered.
Is anyone knowing a way to force the registration of them ?
Btw my code is flawless.

 

That is very unlikely, unless you've found a Java bug. Which you haven't. What's your code?

 

Code:

    @SuppressWarnings("deprecation")
    private List<Location> BlockRenewer(final Block start,final Block block, final BlockState Active, final BlockState Inactive, final int duration, final int Distance, final boolean area, final double wavespeed, final String waveuid, final boolean Explode, final int ExplosionForce, final Player Larbin, final boolean zone){
        List<Location> nr = new ArrayList<>(); //Creating the Arraylist : If the tests aren't sucessful it'll be empty
        final World blockworld = block.getWorld();
        final Location blocklocation = block.getLocation();
        if (zone) { //Test to see if the function is called after being pre-rendered
            main.Debug("!!! ZONE !!!");
        }
      
        if (area) { //Always true in our case
            if (!block.hasMetadata(waveuid)) { //A UUID to force the plugin to not loop between block
                boolean colorsuccess = false; //Test to see if the block at the location corresponds with the starting block
                if (block.getType() == Active.getType()) {
                    if (Active.getRawData() != (byte) 0 && Inactive.getRawData() != (byte) 0) {
                        if (block.getData() == Active.getRawData()) {
                            colorsuccess = true;
                        }else if (block.hasMetadata(main.mColor)) {
                            if (block.getMetadata(main.mColor).get(0).asByte() == Active.getRawData()) {
                                colorsuccess = true;
                            }
                        }
                    } else {
                        colorsuccess = true;
                    }
                }
                boolean Reloadable = false;
                if (block.getType() == main.WorldUtils.getInfo(MatInfo.Colored, blockworld) || block.getType() == main.WorldUtils.getInfo(MatInfo.LandMine, blockworld)) {
                    Reloadable = true;
                }
                if (Reloadable && colorsuccess && block.hasMetadata(main.mInactive) == false) { //Tests if the previous tests have been passed and the block isn't inactive. For the simplicity, let's say that it's passed
                  //All the following is purely animations
                    int truedelay = (int) (Math.round(start.getLocation().distance(blocklocation) * wavespeed)); //Calculation of the delay for the block to be reloaded.
                  
                    new BukkitRunnable() {
                      
                        @Override
                        public void run() {
                            block.setMetadata(main.mRUUID, new FixedMetadataValue(plugin, waveuid));
                            BlockReload(duration, Distance, block, Active, Inactive, waveuid);
                            if (Explode) {
                                main.Explosions.SimulateExplosion(block.getLocation().add(0.5, 0.5, 0.5), ExplosionForce, 0.5, 1, false, Larbin, true);
                            }
                        }
                    }.runTaskLater(plugin, truedelay);
                  
                    block.setMetadata(waveuid, new FixedMetadataValue(plugin, true)); //Setting the anti-loop metadata
                  
                    new BukkitRunnable() {
                      
                        @Override
                        public void run() {
                            block.removeMetadata(waveuid, plugin);
                          
                        }
                    }.runTaskLater(plugin, 1200);
                    if (!zone) { //If zone is false, in the case of pre-rendering it
                        nr.add(blocklocation); //Adds itself in the List
                        for (Location loc : getNearBlocks(block.getLocation(), getType.PLANE)) { //Then it runs the same function all around him, and retrieves the locations
                            List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                            if (!tempz.isEmpty()) { //Test if the function was successful or not, it may be not if the block tested isn't the same as the first block or the block as already been processed
                                nr.addAll(tempz); //Adds all the values of the function into this one
                            }
                        }
                      
                        if (!main.WorldUtils.getInfo(BooleanInfo.Planear, blockworld)) { //Same thing, but into two other plans
                          
                            for (Location loc : getNearBlocks(block.getLocation(), getType.NONPLANE)) {
                                BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                if (!tempz.isEmpty()) {
                                    nr.addAll(tempz);
                                }
                            }
                        }
                    }
                }
            }
        } else {
            String reloadUUID = UUID.randomUUID().toString();
            block.setMetadata(main.mRUUID, new FixedMetadataValue(plugin, reloadUUID));
            BlockReload(duration, Distance, block, Active, Inactive, reloadUUID);
        }
        return nr; //Then it return what he got.
    }
    

Then nr is stored inside a special class, and then it tells me wich blocks were inside. For small numbers of locations, it works fine, but if there is too much it struggles.

 

@Betagear
Well, without studying your code for a really long time, the only thing that stands out is that you don't seem to be adding anything at all if "zone" is true. But I assume you know that.

If I were you, I'd try to narrow down some reproduction steps, find out exactly what makes the problem happen. Is it a certain number of locations? Maybe it happens when a certain variable or condition is true? Is there any case where you do the exact same thing twice, and get different results? etc.

 

@adam753 "zone" is used after the locations have stored, to rerun the thing with only the stored locations, saving some ressources

It happens after more than 50 locations stored. As I register the locations in this order (+x, +z, -x, -z), the locations are all stored in the direction +x and +z, as they are the first to come. So, it depends on the starting location.

As you may have seen, it is maybe in the way I create the list that makes a problem.

 

@Betagear

I mean, "flawless" is not the word I would use.

It is an error in your code if all the Locations are not being added. It is not the fault of the ArrayList. That much we know. So now that we know the error is in your code, try debugging your code.

You're using a lot of class-level variables where they really shouldn't be being used. This makes code extremely hard to reason about, which is probably why you're having issues figuring out where the problem is in your code. Reduce the over usage of class level variables, split that method apart, reduce the number of parameters, and format your code better. Then you might have a fighting chance of understanding your issue(s).

 

@mythbusterma Ok. All the variables are used to block the function from looping between blocks and selecting only a type of block. Somes are used in order to make an animation.
The only part that may interest you is from the line 57 to 75.
I'll add comments.

 

@Betagear

Is this function recursive? If that's the case, you really need to fix this. This code is atrocious.

 

For curiousity's sake. Why is it bad to have a recursive method? Or are you just referring to this context?

@Betagear
Can you try refactoring your code into smaller methods/classes? May help you catch the error, or at least understand better where it goes wrong. As @mythbusterma said, makes it easier to reason about.

 

@BreezerFly

It's not necessarily bad, but I don't see any base case here, and not only that, there's no reason for something like this to be recursive, and a recursive function should be extremely short and easy to reason about, this one is neither.

It's also just a general rule to prefer iteration over recursion, where both are practical.

 

@mythbusterma
Woop, agreed. Was just curious, I like using recursive functions to make code easier to reason about.
And yeah, iteration is generally faster as well, right?

 

Here it is :

Code:

    private List<Location> BlockRenewer(final Block start,final Block block, final BlockState Active, final BlockState Inactive, final int duration, final int Distance, final boolean area, final double wavespeed, final String waveuid, final boolean Explode, final int ExplosionForce, final Player Larbin, final boolean zone){
        List<Location> nr = new ArrayList<>();
        final World blockworld = block.getWorld();
        final Location blocklocation = block.getLocation();
        if (zone) {
            main.Debug("!!! ZONE !!!");
        }
       
        if (area) {
            if (!block.hasMetadata(waveuid)) {
                boolean Reloadable = false;
                if (block.getType() == main.WorldUtils.getInfo(MatInfo.Colored, blockworld) || block.getType() == main.WorldUtils.getInfo(MatInfo.LandMine, blockworld)) {
                    Reloadable = true;
                }
                if (Reloadable && isOk(block, Active) && block.hasMetadata(main.mInactive) == false) {
                   
                    setReloadingFlags(waveuid, block, start, Active, Inactive, wavespeed, duration, Distance, Explode, ExplosionForce, Larbin);
                   
                    if (!zone) {
                        nr.add(blocklocation);
                        for (Location loc : getNearBlocks(block.getLocation(), getType.PLANE)) {
                            List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                            if (!tempz.isEmpty()) {
                                nr.addAll(tempz);
                            }
                        }
                       
                        if (!main.WorldUtils.getInfo(BooleanInfo.Planear, blockworld)) {
                           
                            for (Location loc : getNearBlocks(block.getLocation(), getType.NONPLANE)) {
                                BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                if (!tempz.isEmpty()) {
                                    nr.addAll(tempz);
                                }
                            }
                        }
                    }
                }
            }
        } else {
            String reloadUUID = UUID.randomUUID().toString();
            block.setMetadata(main.mRUUID, new FixedMetadataValue(plugin, reloadUUID));
            BlockReload(duration, Distance, block, Active, Inactive, reloadUUID);
        }
        return nr;
    }

 

Still not any idea ? The error isn't coming from my code, as you may have seen it.
(kind of a bump)

 

@Betagear

The error is definitely in your code, and your code is impossible to reason about. We can't help you other than to say debug and rearrange your code so that it is possible to read.

Code should never look like this for this exact reason. You can't figure out where your error is.

Also of note, the nondescript method names and static fields from other classes compound the difficulty.

 

@mythbusterma Just look at that :

Code:

if (!zone) {
                        nr.add(blocklocation);
                        for (Location loc : getNearBlocks(block.getLocation(), getType.PLANE)) {
                            List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                            if (!tempz.isEmpty()) {
                                nr.addAll(tempz);
                            }
                        }
                      
                        if (!main.WorldUtils.getInfo(BooleanInfo.Planear, blockworld)) {
                          
                            for (Location loc : getNearBlocks(block.getLocation(), getType.NONPLANE)) {
                                BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                List<Location> tempz = BlockRenewer(start, loc.getBlock(), Active, Inactive, duration, Distance, area, wavespeed, waveuid, Explode, ExplosionForce, Larbin, zone);
                                if (!tempz.isEmpty()) {
                                    nr.addAll(tempz);
                                }
                            }
                        }
                    }

This code is always passed, but after a certain amount of times, it stops working.
What should I do ? should I create a class for it and stop getting a lot of result remashed into the previous class to be then sent to the first one ?

 

You need to split it up into methods and then add comments so that we know what your trying to do. Also need to see the methods you made that you're calling in this method.

 

@BreezerFly Error busted : Used a class with a dedicated List instead of chaining a lot of times the functions, and it worked.
This error was due to the chaining of the same function a lot of times, always adding more locations to a list, wich was then sent to the parent.

 

Yeah, as we said further up, you should make it an iterative function instead of a recursive one