Skip to content

Commit

Permalink
Spark performance improvements (#4459)
Browse files Browse the repository at this point in the history
Fixes #4389 

Sparks already had a lot of the infrastructure needed, but I basically
changed spark from recalculating everything every tick to being notified
when stuff changes. Note that this approach is more prone to bugs,
especially when more funcitonality gets added. The issue also goes a lot
more into detail. I also tested using Fabric's block cache API but it
didn't turn out to be worth it.
  • Loading branch information
anonymous123-code authored Oct 15, 2023
1 parent 9f8e55c commit 1d37051
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,19 @@ public interface ManaSpark extends SparkEntity {
/**
* Gets a collection of all Sparks this is tranfering to.
*/
Collection<ManaSpark> getTransfers();
Collection<ManaSpark> getOutgoingTransfers();

/**
* Registers the Spark passed in as a Spark meant for mana to be transfered towards.
*/
void registerTransfer(ManaSpark entity);

/**
* Makes that spark register transfers for all relevant sparks, needs to be called whenever the connected sparks may
* change (setNetwork, remove and setUpgrade should already do this)
*/
void updateTransfers();

SparkUpgradeType getUpgrade();

void setUpgrade(SparkUpgradeType upgrade);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import vazkii.botania.xplat.XplatAbstractions;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

public class SparkTinkererBlockEntity extends ExposedSimpleInventoryBlockEntity {
Expand Down Expand Up @@ -59,10 +58,6 @@ public void doSwap() {
ItemStack sparkStack = SparkAugmentItem.getByType(upg);
SparkUpgradeType newUpg = changeStack.isEmpty() ? SparkUpgradeType.NONE : ((SparkAugmentItem) changeStack.getItem()).type;
spark.setUpgrade(newUpg);
Collection<ManaSpark> transfers = spark.getTransfers();
if (transfers != null) {
transfers.clear();
}
getItemHandler().setItem(0, sparkStack);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ public static void commonTick(Level level, BlockPos worldPosition, BlockState st
for (Direction dir : Direction.values()) {
var relPos = worldPosition.relative(dir);
if (level.hasChunkAt(relPos)) {
var receiverAt = XplatAbstractions.INSTANCE.findManaReceiver(level, relPos,
level.getBlockState(relPos), level.getBlockEntity(relPos), dir.getOpposite());
var receiverAt = XplatAbstractions.INSTANCE.findManaReceiver(level, relPos, dir.getOpposite());
if (receiverAt instanceof ManaPool pool) {
if (wasInNetwork && (pool != self.receiver || self.getVariant() == ManaSpreaderBlock.Variant.REDSTONE)) {
if (pool instanceof KeyLocked locked && !locked.getOutputKey().equals(self.getInputKey())) {
Expand Down Expand Up @@ -348,9 +347,7 @@ public void readPacketNBT(CompoundTag cmp) {
int z = cmp.getInt(TAG_FORCE_CLIENT_BINDING_Z);
if (y != Integer.MIN_VALUE) {
var pos = new BlockPos(x, y, z);
var state = level.getBlockState(pos);
var be = level.getBlockEntity(pos);
receiver = XplatAbstractions.INSTANCE.findManaReceiver(level, pos, state, be, null);
receiver = XplatAbstractions.INSTANCE.findManaReceiver(level, pos, null);
} else {
receiver = null;
}
Expand Down
130 changes: 97 additions & 33 deletions Xplat/src/main/java/vazkii/botania/common/entity/ManaSparkEntity.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,13 @@ public class ManaSparkEntity extends SparkBaseEntity implements ManaSpark {
private static final String TAG_UPGRADE = "upgrade";
private static final EntityDataAccessor<Integer> UPGRADE = SynchedEntityData.defineId(ManaSparkEntity.class, EntityDataSerializers.INT);

private final Set<ManaSpark> transfers = Collections.newSetFromMap(new WeakHashMap<>());
private final Set<ManaSpark> outgoingTransfers = Collections.newSetFromMap(new WeakHashMap<>());

private int removeTransferants = 2;
private final ArrayList<ManaSpark> transfersTowardsSelfToRegister = new ArrayList<>();

private boolean shouldFilterTransfers = true;
private boolean receiverWasFull = true;
private boolean firstTick = true;

public ManaSparkEntity(EntityType<ManaSparkEntity> type, Level world) {
super(type, world);
Expand Down Expand Up @@ -89,6 +93,11 @@ public void tick() {
return;
}

// When loaded, initialize transfers
if (firstTick) {
updateTransfers();
}

SparkAttachable tile = getAttachedTile();
if (tile == null) {
dropAndKill();
Expand All @@ -97,7 +106,7 @@ public void tick() {
var receiver = getAttachedManaReceiver();

SparkUpgradeType upgrade = getUpgrade();
Collection<ManaSpark> transfers = getTransfers();
Collection<ManaSpark> transfers = getOutgoingTransfers();

switch (upgrade) {
case DISPERSIVE -> {
Expand Down Expand Up @@ -163,29 +172,25 @@ public void tick() {

}
case DOMINANT -> {
List<ManaSpark> validSparks = SparkHelper.getSparksAround(level(), getX(), getY() + (getBbHeight() / 2), getZ(), getNetwork());
validSparks.removeIf(s -> {
SparkUpgradeType otherUpgrade = s.getUpgrade();
return s == this || otherUpgrade != SparkUpgradeType.NONE || !(s.getAttachedManaReceiver() instanceof ManaPool);
});
if (!validSparks.isEmpty()) {
validSparks.get(level().random.nextInt(validSparks.size())).registerTransfer(this);
if (receiverWasFull && !receiver.isFull()) {
updateTransfers();
}
if (!transfersTowardsSelfToRegister.isEmpty()) {
transfersTowardsSelfToRegister.remove(transfersTowardsSelfToRegister.size() - 1).registerTransfer(this);
}

}
case RECESSIVE -> {
var otherSparks = SparkHelper.getSparksAround(level(), getX(), getY() + (getBbHeight() / 2), getZ(), getNetwork());
for (var otherSpark : otherSparks) {
SparkUpgradeType otherUpgrade = otherSpark.getUpgrade();
if (otherSpark != this
&& otherUpgrade != SparkUpgradeType.DOMINANT
&& otherUpgrade != SparkUpgradeType.RECESSIVE
&& otherUpgrade != SparkUpgradeType.ISOLATED) {
transfers.add(otherSpark);
}
// Recessive does not need to be handled because recessive sparks get notified in all relevant cases
default -> {
if (receiverWasFull && !receiver.isFull()) {
notifyOthers(getNetwork());
}
}
default -> {}
}

if (receiver != null) {
receiverWasFull = receiver.isFull();
} else {
receiverWasFull = true;
}

if (!transfers.isEmpty()) {
Expand All @@ -194,11 +199,17 @@ public void tick() {
int manaSpent = 0;

if (manaTotal > 0) {
if (shouldFilterTransfers) {
filterTransfers();
shouldFilterTransfers = false;
}

for (ManaSpark spark : transfers) {
count--;
SparkAttachable attached = spark.getAttachedTile();
var attachedReceiver = spark.getAttachedManaReceiver();
if (attached == null || attachedReceiver == null || attachedReceiver.isFull() || spark.areIncomingTransfersDone()) {
shouldFilterTransfers = true;
continue;
}

Expand All @@ -212,8 +223,36 @@ public void tick() {
}
}

if (removeTransferants > 0) {
removeTransferants--;
firstTick = false;
}

@Override
public void updateTransfers() {
transfersTowardsSelfToRegister.clear();
switch (getUpgrade()) {
case RECESSIVE -> {
var otherSparks = SparkHelper.getSparksAround(level(), getX(), getY() + (getBbHeight() / 2), getZ(), getNetwork());
Collections.shuffle(otherSparks);
for (var otherSpark : otherSparks) {
SparkUpgradeType otherUpgrade = otherSpark.getUpgrade();
if (otherSpark != this
&& otherUpgrade != SparkUpgradeType.DOMINANT
&& otherUpgrade != SparkUpgradeType.RECESSIVE
&& otherUpgrade != SparkUpgradeType.ISOLATED) {
outgoingTransfers.add(otherSpark);
}
}
}
case DOMINANT -> {
List<ManaSpark> validSparks = SparkHelper.getSparksAround(level(), getX(), getY() + (getBbHeight() / 2), getZ(), getNetwork());
for (var spark : validSparks) {
SparkUpgradeType otherUpgrade = spark.getUpgrade();
if (spark != this && otherUpgrade == SparkUpgradeType.NONE && spark.getAttachedManaReceiver() instanceof ManaPool) {
transfersTowardsSelfToRegister.add(spark);
}
}
Collections.shuffle(transfersTowardsSelfToRegister);
}
}
filterTransfers();
}
Expand Down Expand Up @@ -244,6 +283,12 @@ private void dropAndKill() {
discard();
}

@Override
public void remove(RemovalReason removalReason) {
super.remove(removalReason);
notifyOthers(getNetwork());
}

@Override
public InteractionResult interact(Player player, InteractionHand hand) {
ItemStack stack = player.getItemInHand(hand);
Expand All @@ -256,8 +301,9 @@ public InteractionResult interact(Player player, InteractionHand hand) {
spawnAtLocation(SparkAugmentItem.getByType(upgrade), 0F);
setUpgrade(SparkUpgradeType.NONE);

transfers.clear();
removeTransferants = 2;
// Recalculate transfers, recessive and dominant will register the proper transfers
outgoingTransfers.clear();
notifyOthers(getNetwork());
} else {
dropAndKill();
}
Expand Down Expand Up @@ -318,7 +364,7 @@ public ManaReceiver getAttachedManaReceiver() {
}

private void filterTransfers() {
Iterator<ManaSpark> iter = transfers.iterator();
Iterator<ManaSpark> iter = outgoingTransfers.iterator();
while (iter.hasNext()) {
ManaSpark spark = iter.next();
SparkUpgradeType upgr = getUpgrade();
Expand All @@ -340,21 +386,27 @@ private void filterTransfers() {
}

@Override
public Collection<ManaSpark> getTransfers() {
filterTransfers();
return transfers;
public Collection<ManaSpark> getOutgoingTransfers() {
return outgoingTransfers;
}

private boolean hasTransfer(ManaSpark entity) {
return transfers.contains(entity);
return outgoingTransfers.contains(entity);
}

@Override
public void registerTransfer(ManaSpark entity) {
if (hasTransfer(entity)) {
return;
}
transfers.add(entity);
outgoingTransfers.add(entity);
filterTransfers();
}

private void notifyOthers(DyeColor network) {
for (var spark : SparkHelper.getSparksAround(level(), getX(), getY() + (getBbHeight() / 2), getZ(), network)) {
spark.updateTransfers();
}
}

@Override
Expand All @@ -365,12 +417,24 @@ public SparkUpgradeType getUpgrade() {
@Override
public void setUpgrade(SparkUpgradeType upgrade) {
entityData.set(UPGRADE, upgrade.ordinal());
updateTransfers();
notifyOthers(getNetwork());
}

@Override
public void setNetwork(DyeColor color) {
// The previous network needs to filter this spark out
var previousNetwork = getNetwork();
super.setNetwork(color);
updateTransfers();
notifyOthers(color);
notifyOthers(previousNetwork);
}

@Override
public boolean areIncomingTransfersDone() {
if (getAttachedManaReceiver() instanceof ManaPool) {
return removeTransferants > 0;
return false;
}

SparkAttachable attachable = getAttachedTile();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ public InteractionResult useOn(UseOnContext ctx) {
}

public static boolean applyStick(Level world, BlockPos pos) {
var state = world.getBlockState(pos);
var be = world.getBlockEntity(pos);
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(world, pos, state, be, null);
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(world, pos, null);
if (receiver instanceof ManaPool || receiver instanceof ManaCollector) {
int range = receiver instanceof ManaPool ? FunctionalFlowerBlockEntity.LINK_RANGE : GeneratingFlowerBlockEntity.LINK_RANGE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ public void updateBurst(ManaBurst burst, ItemStack stack) {
final boolean sourceless = source.equals(ManaBurst.NO_SOURCE);

Predicate<BlockPos> predicate = pos -> {
var state = entity.level().getBlockState(pos);
var be = entity.level().getBlockEntity(pos);
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(entity.level(), pos, state, be, null);
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(entity.level(), pos, null);
return receiver != null
&& (sourceless || pos.distSqr(source) > 9)
&& receiver.canReceiveManaFromBursts()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ private void handleHitBlock(ManaBurst burst, BlockHitResult result) {

var entity = burst.entity();
var hitPos = result.getBlockPos();
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(entity.level(), hitPos,
entity.level().getBlockState(hitPos), entity.level().getBlockEntity(hitPos), result.getDirection());
var receiver = XplatAbstractions.INSTANCE.findManaReceiver(entity.level(), hitPos, result.getDirection());
if (receiver instanceof ManaSpreader spreader) {
Vec3 tileVec = Vec3.atCenterOf(hitPos);
Vec3 diffVec = sourceVec.subtract(tileVec);
Expand Down

0 comments on commit 1d37051

Please sign in to comment.