Fix ticking behavior for child BlockEntity classes

cardinal-components-block/src/main/java/dev/onyxstudios/cca/internal/block/CardinalBlockInternals.java

@@ -56,6 +56,9 @@ private static synchronized ComponentContainer.Factory<BlockEntity> getBeCompone
             @SuppressWarnings("unchecked") var superclass = (Class<? extends BlockEntity>) entityClass.getSuperclass();
             assert BlockEntity.class.isAssignableFrom(superclass) : "requiresStaticFactory returned false on BlockEntity?";
             factory = /* recursive call */ getBeComponentFactory(superclass);
+
+            // if parent class needs to tick, this one does, too!
+            StaticBlockComponentPlugin.INSTANCE.registerTickersFor(entityClass, superclass);
         }
         entityContainerFactories.put(entityClass, factory);
         return factory;

cardinal-components-block/src/main/java/dev/onyxstudios/cca/internal/block/StaticBlockComponentPlugin.java

@@ -40,6 +40,7 @@
 import net.minecraft.block.entity.BlockEntityTicker;
 import net.minecraft.world.World;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.VisibleForTesting;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -63,8 +64,10 @@ private StaticBlockComponentPlugin() {
 
     private final List<PredicatedComponentFactory<?>> dynamicFactories = new ArrayList<>();
     private final Map<Class<? extends BlockEntity>, Map<ComponentKey<?>, QualifiedComponentFactory<ComponentFactory<? extends BlockEntity, ?>>>> beComponentFactories = new Reference2ObjectOpenHashMap<>();
-    private final Set<Class<? extends BlockEntity>> clientTicking = new ReferenceOpenHashSet<>();
-    private final Set<Class<? extends BlockEntity>> serverTicking = new ReferenceOpenHashSet<>();
+    @VisibleForTesting
+    public final Set<Class<? extends BlockEntity>> clientTicking = new ReferenceOpenHashSet<>();
+    @VisibleForTesting
+    public final Set<Class<? extends BlockEntity>> serverTicking = new ReferenceOpenHashSet<>();
 
     @Nullable
     public <T extends BlockEntity> BlockEntityTicker<T> getComponentTicker(World world, T be, @Nullable BlockEntityTicker<T> base) {
@@ -97,12 +100,12 @@ public boolean requiresStaticFactory(Class<? extends BlockEntity> entityClass) {
     public ComponentContainer.Factory<BlockEntity> buildDedicatedFactory(Class<? extends BlockEntity> entityClass) {
         StaticBlockComponentPlugin.INSTANCE.ensureInitialized();
 
-        var compiled = new LinkedHashMap<>(this.beComponentFactories.getOrDefault(entityClass, Collections.emptyMap()));
+        var compiled = new LinkedHashMap<>(this.beComponentFactories.getOrDefault(entityClass, Map.of()));
         Class<? extends BlockEntity> type = entityClass;
 
         while (type != BlockEntity.class) {
             type = type.getSuperclass().asSubclass(BlockEntity.class);
-            for (var e : this.beComponentFactories.getOrDefault(type, Collections.emptyMap()).entrySet()) {
+            for (var e : this.beComponentFactories.getOrDefault(type, Map.of()).entrySet()) {
                 compiled.putIfAbsent(e.getKey(), e.getValue());
             }
         }
@@ -119,6 +122,15 @@ public ComponentContainer.Factory<BlockEntity> buildDedicatedFactory(Class<? ext
         return builder.build();
     }
 
+    public void registerTickersFor(Class<? extends BlockEntity> entityClass, Class<? extends BlockEntity> parentClass) {
+        if(this.clientTicking.contains(parentClass)) {
+            this.clientTicking.add(entityClass);
+        }
+        if(this.serverTicking.contains(parentClass)) {
+            this.serverTicking.add(entityClass);
+        }
+    }
+
     private <C extends Component> void addToBuilder(ComponentContainer.Factory.Builder<BlockEntity> builder, Map.Entry<ComponentKey<?>, QualifiedComponentFactory<ComponentFactory<? extends BlockEntity, ?>>> entry) {
         @SuppressWarnings("unchecked") var key = (ComponentKey<C>) entry.getKey();
         @SuppressWarnings("unchecked") var factory = (ComponentFactory<BlockEntity, C>) entry.getValue().factory();

cardinal-components-block/src/testmod/java/dev/onyxstudios/cca/test/block/CcaBlockTestMod.java

@@ -30,10 +30,7 @@
 import dev.onyxstudios.cca.test.base.Vita;
 import net.fabricmc.api.ModInitializer;
 import net.fabricmc.fabric.api.lookup.v1.block.BlockApiLookup;
-import net.minecraft.block.entity.BlockEntityType;
-import net.minecraft.block.entity.CommandBlockBlockEntity;
-import net.minecraft.block.entity.EndGatewayBlockEntity;
-import net.minecraft.block.entity.EndPortalBlockEntity;
+import net.minecraft.block.entity.*;
 import net.minecraft.util.Identifier;
 import net.minecraft.util.math.Direction;
 
@@ -46,6 +43,7 @@ public void registerBlockComponentFactories(BlockComponentFactoryRegistry regist
         registry.registerFor(EndGatewayBlockEntity.class, VitaCompound.KEY, VitaCompound::new);
         registry.registerFor(EndPortalBlockEntity.class, TickingTestComponent.KEY, be -> new TickingTestComponent());
         registry.registerFor(CommandBlockBlockEntity.class, LoadAwareTestComponent.KEY, be -> new LoadAwareTestComponent());
+        registry.registerFor(BlockEntity.class, GlobalTickingComponent.KEY, GlobalTickingComponent::new);
     }
 
     @Override

cardinal-components-block/src/testmod/java/dev/onyxstudios/cca/test/block/CcaBlockTestSuite.java

@@ -22,6 +22,7 @@
  */
 package dev.onyxstudios.cca.test.block;
 
+import dev.onyxstudios.cca.internal.block.StaticBlockComponentPlugin;
 import dev.onyxstudios.cca.test.base.LoadAwareTestComponent;
 import dev.onyxstudios.cca.test.base.TickingTestComponent;
 import dev.onyxstudios.cca.test.base.Vita;
@@ -39,46 +40,28 @@
 import org.jetbrains.annotations.NotNull;
 
 import java.util.Objects;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.BooleanSupplier;
 
 public class CcaBlockTestSuite implements FabricGameTest {
-    @GameTest(templateName = EMPTY_STRUCTURE)
-    public void beSerialize(TestContext ctx) {
+    @GameTest(templateName = EMPTY_STRUCTURE) public void beSerialize(TestContext ctx) {
         BlockPos pos = ctx.getAbsolutePos(BlockPos.ORIGIN);
-        BlockEntity be = Objects.requireNonNull(
-            BlockEntityType.END_GATEWAY.instantiate(
-                pos,
-                Blocks.END_GATEWAY.getDefaultState()
-            )
-        );
+        BlockEntity be = Objects.requireNonNull(BlockEntityType.END_GATEWAY.instantiate(pos, Blocks.END_GATEWAY.getDefaultState()));
         be.getComponent(Vita.KEY).setVitality(42);
         NbtCompound nbt = be.createNbt();
-        BlockEntity be1 = Objects.requireNonNull(
-            BlockEntityType.END_GATEWAY.instantiate(
-                pos, Blocks.END_GATEWAY.getDefaultState()
-            )
-        );
+        BlockEntity be1 = Objects.requireNonNull(BlockEntityType.END_GATEWAY.instantiate(pos, Blocks.END_GATEWAY.getDefaultState()));
         GameTestUtil.assertTrue("New BlockEntity should have values zeroed", be1.getComponent(Vita.KEY).getVitality() == 0);
         be1.readNbt(nbt);
         GameTestUtil.assertTrue("BlockEntity component data should survive deserialization", be1.getComponent(Vita.KEY).getVitality() == 42);
         ctx.complete();
     }
 
-    @GameTest(templateName = EMPTY_STRUCTURE)
-    public void canQueryThroughLookup(TestContext ctx) {
+    @GameTest(templateName = EMPTY_STRUCTURE) public void canQueryThroughLookup(TestContext ctx) {
         BlockPos pos = ctx.getAbsolutePos(BlockPos.ORIGIN);
-        BlockEntity be = Objects.requireNonNull(
-            BlockEntityType.END_GATEWAY.instantiate(
-                pos,
-                Blocks.END_GATEWAY.getDefaultState()
-            )
-        );
+        BlockEntity be = Objects.requireNonNull(BlockEntityType.END_GATEWAY.instantiate(pos, Blocks.END_GATEWAY.getDefaultState()));
         getVita(ctx, pos, be).setVitality(42);
         NbtCompound nbt = be.createNbt();
-        BlockEntity be1 = Objects.requireNonNull(
-            BlockEntityType.END_GATEWAY.instantiate(
-                pos, Blocks.END_GATEWAY.getDefaultState()
-            )
-        );
+        BlockEntity be1 = Objects.requireNonNull(BlockEntityType.END_GATEWAY.instantiate(pos, Blocks.END_GATEWAY.getDefaultState()));
         GameTestUtil.assertTrue("New BlockEntity should have values zeroed", getVita(ctx, pos, be1).getVitality() == 0);
         be1.readNbt(nbt);
         GameTestUtil.assertTrue("BlockEntity component data should survive deserialization", getVita(ctx, pos, be1).getVitality() == 42);
@@ -89,36 +72,83 @@ public void canQueryThroughLookup(TestContext ctx) {
         return Objects.requireNonNull(CcaBlockTestMod.VITA_API_LOOKUP.find(ctx.getWorld(), pos, null, be, Direction.DOWN));
     }
 
-    @GameTest(templateName = EMPTY_STRUCTURE)
-    public void beComponentsTick(TestContext ctx) {
+    @GameTest(templateName = EMPTY_STRUCTURE) public void beComponentsTick(TestContext ctx) {
         ctx.setBlockState(BlockPos.ORIGIN, Blocks.END_PORTAL);
+
+        var blockentity = ctx.getBlockEntity(BlockPos.ORIGIN);
+        GameTestUtil.assertTrue("Block entity should not be null", blockentity != null);
+        GameTestUtil.assertTrue("BlockEntity should have TickingTestComponent", TickingTestComponent.KEY.getNullable(blockentity) != null);
+        GameTestUtil.assertTrue("Class should be registered as server ticker", StaticBlockComponentPlugin.INSTANCE.serverTicking.contains(blockentity.getClass()));
+        GameTestUtil.assertTrue("Class should be registered as client ticker", StaticBlockComponentPlugin.INSTANCE.clientTicking.contains(blockentity.getClass()));
+
         ctx.waitAndRun(5, () -> {
-            int ticks = Objects.requireNonNull(ctx.getBlockEntity(BlockPos.ORIGIN)).getComponent(TickingTestComponent.KEY).serverTicks();
+            var blockentity2 = ctx.getBlockEntity(BlockPos.ORIGIN);
+            GameTestUtil.assertTrue("Block entity should still exist", blockentity2 != null);
+            int ticks = blockentity2.getComponent(TickingTestComponent.KEY).serverTicks();
             GameTestUtil.assertTrue("Component should tick 5 times", ticks == 5);
             ctx.complete();
         });
     }
 
-    @GameTest(templateName = EMPTY_STRUCTURE)
-    public void beComponentsLoadUnload(TestContext ctx) {
+    @GameTest(templateName = EMPTY_STRUCTURE) public void beComponentsLoadUnload(TestContext ctx) {
         BlockEntity firstCommandBlock = new CommandBlockBlockEntity(ctx.getAbsolutePos(BlockPos.ORIGIN), Blocks.CHAIN_COMMAND_BLOCK.getDefaultState());
-        GameTestUtil.assertTrue(
-            "Load counter should not be incremented until the block entity joins the world",
-            LoadAwareTestComponent.KEY.get(firstCommandBlock).getLoadCounter() == 0
-        );
+        GameTestUtil.assertTrue("Load counter should not be incremented until the block entity joins the world", LoadAwareTestComponent.KEY.get(firstCommandBlock).getLoadCounter() == 0);
         ctx.setBlockState(BlockPos.ORIGIN, Blocks.CHAIN_COMMAND_BLOCK);
         BlockEntity commandBlock = Objects.requireNonNull(ctx.getBlockEntity(BlockPos.ORIGIN));
-        GameTestUtil.assertTrue(
-            "Load counter should be incremented once when the block entity joins the world",
-            LoadAwareTestComponent.KEY.get(commandBlock).getLoadCounter() == 1
-        );
+        GameTestUtil.assertTrue("Load counter should be incremented once when the block entity joins the world", LoadAwareTestComponent.KEY.get(commandBlock).getLoadCounter() == 1);
         ctx.setBlockState(BlockPos.ORIGIN, Blocks.AIR);
         ctx.waitAndRun(1, () -> {
-            GameTestUtil.assertTrue(
-                "Load counter should be decremented when the block entity leaves the world",
-                LoadAwareTestComponent.KEY.get(commandBlock).getLoadCounter() == 0
-            );
+            GameTestUtil.assertTrue("Load counter should be decremented when the block entity leaves the world", LoadAwareTestComponent.KEY.get(commandBlock).getLoadCounter() == 0);
+            ctx.complete();
+        });
+    }
+
+    @GameTest(templateName = EMPTY_STRUCTURE) public void rootClassServerTicker(TestContext ctx) {
+        ctx.setBlockState(BlockPos.ORIGIN, Blocks.BARREL);
+
+        var blockentity = ctx.getBlockEntity(BlockPos.ORIGIN);
+        GameTestUtil.assertTrue("Block entity should not be null", blockentity != null);
+
+        GameTestUtil.assertTrue("Class should be registered as server ticker", StaticBlockComponentPlugin.INSTANCE.serverTicking.contains(blockentity.getClass()));
+        GameTestUtil.assertFalse("Class should NOT be registered as client ticker", StaticBlockComponentPlugin.INSTANCE.clientTicking.contains(blockentity.getClass()));
+
+        var component = GlobalTickingComponent.KEY.getNullable(blockentity);
+        GameTestUtil.assertTrue("Component should exist", component != null);
+
+        AtomicInteger flag = new AtomicInteger(0);
+        BooleanSupplier action = () -> {
+            flag.getAndIncrement();
+            return false;
+        };
+        component.setTickAction(action);
+        GameTestUtil.assertTrue("Tick action should be set", component.getTickAction().isPresent());
+
+        ctx.waitAndRun(5, () -> {
+            var blockentity2 = ctx.getBlockEntity(BlockPos.ORIGIN);
+            GameTestUtil.assertTrue("Block entity should still exist", blockentity2 != null);
+            GameTestUtil.assertTrue("Tick action should be cleared", blockentity2.getComponent(GlobalTickingComponent.KEY).getTickAction().isEmpty());
+            GameTestUtil.assertTrue("Tick action should have run exactly once", flag.get() == 1);
+
             ctx.complete();
         });
     }
+
+    /**
+     * same as {@link CcaBlockTestSuite#rootClassServerTicker(TestContext)} but for a BlockEntity that has an explicit
+     * component registered, so that {@link StaticBlockComponentPlugin#requiresStaticFactory(Class)} returns true for
+     * the class itself rather than delegating to the parent class.
+     */
+    @GameTest(templateName = EMPTY_STRUCTURE)
+    public void rootClassServerTickerWithExplicitRegistration(TestContext ctx) {
+        ctx.setBlockState(BlockPos.ORIGIN, Blocks.COMMAND_BLOCK);
+
+        var blockentity = ctx.getBlockEntity(BlockPos.ORIGIN);
+        GameTestUtil.assertTrue("Block entity should not be null", blockentity != null);
+        GameTestUtil.assertTrue("Class should be registered as server ticker", StaticBlockComponentPlugin.INSTANCE.serverTicking.contains(blockentity.getClass()));
+
+        var component = GlobalTickingComponent.KEY.getNullable(blockentity);
+        GameTestUtil.assertTrue("Component should exist", component != null);
+
+        ctx.complete();
+    }
 }

cardinal-components-block/src/testmod/java/dev/onyxstudios/cca/test/block/GlobalTickingComponent.java

@@ -0,0 +1,48 @@
+package dev.onyxstudios.cca.test.block;
+
+import dev.onyxstudios.cca.api.v3.component.Component;
+import dev.onyxstudios.cca.api.v3.component.ComponentKey;
+import dev.onyxstudios.cca.api.v3.component.ComponentRegistry;
+import dev.onyxstudios.cca.api.v3.component.tick.ServerTickingComponent;
+import net.minecraft.nbt.NbtCompound;
+import net.minecraft.util.Identifier;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Optional;
+import java.util.function.BooleanSupplier;
+
+public class GlobalTickingComponent implements Component, ServerTickingComponent {
+
+    public static final ComponentKey<GlobalTickingComponent> KEY = ComponentRegistry.getOrCreate(new Identifier(CcaBlockTestMod.MOD_ID, "global_ticking_test"), GlobalTickingComponent.class);
+
+    public GlobalTickingComponent(Object provider) {
+        // NO-OP
+    }
+
+    @Nullable
+    private BooleanSupplier onTick;
+
+    void setTickAction(@Nullable BooleanSupplier onTick) {
+        this.onTick = onTick;
+    }
+
+    @Override public void serverTick() {
+        if(this.onTick != null) {
+            if(!this.onTick.getAsBoolean()) {
+                this.onTick = null;
+            }
+        }
+    }
+
+    public Optional<BooleanSupplier> getTickAction() {
+        return Optional.ofNullable(this.onTick);
+    }
+
+    @Override public void readFromNbt(NbtCompound tag) {
+        // NO-OP
+    }
+
+    @Override public void writeToNbt(NbtCompound tag) {
+        // NO-OP
+    }
+}

cardinal-components-block/src/testmod/resources/fabric.mod.json

@@ -25,7 +25,8 @@
     "license": "MIT",
     "custom": {
         "cardinal-components": [
-          "cca-block-test:vita_compound"
+          "cca-block-test:vita_compound",
+          "cca-block-test:global_ticking_test"
         ]
     }
 }