Trying to make a minecraft plugin here. I am trying to make a custom ore generator. The generator is cube, it has no exterior walls on the outside and the middle is filled with ores.
When I create a generator I don't save every single location object because every player can have multiple generator and that would be a massive amount of blocks in no time. What I do instead is that I save the min and max location for the border and for that inside cube(Why save both for border and cube? Well I'll explain in a bit) in a Object called CubeLocation that also stores the cubeId.
So I save the cubes in a HashMap<UUID, PlayerCube> - (This is not the id of a player, but i generate a random uuid when creating each code).
And I also save the Location in a HashSet <CubeLocation
Now for my problem. I need to check if the location of the broken block is inside a cube. What I have at the moment is something like this
public class LocationManager {
private HashMap<UUID, PlayerCube> cubes;
private HashSet<CubeLocation> cubeLocations;
private HashMap<UUID, PlayerHold> playerHolds;
private HashMap<UUID, Integer> tasks;
private HashMap<UUID, UUID> lastKnownGenerator;
public LocationManager() {
cubes = new HashMap<>();
cubeLocations = new HashSet<>();
playerHolds = new HashMap<>();
tasks = new HashMap<>();
lastKnownGenerator = new HashMap<>();
}
public HashMap<UUID, PlayerCube> getCubes(){
return cubes;
}
public HashSet<CubeLocation> getCubeLocations(){
return cubeLocations;
}
public HashMap<UUID, PlayerHold> getPlayerHolds(){
return playerHolds;
}
public HashMap<UUID, Integer> getTasks(){
return tasks;
}
public HashMap<UUID, UUID> getLastKnownGenerator(){
return lastKnownGenerator;
}
public String[] isLocationInsideCube(UUID player, Location loc) {
UUID world = loc.getWorld().getUID();
int x = loc.getBlockX();
int y = loc.getBlockY();
int z = loc.getBlockZ();
String arr[] = {null, null};
UUID last = lastKnownGenerator.get(player);
if(last != null) {
CubeLocation cubeLoc = cubes.get(last).getCubeLocation();
lastKnownGenerator.remove(player);
if(cubeLoc != null && cubeLoc.isInside(world, x, y, z)) {
if(cubeLoc.isCubeLocation(world, x, y, z)) {
arr[1] = last.toString();
return arr;
}
if(arr[0] == null && cubeLoc.isBorderLocation(world, x, y, z)) {
arr[0] = last.toString();
return arr;
}
}
}
for(CubeLocation cubeLoc : cubeLocations) {
if(!cubeLoc.isInside(world, x, y, z))
continue;
String id = cubeLoc.getCubeId().toString();
if(arr[1] == null && cubeLoc.isCubeLocation(world, x, y, z)) {
arr[1] = id;
break;
}
if(arr[0] == null && cubeLoc.isBorderLocation(world, x, y, z)) {
arr[0] = id;
break;
}
}
return arr;
}
}
And this is CubeLocation
public class CubeLocation {
private UUID world;
private int minX;
private int maxX;
private int minY;
private int maxY;
private int minZ;
private int maxZ;
private int bMinX;
private int bMaxX;
private int bMinY;
private int bMaxY;
private int bMinZ;
private int bMaxZ;
private UUID cubeId;
public CubeLocation(UUID world, UUID cubeId) {
this.world = world;
this.cubeId = cubeId;
}
public void setCoords(int x1, int x2, int y1, int y2, int z1, int z2) {
//Border
this.bMinX = x1;
this.bMaxX = x2;
this.bMinY = y1;
this.bMaxY = y2;
this.bMinZ = z1;
this.bMaxZ = z2;
//Cube
this.minX = x1 1;
this.maxX = x2 - 1;
this.minY = y1 1;
this.maxY = y2 - 1;
this.minZ = z1 1;
this.maxZ = z2 - 1;
}
public UUID getCubeId() {
return cubeId;
}
public boolean isInside(UUID world, int x, int y, int z) {
if(!world.equals(this.world))
return false;
return (x >= bMinX && x <= bMaxX) && (y >= bMinY && y <= bMaxY) && (z >= bMinZ && z <= bMaxZ);
}
public boolean isBorderLocation(UUID world, int x, int y, int z) {
if(!world.equals(this.world))
return false;
if(y == bMinY || y == bMaxY) {
return (x == bMinX && (z >= bMinZ || z <= bMaxZ)) || (x == bMaxX && (z >= bMinZ || z <= bMaxZ)) || (z == bMinZ && (x >= bMinX || x <= bMaxX)) || (z == bMaxZ && (x >= bMinX || x <= bMaxX));
}
return (x == bMinX && z == bMinZ) || (x == bMaxX && z == bMinZ) || (x == bMinX && z == bMaxZ) || (x == bMaxX && z == bMaxZ);
}
public boolean isCubeLocation(UUID world, int x, int y, int z) {
if(!world.equals(this.world))
return false;
return (x >= minX && x <= maxX) && (y >= minY && y <= maxY) && (z >= minZ && z <= maxZ);
}
}
This was also why I saved the border separate from the cube. Because I don't want the player to be able to break the border itself but also leave not take the walls into account(Anyways this isn't the problem but I wanted to explain that).
The problem is that every time a block is broken it will check each and every single CubeLocation to check if the location is a generator. That means that every time a block is broken it will do this check every single time (This is why I added the lastKnownGenerator, it'll first check this to see if the player is breaking blocks in the same generator so it doesn't have to do the loop again).
However this does not help if the player breaks blocks that are not inside a generator (For example in a mine, a house or idk) it will do the loop for every single broken block. And this is not even the worst part, the worst part is that BlockBreakEvent (The event that triggers when a player breaks a block) can be trigger dozens of times per second by a single player (If you have enchants on your pickaxe, shovel or axe) and not only that but it can be triggered by multiple players at once.
So image this, there have been like 30k cube placed on the world in total and there are like 30 players with full enchant minning 20 blocks per second each. The means that every second the plugin checks 600k coords per second. This won't do, it's too wasteful.
You also can't save each and every location separate or mark a block with block.setMetadata(key, FixedMetadataValue(plugin, value)); Because you would have to loop through (Let's say all the generator in our case are 11x11x11 which is 1331 per generator) 30.000 * 1331 which is a whopping 39.930.000 blocks so it's even worse than what I'm doing.
What would be the appropriate method to save the cubes and also check if the broken block is inside a cube. Am I completely wrong with what I was trying to do? Any and all suggestion are welcome!
Thanks for reading!
CodePudding user response:
I suggest you to do multiple things :
- Use less variable. It will not directly fix the amount of items, but it will help in performance. It's also mostly a suggestion than will make the code quickly more readable. For example, you can add position of a center, then store the size. So you require only 5 variables:
public int centerX, centerY, centerZ;
public int sizeBorder, sizeCube;
With these, if you have 30k instance, you will economise 7*30000 so 210k var.
- Use chunk. This can really help you. The objective is to store cube according to chunk.
Firstly, you need to get chunk X/Z :
public static getChunkX(Location loc) {
rreturn floor(loc.getX()) >> 4;
}
/**
* NumberConversions#floor(double), used by Location#getBlockX(), Location#getBlockZ()
*
* @param num the number to floor
* @return the floored number
*/
public static int floor(double num) {
int floor = (int) num;
return floor == num ? floor : floor - (int) (Double.doubleToRawLongBits(num) >>> 63);
}
(Source)
Now, you should declare the variable that will contains everything:
HashMap<Integer, HashMap<Integer, CubeLocation>> cubePerChunk = new HashMap<>();
It's a table of table of an array. You can refer to this if you want to learn more.
Location myLoc = ...;
List<CubeLocation> allCubesInChunk = cubePerChunk.get(getChunkX(myLoc)).get(getChunkZ(myLoc));
Now, you can do what you did with all cube location, but instead of a list of 30k items you will have only item in this chunk, so less than 5 one.
To store values, you should get chunk of 4 points and try to add it in all list. If all points are in the same chunk, you will have the items stored only one time, else it will be available from all chunk where it is. Example of code:
public void loadCube(CubeLocation loadedCube) {
for(Location loc : loadedCube.getAllXZPoints()) {
List<CubeLocation> allCubesInChunk = cubePerChunk.computeIfAbsent(getChunkX(loc), HashMap::new)
.computeIfAbsent(getChunkZ(loc), ArrayList::new);
if(!allCubesInChunk.contains(loadedCube)) // if not already added
allCubesInChunk.add(loadedCube); // add it
}
}
Just, about the getAllPoints()
method :
It's just something that return all points without taking in count Y, such as:
public List<Location> getAllXZPoints() {
// w is your world
return Arrays.asList(new Location(w, minX, minY, minZ), new Location(w, maxX, minY, minZ),
new Location(w, minX, minY, maxZ), new Location(w, minX, minY, maxZ));
}
CodePudding user response:
Some more tips for you dmf, when you're looking for performance here you have two points you can work on, which is memory optimization, basically you're going to use less memory, which is basically what Elikill already told you up here ^^.
The second thing is you use NMS, since you're setting (i believe, a lot of blocks), its best if you access the deepest nms method you can access, you can find some tutorials on the main spigot website on how to place blocks fast(or you could go and look at FAWE src).
Another thing is, to check if the block broken is inside of the cube all you have todo is get the maximum point of the cube, get the minimum point of the cube, and check if the x of the block is <= max x, and the same process, for all the coordinates, x y z.
Please do not call .getAllPoints() to just check if the block is inside of the cube, that would be the worst scenario possible you could write.