Working on redstone sulfur cube#45
Conversation
|
/codesmith |
|
Reviewing redstone sulfur cube PR context |
Reviewed the redstone sulfur cube change end to end. Since this repo has no CI, I set up a JDK, built it, and booted a dedicated server to actually exercise the mixins. Good news first: it compiles, and a server boots cleanly on Minecraft 26.2 / Fabric Loader 0.19.3 with all three mixins applied and no injection errors (ran 60s idle, then I stopped it). I also verified against the 26.2 remapped jars that every A few things worth your attention, roughly in priority order: 1. Performance: entity scan on the two hottest redstone primitives. 2. Behavior: 3. Missing tag translations (guideline). The two new item tags have no "tag.item.minecraft.sulfur_cube_swallowable": "Sulfur Cube Swallowable",
"tag.item.minecraft.sulfur_cube_archetype.fast_flat": "Sulfur Cube Fast Flat Archetype",4. Minor nits.
Want me to implement 1, 2, and 4 and land the translations? Since the branch is protected, I'd open a PR into |
|
I will tackle this tomorrow, its lwk too hot atm |
|
all 4 things need to be fixed tho |
|
/codesmith |
Picking this up from where we left off. Your What changed (two files):
I could not open a PR: the repo rulesets block both paths. Pushing to diff --git a/src/main/java/blob/vanillasquared/mixin/world/ServerLevelMixin.java b/src/main/java/blob/vanillasquared/mixin/world/ServerLevelMixin.java
index 6254359..1fffb3a 100644
--- a/src/main/java/blob/vanillasquared/mixin/world/ServerLevelMixin.java
+++ b/src/main/java/blob/vanillasquared/mixin/world/ServerLevelMixin.java
@@ -11,12 +11,16 @@ import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BooleanSupplier;
@Mixin(ServerLevel.class)
public abstract class ServerLevelMixin implements VSQEntityRedstonePowerLevelAccess {
+ // Entity deserialization runs off the main server thread (EntityStorage's background
+ // deserializer queue), so this counter is mutated concurrently with the main-thread
+ // tick/removal/signal paths. Use an atomic to avoid lost updates and stale reads.
@Unique
- private int vsq$poweredEntityCount;
+ private final AtomicInteger vsq$poweredEntityCount = new AtomicInteger();
@Inject(method = "tick", at = @At("TAIL"))
private void vsq$tickChanneling(BooleanSupplier haveTime, CallbackInfo ci) {
@@ -27,18 +31,16 @@ public abstract class ServerLevelMixin implements VSQEntityRedstonePowerLevelAcc
@Override
public int vsq$getPoweredEntityCount() {
- return this.vsq$poweredEntityCount;
+ return this.vsq$poweredEntityCount.get();
}
@Override
public void vsq$incrementPoweredEntityCount() {
- this.vsq$poweredEntityCount++;
+ this.vsq$poweredEntityCount.incrementAndGet();
}
@Override
public void vsq$decrementPoweredEntityCount() {
- if (this.vsq$poweredEntityCount > 0) {
- this.vsq$poweredEntityCount--;
- }
+ this.vsq$poweredEntityCount.updateAndGet(count -> count > 0 ? count - 1 : 0);
}
}
diff --git a/src/main/java/blob/vanillasquared/mixin/world/redstone/SignalGetterMixin.java b/src/main/java/blob/vanillasquared/mixin/world/redstone/SignalGetterMixin.java
index 9fa454c..e1a0ef1 100644
--- a/src/main/java/blob/vanillasquared/mixin/world/redstone/SignalGetterMixin.java
+++ b/src/main/java/blob/vanillasquared/mixin/world/redstone/SignalGetterMixin.java
@@ -12,6 +12,9 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
@Mixin(SignalGetter.class)
public interface SignalGetterMixin {
+ // A block of redstone is a weak power source only: it powers adjacent components/dust but
+ // does not strongly power a solid block. Mirror that by augmenting getSignal (weak power)
+ // only, and leaving getDirectSignal (strong power) untouched.
@Inject(method = "getSignal", at = @At("RETURN"), cancellable = true)
private void vsq$getEntityRedstoneSignal(BlockPos pos, Direction direction, CallbackInfoReturnable<Integer> cir) {
SignalGetter signalGetter = (SignalGetter) this;
@@ -19,12 +22,4 @@ public interface SignalGetterMixin {
cir.setReturnValue(Math.max(cir.getReturnValue(), VSQEntityRedstonePower.getSignal(level, pos)));
}
}
-
- @Inject(method = "getDirectSignal", at = @At("RETURN"), cancellable = true)
- private void vsq$getDirectEntityRedstoneSignal(BlockPos pos, Direction direction, CallbackInfoReturnable<Integer> cir) {
- SignalGetter signalGetter = (SignalGetter) this;
- if (signalGetter instanceof ServerLevel level) {
- cir.setReturnValue(Math.max(cir.getReturnValue(), VSQEntityRedstonePower.getSignal(level, pos)));
- }
- }
}If you'd rather I open the PR directly, let the Blacksmith bot create branches (or push to this one) in the repo ruleset and re-ping me, and I'll push it up. With this applied, all four original items plus the concurrency regression are resolved. |
|
@mizius-bot review and give me all issues immediately |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR adds entity-powered redstone to VanillaSquared: entities can store and emit a redstone signal (0–15) via a new vsq:powerRedstone NBT field, and Sulfur Cubes automatically emit power 15 when their BODY slot holds a redstone block. Signal queries in SignalGetter are extended to scan entity AABBs, and a per-level powered-entity counter enables an early-exit optimisation when no powered entities exist.
- Entity power storage & count tracking (
EntityRedstonePowerMixin,ServerLevelMixin): NBT persistence, increment/decrement bookkeeping, and level-change handling look correct. Unconditional 2-tick neighbor updates fire even for completely stationary entities, which can cause unnecessary redstone recalculations in complex circuits. - Signal injection (
SignalGetterMixin): BothgetSignalandgetDirectSignalcorrectly take the max of block and entity signals, gated by the powered-entity count optimisation. - Load-order interaction (
SulfurCubeMixin+EntityRedstonePowerMixin):vsq$setRedstonePowerForContentruns atreadAdditionalSaveDataTAIL, butvsq$loadEntityRedstonePowerruns atEntity.load()TAIL (which wrapsreadAdditionalSaveData), so the NBT value overwrites the derived value; a freshly upgraded entity without the NBT key loads with power 0 for one tick.
Reviewed & summarized for commit 89d268c.
|
@mizius-bot review and give me all issues immediately |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR introduces entity-powered redstone by adding a vsq:powerRedstone NBT attribute to all entities (0–15), hooking it into the SignalGetter interface so redstone circuits can read signal strength from entities overlapping a block, and wiring Sulfur Cubes to emit power 15 when their BODY slot contains a redstone block.
- New infrastructure (
VSQEntityRedstonePower,VSQEntityRedstonePowerAccess,VSQEntityRedstonePowerLevelAccess) tracks a per-level powered-entity count so signal queries short-circuit to 0 when no powered entity exists, keeping the hot path cheap. EntityRedstonePowerMixinadds NBT persistence, a reconcile/unregister counter lifecycle on every entity, and firesupdateNeighborsAtevery 2 ticks and on movement/removal to keep adjacent redstone in sync.SulfurCubeMixinderives power from the BODY slot item (redstone block → 15, else 0) on each AI tick and on load; two new item tags (sulfur_cube_swallowable,sulfur_cube_archetype/fast_flat) and matchingen_ustranslations are added.
Reviewed & summarized for commit 89d268c.
|
@mizius-bot review and give me all issues immediately |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR introduces entity-powered redstone: entities can now emit redstone signal (0–15) stored via NBT, and a Sulfur Cube emits power 15 when its BODY slot holds a redstone block. The SignalGetter interface is extended so both direct and indirect signal queries include entity contributions, gated by a per-level powered-entity counter to avoid scans when no powered entities exist.
EntityRedstonePowerMixinadds load/save, power bookkeeping, and per-tick neighbor-update logic to all entities;ServerLevelMixintracks the powered-entity count.SignalGetterMixininjects intogetSignal/getDirectSignalonServerLevelto merge entity signals with block signals.SulfurCubeMixinwires the BODY-slot item check intocustomServerAiStepandreadAdditionalSaveData; two new item tags and their translations are also added.
Reviewed & summarized for commit 5342c18.
|
@mizius-bot review |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR adds entity-backed redstone behavior for sulfur cubes. The main changes are:
- Entity redstone power storage and NBT persistence.
- Server-level tracking for powered entities.
- Signal lookup that includes powered entities overlapping a block.
- Sulfur cube redstone-block power resolution.
- New sulfur cube item tags and translations.
Reviewed & summarized for commit 80bc52a.
|
@mizius-bot review |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR adds entity-based redstone power for Sulfur Cubes. The main changes are:
- New entity redstone power storage and NBT persistence.
- Server-level powered-entity counting and signal scanning.
- Redstone signal mixins that include overlapping powered entities.
- Sulfur Cube BODY-slot power from redstone blocks.
- New sulfur cube tags and translations.
Reviewed & summarized for commit 39c775c.
|
@mizius-bot review |
|
@PainterFlow, please run ingame tests to verify that everything still works as intended, then the pull request can be merged. |
mizius-bot
left a comment
There was a problem hiding this comment.
Summary
This PR adds entity-powered redstone for Sulfur Cubes. The main changes are:
- New entity redstone power storage and NBT persistence.
- Server-level powered-entity counting to skip unnecessary scans.
- Redstone signal reads that include powered entities overlapping a block.
- Neighbor updates when powered entity bounds or power changes.
- Sulfur Cube tags and translations for redstone block swallowing.
Reviewed & summarized for commit 98cf7cc.
| private void vsq$customServerAiStep(ServerLevel level, CallbackInfo ci) { | ||
| this.vsq$setRedstonePowerForContent(); |
There was a problem hiding this comment.
When the Sulfur Cube body slot changes outside customServerAiStep, the stored redstone power is not refreshed until the next AI tick. During that tick gap, circuits can read the old entity power and neighbors are not notified at the time the carried block actually changes.
Context Used: AGENTS.md (source)
| public abstract class SignalGetterMixin implements SignalGetter { | ||
| @Override | ||
| public int getSignal(BlockPos pos, Direction direction) { | ||
| int signal = SignalGetter.super.getSignal(pos, direction); |
There was a problem hiding this comment.
This override calls SignalGetter.super.getSignal(...), but getSignal(BlockPos, Direction) is the signal contract that worlds implement rather than a default helper body. When this mixin is compiled or applied, the interface-super call has no concrete method to dispatch to, so the mod can fail to build or start.
Context Used: AGENTS.md (source)
|
|
||
| @Override | ||
| public int getDirectSignal(BlockPos pos, Direction direction) { | ||
| int signal = SignalGetter.super.getDirectSignal(pos, direction); |
There was a problem hiding this comment.
Entity Power Becomes Strong Power
This adds the entity signal to getDirectSignal the same way as getSignal, so an entity overlapping a block makes that position look like it strongly powers every queried direction. Redstone dust and components that use direct signal checks can then be powered through a neighboring position as if there were a strong power source there, instead of only reading the entity signal at the intended block.
| VSQEntityRedstonePower.incrementPoweredEntityCount(level); | ||
| this.vsq$redstonePowerCounted = true; | ||
| this.vsq$redstonePowerCountedLevel = level; |
There was a problem hiding this comment.
Dimension Move Uses Stale Bounds
When a powered entity changes to another ServerLevel, this branch unregisters the old level and registers the new one but leaves vsq$previousRedstoneSourceBounds pointing at the old level's coordinates. On the first tick after the move, the bounds-change path can notify those old coordinates in the new level instead of clearing the redstone neighbors in the level the entity left, leaving circuits in the old dimension stale until another block update reaches them.

Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Add entity-powered redstone. Sulfur Cubes now emit power 15 when carrying/swallowing a redstone block, and circuits can read signals from entities overlapping a block.
vsq:powerRedstone(saved to NBT).minecraft:redstone_block→ 15, else 0; updated on AI tick and load. Tags added:sulfur_cube_swallowableandsulfur_cube_archetype/fast_flatwithminecraft:redstone_block.Written for commit 5342c18. Summary will update on new commits.