[Solved] [1.14.4] Trouble converting Iterable to ArrayList

[Croa]

I am updating my code from 1.12.2 to 1.14.4 and I am having trouble with converting BlockPos.getAllInBoxMutable() to an ArrayList. My issue is that the last BlockPos in the Iterable is copied to all parts of the array instead of each to their own. 

My code and output are here:

CODE:
ArrayList<BlockPos> list = new ArrayList<BlockPos>();
Iterable<BlockPos> blocks = BlockPos.getAllInBoxMutable(new MutableBlockPos(0,0,0),new MutableBlockPos(2,0,0));
Iterable<BlockPos> iterable = Arrays.asList(new MutableBlockPos(0,0,0),new MutableBlockPos(1,0,0),new MutableBlockPos(2,0,0));

if(!ZBuild.doneSelection) {
	try {
		blocks.forEach(x->System.out.println(x));
		blocks.forEach(x->list.add(x));
		System.out.println(list+"\n\n");
		list.clear();
		iterable.forEach(x->System.out.println(x));
		iterable.forEach(x->list.add(x));
		System.out.println(list);
	}catch(Exception e) {
		System.out.println(e);
	}
	ZBuild.doneSelection=true;
}
======================================================================================================

OUTPUT:
MutableBlockPos{x=0, y=0, z=0}
MutableBlockPos{x=1, y=0, z=0}
MutableBlockPos{x=2, y=0, z=0}
[MutableBlockPos{x=2, y=0, z=0}, MutableBlockPos{x=2, y=0, z=0}, MutableBlockPos{x=2, y=0, z=0}]


MutableBlockPos{x=0, y=0, z=0}
MutableBlockPos{x=1, y=0, z=0}
MutableBlockPos{x=2, y=0, z=0}
[MutableBlockPos{x=0, y=0, z=0}, MutableBlockPos{x=1, y=0, z=0}, MutableBlockPos{x=2, y=0, z=0}]

 

Some of the solutions I have tried so far include:

• a for loop that adds each BlockPos individually
• changing getAllInBoxMutable to getAllInBox and converting the stream into an array
• changing all MutableBlockPos into BlockPos
• commenting out all code in the class that pertains to any of the variables

 

Everything so far has yielded the same results and I am not quite sure why. Any help would be appreciated.

Edited January 7, 2020 by Croa
Issue Solved

[DavidM]

Because getAllInBoxMutable returns the same MutableBlockPos every iteration step.

With your current method, you are basically filling an array with the same object.

To achieve what you want, simply create a new BlockPos (which is immutable) per iteration step based of the value from the iterator, then store the newly created BlockPos in your array.

 

30 minutes ago, Croa said:

try

Also don’t randomly add try blocks when there is no error to catch.

[Cadiboo]

4 hours ago, DavidM said:

To achieve what you want, simply create a new BlockPos (which is immutable) per iteration step based of the value from the iterator, then store the newly created BlockPos in your array.

Use BlockPos#toImmutable().

[Croa]

Thanks for the assistance. The try block was to temporarily prevent the game from crashing while I toyed around with all of my solutions. For example, I forgot that I closed a stream and tried to use it again resulting in a notification rather than a crash. Anyway, my final code is here if anyone runs into the same issue:

public static ArrayList<BlockPos> Selected = new ArrayList<BlockPos>();
public static BlockPos blockBuildpos1,blockBuildpos2;
  
  //the rest of my class
  
Iterable<BlockPos> blocks = BlockPos.getAllInBoxMutable(blockBuildpos1,blockBuildpos2);
        	if(!ZBuild.doneSelection) {
        			for(BlockPos pos:blocks) {
        				BlockPos temp = new BlockPos(pos);
        				Selected.add(temp.toImmutable());
        			}
	            ZBuild.doneSelection=true;
        	}

 

[Draco18s]

So.

Question.

Why are you asking for an interable of mutable positions if you're converting it to a list of immutable positions? BlockPos.getAllInBox(...) already returns an itterable of immutable positions. You've literally gone out of your way to ask for a mutable list and then gone and turned it back into an immutable one when you could have requested an immutable list in the first place.

 

Related:

Quote

BlockPos temp = new BlockPos(pos);

This does nothing useful. You could have just done Selected.add(pos.toImmutable()).

[Croa]

Draco18s,

Thanks for the pointer. My mind just wanted to create a new BlockPos as the previous comments suggested. Selected.add(pos.toImmutable()); is now what I have in the for loop

 

[Draco18s]

And the other half of my post?!

You still haven't explained why you're going out of your way to get mutable objects only to make them immutable when getAllInBox already returns an immutable list. 

[Croa]

Draco,

It is easier (with my current knowledge) to convert the Iterable that getAllInBoxMutable returns to a Collection than the Stream that getAllInBox returns. The rest of the class requires a Collection to operate as intended the way it is currently written. This part of the mod will copy/paste/cut/etc areas in the world with individual button presses. I am sure that this is inefficient, but I will don't want to ask people to code my mod for me unless it is absolutely necessary.

[Croa]

diesieben07,

I am aware how to convert the Stream into a Collection. I took a look at that previously and could not avoid the original problem of each array slot being filled with the last BlockPos in the stream. getAllInBox appears to return MutableBlockPos according to the console. The only way so far I have successfully received correct coordinates to the Collection is with my previous code

CODE:
Stream<BlockPos> b1 = BlockPos.getAllInBox(new BlockPos(0,0,0),new BlockPos(3,0,0));
if(!ZBuild.doneSelection) {
	List<BlockPos> result = (b1.collect(Collectors.toList()));
  	result.forEach(System.out::println);
  	ZBuild.doneSelection=true;
}
  
OUTPUT:
  MutableBlockPos{x=3, y=0, z=0}
  MutableBlockPos{x=3, y=0, z=0}
  MutableBlockPos{x=3, y=0, z=0}
  MutableBlockPos{x=3, y=0, z=0}
  

 

[Cadiboo]

BlockPos#getAllInBox does return a mutable block pos. You can greatly simplify your code by using Streams.

Something like

List<BlockPos> allInBox = BlockPos.getAllInBox(startPos, endPos)
  	.map(BlockPos::toImmutable)
  	.collect(Collectors.toList());