Skip to content

Commit

Permalink
change: make BlockEntityList be more similar to the vanilla implement…
Browse files Browse the repository at this point in the history 
…ation
  • Loading branch information
@2No2Name
2No2Name committed Sep 18, 2020
1 parent 15487bc commit bd89303
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 65 deletions.
175 changes: 116 additions & 59 deletions src/main/java/me/jellysquid/mods/lithium/common/util/collections/BlockEntityList.java
View file
Original file line number Diff line number Diff line change
@@ -1,91 +1,117 @@
package me.jellysquid.mods.lithium.common.util.collections;

import it.unimi.dsi.fastutil.longs.Long2ReferenceLinkedOpenHashMap;
import it.unimi.dsi.fastutil.longs.Long2ReferenceOpenHashMap;
import it.unimi.dsi.fastutil.objects.ReferenceLinkedOpenHashSet;
import net.minecraft.block.entity.BlockEntity;
import net.minecraft.util.math.BlockPos;

import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.*;

public class BlockEntityList implements List<BlockEntity> {
private final Long2ReferenceLinkedOpenHashMap<BlockEntity> map = new Long2ReferenceLinkedOpenHashMap<>();
//BlockEntityList does not support double-add of the same object. But it does support multiple at the same position.
//This collection behaves like a set with insertion order. It also provides a position->blockEntity lookup.

private final ReferenceLinkedOpenHashSet<BlockEntity> allBlockEntities;

//When there is only 1 BlockEntity at a position, it is stored in posMap.
//When there are multiple at a position, the first added is stored in posMap
//and all of them are stored in posMapMulti using a List (in the order they were added)
private final Long2ReferenceOpenHashMap<BlockEntity> posMap;
private final Long2ReferenceOpenHashMap<List<BlockEntity>> posMapMulti;
public BlockEntityList(List<BlockEntity> list, boolean hasPositionLookup) {
this.posMap = hasPositionLookup ? new Long2ReferenceOpenHashMap<>() : null;
this.posMapMulti = hasPositionLookup ? new Long2ReferenceOpenHashMap<>() : null;

if (this.posMap != null) {
this.posMap.defaultReturnValue(null);
this.posMapMulti.defaultReturnValue(null);
}

public BlockEntityList(List<BlockEntity> list) {
this.allBlockEntities = new ReferenceLinkedOpenHashSet<>();
this.addAll(list);
}

@Override
public int size() {
return this.map.size();
return this.allBlockEntities.size();
}

@Override
public boolean isEmpty() {
return this.map.isEmpty();
return this.allBlockEntities.isEmpty();
}

@Override
public boolean contains(Object o) {
if (o instanceof BlockEntity) {
return this.map.containsKey(getEntityPos((BlockEntity) o));
}

return false;
return this.allBlockEntities.contains(o);
}

@Override
public Iterator<BlockEntity> iterator() {
return this.map.values().iterator();
return this.allBlockEntities.iterator();
}

@Override
public Object[] toArray() {
return this.map.values().toArray();
return this.allBlockEntities.toArray();
}

@Override
@SuppressWarnings("SuspiciousToArrayCall")
public <T> T[] toArray(T[] a) {
return this.map.values().toArray(a);
return this.allBlockEntities.toArray(a);
}

@Override
public boolean add(BlockEntity blockEntity) {
long pos = getEntityPos(blockEntity);
boolean added = this.allBlockEntities.add(blockEntity);

BlockEntity prev = this.map.putAndMoveToLast(pos, blockEntity);
if (added && this.posMap != null) {
long pos = getEntityPos(blockEntity);

// Replacing a block entity should always mark the previous entry as removed
if (prev != null && prev != blockEntity) {
prev.markRemoved();
BlockEntity prev = this.posMap.putIfAbsent(pos, blockEntity);
if (prev != null) {
List<BlockEntity> multiEntry = this.posMapMulti.computeIfAbsent(pos, (long l) -> new ArrayList<>());
if (multiEntry.size() == 0) {
//newly created multi entry: make sure it contains all elements
multiEntry.add(prev);
}
multiEntry.add(blockEntity);
}
}

return true;
return added;
}

@Override
public boolean remove(Object o) {
if (o instanceof BlockEntity) {
BlockEntity blockEntity = (BlockEntity) o;

return this.map.remove(getEntityPos(blockEntity), blockEntity);
if (this.allBlockEntities.remove(o)) {
if (this.posMap != null) {
long pos = getEntityPos(blockEntity);
List<BlockEntity> multiEntry = this.posMapMulti.get(pos);
if (multiEntry != null) {
multiEntry.remove(blockEntity);
if (multiEntry.size() <= 1) {
this.posMapMulti.remove(pos);
}
}
if (multiEntry != null && multiEntry.size() > 0) {
this.posMap.put(pos, multiEntry.get(0));
} else {
this.posMap.remove(pos);
}
}
return true;
}
}

return false;
}

@Override
public boolean containsAll(Collection<?> c) {
for (Object obj : c) {
if (!(obj instanceof BlockEntity)) {
return false;
} else if (this.map.get(getEntityPos((BlockEntity) obj)) == obj) {
return false;
}
}

return true;
return this.allBlockEntities.containsAll(c);
}

@Override
Expand All @@ -107,27 +133,30 @@ public boolean removeAll(Collection<?> c) {
boolean modified = false;

for (Object obj : c) {
if (obj instanceof BlockEntity) {
BlockEntity blockEntity = (BlockEntity) obj;

if (this.map.remove(getEntityPos(blockEntity), blockEntity)) {
modified = true;
}
}
modified |= this.remove(obj);
}

return modified;
}

@Override
public boolean retainAll(Collection<?> c) {
return this.map.values()
.retainAll(c);
boolean modified = false;
for (BlockEntity blockEntity : this.allBlockEntities) {
if (!c.contains(blockEntity)) {
modified |= this.remove(blockEntity);
}
}
return modified;
}

@Override
public void clear() {
this.map.clear();
this.allBlockEntities.clear();
if (this.posMap != null) {
this.posMap.clear();
this.posMapMulti.clear();
}
}

@Override
Expand Down Expand Up @@ -179,24 +208,52 @@ private static long getEntityPos(BlockEntity e) {
return e.getPos().asLong();
}

public BlockEntity getEntityAtPosition(long pos) {
return this.map.get(pos);

public boolean addIfAbsent(BlockEntity blockEntity) {
//we are not checking position equality but object/reference equality (like vanilla)
//the hashset prevents double add of the same object
return this.add(blockEntity);
}

public boolean tryAdd(BlockEntity entity) {
long pos = getEntityPos(entity);
BlockEntity value = this.map.putIfAbsent(pos, entity);
@SuppressWarnings("unused")
public boolean hasPositionLookup() {
return this.posMap != null;
}

if (value == null) {
return true;
//Methods only supported when posMap is present!
public void markRemovedAndRemoveAllAtPosition(BlockPos blockPos) {
long pos = blockPos.asLong();
BlockEntity blockEntity = this.posMap.remove(pos);
if (blockEntity != null) {
List<BlockEntity> multiEntry = this.posMapMulti.remove(pos);
if (multiEntry != null) {
for (BlockEntity blockEntity1 : multiEntry) {
blockEntity1.markRemoved();
this.allBlockEntities.remove(blockEntity1);
}
} else {
blockEntity.markRemoved();
this.allBlockEntities.remove(blockEntity);
}
}
}

public BlockEntity getFirstNonRemovedBlockEntityAtPosition(long pos) {
BlockEntity blockEntity = this.posMap.get(pos);
//usual case: we find no BlockEntity or only one that also is not removed
if (blockEntity == null || !blockEntity.isRemoved()) {
return blockEntity;
}
if (value == entity) {
return false;
//vanilla edge case: two BlockEntities at the same position
//Look up in the posMultiMap to find the first non-removed BlockEntity
List<BlockEntity> multiEntry = this.posMapMulti.get(pos);
if (multiEntry != null) {
for (BlockEntity blockEntity1 : multiEntry) {
if (!blockEntity1.isRemoved()) {
return blockEntity1;
}
}
}
this.map.put(pos, entity);
// Replacing a block entity should always mark the previous entry as removed
// But vanilla does it only when placing the new one in the chunk.
// So we don't do that here.
return true;
return null;
}
}
22 changes: 16 additions & 6 deletions src/main/java/me/jellysquid/mods/lithium/mixin/world/block_entity_ticking/WorldMixin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
@SuppressWarnings("OverwriteModifiers")
@Mixin(World.class)
public abstract class WorldMixin implements WorldAccess {
@Shadow
@Final
public boolean isClient;
@Mutable
@Shadow
@Final
Expand Down Expand Up @@ -55,12 +58,12 @@ private void reinit(MutableWorldProperties properties, RegistryKey<World> regist
Supplier<Profiler> supplier, boolean bl, boolean bl2, long l, CallbackInfo ci) {
// Replace the fallback collections with our types as well
// This won't guarantee mod compatibility, but at least it should fail loudly when it does
this.blockEntities$lithium = new BlockEntityList(this.blockEntities);
this.blockEntities$lithium = new BlockEntityList(this.blockEntities, false);
this.blockEntities = this.blockEntities$lithium;

this.tickingBlockEntities = new HashedReferenceList<>(this.tickingBlockEntities);

this.pendingBlockEntities$lithium = new BlockEntityList(this.pendingBlockEntities);
this.pendingBlockEntities$lithium = new BlockEntityList(this.pendingBlockEntities, true);
this.pendingBlockEntities = this.pendingBlockEntities$lithium;
}

Expand All @@ -70,7 +73,7 @@ private void reinit(MutableWorldProperties properties, RegistryKey<World> regist
*/
@Overwrite
private BlockEntity getPendingBlockEntity(BlockPos pos) {
return this.pendingBlockEntities$lithium.getEntityAtPosition(pos.asLong());
return this.pendingBlockEntities$lithium.getFirstNonRemovedBlockEntityAtPosition(pos.asLong());
}

// We do not want the vanilla code for adding pending block entities to be ran. We'll inject later in
Expand All @@ -94,15 +97,17 @@ private void postBlockEntityTick(CallbackInfo ci) {
}

// Try-add directly to avoid the double map lookup, helps speed things along
if (this.blockEntities$lithium.tryAdd(entity)) {
if (this.blockEntities$lithium.addIfAbsent(entity)) {
//vanilla has an extra updateListeners(...) call on the client here, but the one below should be enough
if (entity instanceof Tickable) {
this.tickingBlockEntities.add(entity);
}

BlockPos pos = entity.getPos();

// Avoid the double chunk lookup (isLoaded followed by getChunk) by simply inlining getChunk call
Chunk chunk = this.getChunk(pos.getX() >> 4, pos.getZ() >> 4, ChunkStatus.FULL, false);
// pass this.isClient instead of false, so the updateListeners call is always executed on the client (like vanilla)
Chunk chunk = this.getChunk(pos.getX() >> 4, pos.getZ() >> 4, ChunkStatus.FULL, this.isClient);

if (chunk != null) {
BlockState state = chunk.getBlockState(pos);
Expand All @@ -119,7 +124,12 @@ private void postBlockEntityTick(CallbackInfo ci) {
}

// We don't want this code wasting a ton of CPU time trying to scan through our optimized collection
// Instead, we simply skip it since the entity will be replaced in the map internally.
// Instead, we simply run the code on those at the same position directly
@Redirect(method = "setBlockEntity", at = @At(value = "INVOKE", target = "Lnet/minecraft/block/entity/BlockEntity;setLocation(Lnet/minecraft/world/World;Lnet/minecraft/util/math/BlockPos;)V"))
private void setLocationAndRemoveAllAtPosition(BlockEntity blockEntity, World world, BlockPos pos) {
blockEntity.setLocation(world, pos);
this.pendingBlockEntities$lithium.markRemovedAndRemoveAllAtPosition(pos);
}
@Redirect(method = "setBlockEntity", at = @At(value = "INVOKE", target = "Ljava/util/List;iterator()Ljava/util/Iterator;"))
private <E> Iterator<E> nullifyBlockEntityScanDuringSetBlockEntity(List<E> list) {
return Collections.emptyIterator();
Expand Down

0 comments on commit bd89303

Please sign in to comment.