server/world: world-owned redstone engine#1250
Open
HashimTheArab wants to merge 24 commits into
Open
Conversation
Move existing redstone behavior behind a world-level engine while keeping this PR limited to current redstone-capable blocks and compatibility behavior. Constraint: core-only refactor of existing redstone wire, redstone block, lever, redstone torch, and note block behavior; no new gameplay blocks. Rejected: adding button, pressure plate, redstone lamp, or other redstone content | outside core architecture scope. Rejected: collapsing existing block files into one redstone file | makes the review and future feature PRs harder to isolate. Confidence: high Scope-risk: broad Directive: keep new redstone blocks in follow-up feature PRs and preserve weak/direct/strong power separation. Tested: go test ./server/block ./server/world; go test ./...; git diff --check; core-only grep for button/pressure plate/lamp additions Not-tested: live Bedrock multiplayer/client visual timing beyond manual user spot checks
Keep burned-out redstone torches from recovering from their own loop propagation while still allowing later external input removal to relight them. Constraint: fix existing torch burnout behavior only; no new redstone content. Rejected: clearing burnout whenever input power drops | self-clock loops drop their own input immediately after burnout. Rejected: relying only on direct ScheduledTick tests | missed the world scheduler and graph propagation path. Confidence: high Scope-risk: narrow Directive: burnout recovery must distinguish self-caused loop updates from external input removal. Tested: go test ./server/block ./server/world; go test ./...; git diff --check; core-only grep for button/pressure plate/lamp additions Not-tested: visual Bedrock client timing after push
Fix redstone torch burnout to trigger at the configured eighth self turn-off and reload live torch state before scheduled mutation so delayed updates cannot act on stale receivers. Constraint: Minecraft Wiki documents burnout after more than eight state changes within 60 game ticks; this engine tracks forced turn-offs as the burnout signal. Rejected: Leave strict greater-than threshold | It requires a ninth recorded turn-off and contradicts the configured threshold. Confidence: high Scope-risk: narrow Directive: Keep scheduled torch mutation based on live world state, not caller-held block values. Tested: go test ./server/block ./server/world Not-tested: Full manual in-game redstone smoke pass
Make runtime redstone behavior match vanilla-facing expectations for legacy torch states, vertical dust steps, TNT ignition, and action side effects while preserving the new graph scheduler contract. Constraint: PR #28 intentionally accepts the public redstone API break, so this follow-up focuses on correctness and parity issues raised during review. Rejected: Run basic RedstonePowerAction on every dirty same-power evaluation | It repeats side effects for no power transition and makes TNT/action blocks unsafe under ordinary neighbour invalidation. Confidence: high Scope-risk: moderate Directive: Keep context-aware actions eligible for dirty same-power refreshes, but basic old/new power actions must only run on real power transitions. Tested: go test ./server/block -run 'TestTNT|TestRedstoneTorchUnknown|TestRedstoneTorchTurnsOff|TestRedstoneTorchBurnsOut' -count=1 Tested: go test ./server/world -run 'TestRedstoneActionOnlyRunsOnPowerChange|TestRedstoneCancelledActionDoesNotRun|TestRedstoneStepFace|TestRedstoneTorchBurnoutRecoveryUsesRollingWindow' -count=1 Tested: go test ./server/block ./server/world Tested: go test ./... Not-tested: Manual Bedrock client redstone parity pass
Expose one world-level redstone clamp helper so block and engine code use the same vanilla 0-15 signal boundary. Constraint: Cursor Bugbot flagged duplicate newly introduced clamp logic across block and world packages. Rejected: Keep separate block-local clamp helper | It leaves two implementations of the same redstone signal bound and keeps the review thread actionable. Confidence: high Scope-risk: narrow Directive: Reuse ClampRedstonePower for redstone signal bounds instead of reintroducing package-local copies. Tested: go test ./server/block -run 'TestRedstoneWire|TestTNT' -count=1 Tested: go test ./server/world -run 'TestClampRedstonePower|TestRedstoneActionOnlyRunsOnPowerChange|TestRedstoneStepFace' -count=1 Tested: go test ./server/block ./server/world Not-tested: Manual Bedrock client redstone parity pass
Close low-risk review hardening around dirty chunk cleanup, clearer burnout assertions, test block identity, and transaction redstone wording. Constraint: Completion audit found valid older CodeRabbit top-level suggestions that were not active inline threads but still reduced review confidence. Rejected: Leave removeChunk matching dirty.changed without hasChanged | It can drop unrelated dirty entries whose changed position is only the zero value. Confidence: high Scope-risk: narrow Directive: Dirty cleanup by changed position must only use changed when hasChanged is true. Tested: go test ./server/world -run 'TestRedstoneTorchBurnoutRecoveryUsesRollingWindow|TestRedstoneEngineInvalidateAround|TestRedstoneEngineRemoveChunkKeepsUnchangedDirtyOutsideChunk|TestScheduledTickQueue|TestRedstoneStrongPowerConductorExcludesMarkedNonConductors' -count=1 Tested: go test ./server/world Not-tested: Manual Bedrock client redstone parity pass
Add regression coverage for the vanilla behavior where a redstone torch attached to a redstone block sees its attachment as powered. Constraint: Cursor Bugbot reported the existing behavior as incorrect, but Minecraft Wiki describes redstone torches as deactivating when their attachment block is powered and redstone blocks as permanently powered blocks. Rejected: Change torch attachmentPowered to ignore RedstoneBlock's own source power | That would contradict vanilla redstone behavior and make attached torches stay lit incorrectly. Confidence: high Scope-risk: narrow Directive: Do not remove redstone-block attachment power from torch inversion without new vanilla parity evidence. Tested: go test ./server/block -run 'TestRedstoneTorchTurnsOffOnRedstoneBlockAttachment|TestRedstoneTorchTurnsOffOnPoweredConductiveAttachment|TestRedstoneTorchUnknown' -count=1 Not-tested: Manual Bedrock client redstone parity pass
Remove unused comparator-facing API surface and add regression coverage proving lever changes still reach consumers behind the attached block through graph invalidation. Constraint: Cursor reported lever propagation, wire step-down, and unused comparator interface findings on the latest PR review. Rejected: Change wire step-down support semantics | Existing Bedrock/glowstone tests and Minecraft Wiki parity support checking the upper dust support block for downward transmission. Rejected: Reintroduce directional lever propagation | The graph compiler already reaches redstone behind dirty conductive neighbours, and the regression test proves the path. Confidence: high Scope-risk: narrow Directive: Keep lever propagation covered by a ticked consumer-behind-attachment regression. Tested: go test ./server/block -run 'TestLeverUpdatesConsumerBehindAttachedBlock|TestLeverStrongPowersAttachedBlockFace|TestRedstoneWireTransmitsDownGlassInBedrock|TestRedstoneWireTransmitsUpButNotDownGlowstone' -count=1 Tested: go test ./server/world -run TestClampRedstonePower -count=1 Tested: go test ./server/block ./server/world Tested: go test ./... Not-tested: Manual Bedrock client redstone parity pass
Keep the redstone review fixes easier to read by removing redundant clamp and action bookkeeping while preserving the tested runtime behavior. Constraint: Code simplifier pass was limited to the recent redstone PR files and behavior-preserving cleanup. Rejected: Broaden cleanup into test abstraction or API reshaping | The existing tests are explicit parity fixtures and the PR already accepted only the intentional redstone API break. Confidence: high Scope-risk: narrow Directive: Keep redstone clamp calls routed through world.ClampRedstonePower rather than reintroducing block-local wrappers. Tested: git diff --check Tested: go test -count=1 ./server/block ./server/world Tested: go test ./... Not-tested: Manual Bedrock client redstone parity pass
Keep the redstone core focused on capabilities that have current production users, while preserving the context-action path needed by torch burnout and wire discovery. Constraint: Cursor reported wire discovery omitted RedstonePowerContextAction, and review flagged unused redstone API and graph hashing as speculative overhead. Rejected: Keep NetworkID and redstoneGraphID for possible future diagnostics | No caller consumes them today, so the hash is per-tick overhead without behavior value. Rejected: Keep transition/sound/stored-power interfaces for later comparator or sound features | Dragonfly conventions favor adding exported interfaces with the feature that needs them. Confidence: high Scope-risk: moderate Directive: Re-add removed redstone interfaces only with a production block or handler that consumes them. Tested: git diff --check Tested: go test -count=1 ./server/block ./server/world Tested: go test -run '^$' -bench 'BenchmarkRedstoneDirtyTickLongLineWithClocks' ./server/world Tested: go test ./... Not-tested: Manual Bedrock client redstone parity pass
Remove direction parameters the graph solver cannot honor consistently, so relayer loss is modeled as one edge cost across both compiled graph and direct power probing paths. Constraint: Cursor flagged compileEdges passing a synthetic from-face that could diverge from powerFrom for any future direction-sensitive relayer. Rejected: Keep from/to parameters with a clarifying comment | The current graph edge model has no actual entry-face state, so documenting the mismatch would preserve a misleading API. Rejected: Change glowstone/glass wire step-down and step-up semantics | Existing Bedrock parity tests require glowstone ladders to transmit upward, glowstone not to transmit downward, and glass to transmit downward. Confidence: high Scope-risk: narrow Directive: Add directional redstone relayer behavior only with a graph model that carries entry direction through propagation. Tested: git diff --check Tested: go test -count=1 ./server/block -run 'TestRedstoneWireTransmitsUpButNotDownGlowstone|TestRedstoneWireTransmitsDownGlassInBedrock|TestRedstoneWireGlowstoneLadderDoesNotOscillateAfterNeighbourBlockUpdate' Tested: go test -count=1 ./server/world -run 'TestRedstoneRelayerToSinkDoesNotLosePower|TestRedstoneVerticalRelayerPropagation' Tested: go test -count=1 ./server/block ./server/world Tested: go test -run '^$' -bench 'BenchmarkRedstoneDirtyTickLongLineWithClocks' ./server/world Tested: go test ./... Not-tested: Manual Bedrock client redstone parity pass
Keep the redstone base small by removing a discarded action return value, documenting intentional Tx query methods, sharing conductor classification through one exported helper, and avoiding unnecessary old-block reads where redstone updates are suppressed. Constraint: PR 28 is a redstone core/base change, so exported surfaces must be intentional rather than speculative. Rejected: Reintroducing unused future-facing redstone interfaces | they encode unproven repeater/comparator/piston design before implementation needs them. Confidence: high Scope-risk: moderate Directive: Add future redstone extension interfaces with the block feature that first consumes them. Tested: git diff --check; go test -count=1 ./server/block ./server/world; go test ./... Not-tested: External downstream custom block implementations.
Allow burned-out torches to recover from scheduled redstone updates caused by other components while continuing to suppress self-triggered scheduled recovery loops. Constraint: Cursor found scheduled tick recovery was filtered by cause instead of source identity. Rejected: Rejecting all scheduled-tick recovery updates | it blocks valid external input changes from recovering a burned-out torch. Confidence: high Scope-risk: narrow Directive: Use RedstoneUpdate source metadata, not only cause, when distinguishing self-triggered redstone updates. Tested: git diff --check; go test -count=1 ./server/block -run 'TestBurnedOutRedstoneTorch'; go test -count=1 ./server/block ./server/world; go test ./... Not-tested: Live multi-torch in-game contraption timing.
Record cqdetdev's initial redstone refactor commit in this PR branch ancestry while keeping the reviewed redstone-core tree unchanged. Constraint: Preserve contributor credit without replacing the current reviewed implementation. Rejected: Replaying the branch content over current work | it would reintroduce already-reviewed older refactor code. Confidence: high Scope-risk: narrow Directive: Treat this merge as history-only unless intentionally revisiting the older refactor content. Tested: Pre-merge and post-merge tree hash both 14cf5e1; 2a409ac is an ancestor of HEAD. Not-tested: Tests not rerun for the merge commit because the tree is byte-identical to the previously tested head.
Table-drive repeated redstone test cases and remove one duplicate burned-out torch self-recovery check while preserving the same behavioral coverage. Constraint: Keep regression coverage for Cursor findings and Bedrock parity cases while reducing review noise. Rejected: Table-driving the larger end-to-end burnout scheduler file | those cases use distinct setup and timing paths, and forcing one table would obscure the behavior under test. Confidence: high Scope-risk: narrow Directive: Prefer adding new redstone regressions as cases in existing tables when setup and assertions match. Tested: git diff --check; go test -count=1 ./server/block ./server/world; go test ./... Not-tested: Benchmark run, since this only refactors tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Credit to @cqdetdev for 90% of this, I cleaned it up, fixed bugs, and made it more consistent.
Why
The previous per-block redstone model (each block recomputes and re-propagates power itself via
RedstoneUpdate/updateAround…, with a separate BFS for wire) was fine for the basic components dragonfly has today — wire, torch, lever, redstone block, note, TNT — but it wouldn't have scaled to the more complex ones (repeaters, comparators, pistons, observers):Tx.RedstonePower, eachConductor, the wire BFS, and the individual blocks, with slightly different copies in each: weak vs. strong vs. conducted power, dust step up/down, "weakly powered block activates a mechanism but not dust", etc.HandleRedstoneUpdate(pos)got a bare position — no before/after/power context — and cancelling it didn't reliably stop downstream propagation.This PR replaces that with a single world-owned engine that compiles a deterministic power graph once per tick and applies mutations through a cancellable
HandleRedstoneUpdate(RedstoneUpdate)event — keeping the scope to the components that already exist, but making the complex ones a matter of implementing an interface rather than reinventing propagation.What
A
redstoneEngineowned byWorldthat, once per tick, takes the set of changed positions, compiles a connected power graph (sources / relayers / consumers / actions), solves power across it (direct / strong / conducted, with per-edge signal loss through relayers), and applies the resulting block-state changes and side effects — each routed throughHandleRedstoneUpdate(RedstoneUpdate), which can cancel both the mutation and its propagation.Blocks no longer drive redstone; they declare capabilities:
RedstonePowerSource/RedstoneStrongPowerSource/RedstoneWeakBlockPowerer— emits power.RedstonePowerRelayer(+RedstonePowerRelayerNeighbourer) — carries power, defines its signal loss / topology (redstone dust).RedstonePowerConsumer(+RedstonePowerPostUpdater) — block state follows input power (note block, doors, …).RedstonePowerAction/RedstonePowerContextAction— does something on a power edge (TNT priming).RedstoneNonConductive— solid block that doesn't conduct strong power (glowstone, TNT).Tx.Redstone()exposes the engine within a transaction for scheduling re-evaluations, querying power, and managing transient redstone-torch burnout state.Existing components migrated onto it:
RedstonePower/RedstoneStrongPower; manualupdateAround…calls removed.RedstonePowerAction; markedRedstoneNonConductive.RedstoneNonConductive; emits full light.Removed: the old
server/block/redstone.goBFS updater,server/world/redstone/state.go, and theworld.Conductor/WeakBlockPowerer/RedstoneUpdater/ oldRedstonePowerRelayerinterfaces.World wiring: engine created with the world, ticked after neighbour updates, invalidated on block/liquid changes, and cleared on chunk unload;
SetOpts.DisableRedstoneUpdateslets the engine apply its own writes without re-triggering itself. Also fixes a couple ofscheduledTickQueuebookkeeping bugs where stalefurthestTicksentries could block re-scheduling.Benefits
ClampRedstonePower, andRedstoneFullPowerConductorlive inworldinstead of being re-implemented per block.HandleRedstoneUpdatecarries the position, changed neighbour, source, before/after blocks, old/new power, tick, and cause; cancelling suppresses the mutation and downstream propagation.Breaking changes
world.Handler.HandleRedstoneUpdate(ctx, pos)→HandleRedstoneUpdate(ctx, RedstoneUpdate).world.Conductor,world.WeakBlockPowerer,world.RedstoneUpdater, the oldworld.RedstonePowerRelayer, andTx.RedstonePower(pos, face, accountForDust)— replaced by the capability interfaces above andTx.Redstone().Scope
Intentionally limited to the redstone components dragonfly already supports (wire, torch, lever, redstone block, note, TNT). Repeaters, comparators, pistons, observers, buttons, pressure plates, redstone lamps, etc. are left for follow-up feature PRs — the engine is built so those can be added as graph endpoints + scheduled ticks without reworking it.
Testing
New wire / torch / lever / redstone-block / TNT integration tests plus a long-wire-with-clocks benchmark.