-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] handle multi subregion litematica schematics returning null blocks #4363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placeAt
only queries the block after making sure it is in bounds so a null state at this point is a bug in LitematicaSchematic
.
Also logDirect
should be used for direct feedback and important things like error messages, not for borderline spam which can trigger 20 times per second.
Background and reproductionTry to build this litematica schematic (which has 2 subregions) using baritone 1.10.2 under minecraft 1.20.4: try and build with baritone
It crashes with null state for block error...
|
So it was a while back when I looked at this, but the underlying issue is that baritone is returning true if the block is inside the extends of the schematic (assuming it is a rectangular cuboid) but the litematica schematic only defines the blocks for those subregions... and you can see from the image, that there are gaps between the minX and maxX etc |
I wasn't saying that this doesn't happen, I was saying that you are fixing the wrong file. Just like you said yourself, |
Ah, I justed tested it... and yeah it doesn't fix the problem. So I will revisit. |
Any suggestions on how to get litematica mod running under the gradle debug build in intellij? dropping litematica.jar into the mods folder is not working |
@Override | ||
public boolean inSchematic(int x, int y, int z, BlockState currentState) { | ||
// System.out.println("called inSchematic: " + x + "," + y + "," + z + " state: " + currentState); | ||
for (String subReg : getRegions(nbt)) { | ||
|
||
Vec3i offsetSubregionMin = new Vec3i(getMinOfSubregion(nbt, subReg, "x"), getMinOfSubregion(nbt, subReg, "y"), getMinOfSubregion(nbt, subReg, "z")); | ||
|
||
if (inSubregion(nbt, subReg, x + this.offsetMinCorner.getX(), y + offsetMinCorner.getY(), z + offsetMinCorner.getZ())) { | ||
//System.out.println("found " + x + "," + y + "," + z + " in subregion: " + subReg + " state: " + currentState); | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the default method needs a override however i have a few questions.
What is offsetSubregionMin doing? What happens if the schematic is rotated 90 degree? If im not mistaken the inSubregion() methods checks against the file in which the schematic isnt rotated.
Why not use
@Override
public boolean inSchematic(int x, int y, int z, BlockState currentState) {
return getDirect(x, y, z) != null
}
@@ -39,7 +39,7 @@ | |||
* @author rycbar | |||
* @since 22.09.2022 | |||
*/ | |||
public final class LitematicaSchematic extends StaticSchematic { | |||
public class LitematicaSchematic extends StaticSchematic { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you change all the method signatures? is it for your mock test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. to mock the class to avoid having to create the BuiltInRegistries stuff
if (inSubregion(nbt, subReg, x, y, z)) { | ||
this.states[x - (offsetMinCorner.getX() - offsetSubregion.getX())][z - (offsetMinCorner.getZ() - offsetSubregion.getZ())][y - (offsetMinCorner.getY() - offsetSubregion.getY())] = blockList[bitArray.getAt(index)]; | ||
if (inSubregion(nbt, subReg, x + this.offsetMinCorner.getX(), y + offsetMinCorner.getY(), z + offsetMinCorner.getZ())) { | ||
this.states[x][z][y] = blockList[bitArray.getAt(index)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont remember why exactly i did it this way but im sure there was a reason for it.
int index = 0; | ||
for (int y = 0; y < this.y; y++) { | ||
for (int z = 0; z < this.z; z++) { | ||
for (int x = 0; x < this.x; x++) { | ||
if (inSubregion(nbt, subReg, x, y, z)) { | ||
this.states[x - (offsetMinCorner.getX() - offsetSubregion.getX())][z - (offsetMinCorner.getZ() - offsetSubregion.getZ())][y - (offsetMinCorner.getY() - offsetSubregion.getY())] = blockList[bitArray.getAt(index)]; | ||
if (inSubregion(nbt, subReg, x + this.offsetMinCorner.getX(), y + offsetMinCorner.getY(), z + offsetMinCorner.getZ())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this counteracts the changes in the next line in some way but i dont know how
|
||
CompoundTag region = nbt.getCompound("Regions").getCompound(subReg); | ||
|
||
int xSize = region.getCompound("Size").getInt("x"); | ||
int ySize = region.getCompound("Size").getInt("y"); | ||
int zSize = region.getCompound("Size").getInt("z"); | ||
|
||
int xPos = region.getCompound("Position").getInt("x"); | ||
int yPos = region.getCompound("Position").getInt("y"); | ||
int zPos = region.getCompound("Position").getInt("z"); | ||
|
||
int xMin = (xSize >= 0) ? xPos : xPos + xSize + 1; | ||
int yMin = (ySize >= 0) ? yPos : yPos + ySize + 1; | ||
int zMin = (zSize >= 0) ? zPos : zPos + zSize + 1; | ||
|
||
int xMax = xMin + Math.abs(xSize); | ||
int yMax = yMin + Math.abs(ySize); | ||
int zMax = zMin + Math.abs(zSize); | ||
|
||
boolean withinX = xSize != 0 && x >= xMin && x < xMax; | ||
boolean withinY = ySize != 0 && y >= yMin && y < yMax; | ||
boolean withinZ = zSize != 0 && z >= zMin && z < zMax; | ||
|
||
boolean found = withinX && withinY && withinZ; | ||
|
||
return found; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit lengthy but not wrong as far as i can tell.
the xSize != 0 check is redundant (if size is 0, xMax = xMin and x cant be >= and < to the same number)
Thanks for the comments! I will address them. I opened the PR because I thought it was a simple fix, but it seems quite involved. (e.g. it seems to break rotated and mirrored multiple subregions...) I think I will close the PR until I am more confident about how this might interact with the other stuff in LitematicaHelper |
If the litematica schematic has multiple subregions that don't correspond to the full extent of the schematic, then "state" is null so the build crashes.