feat(create): rebuild Create_GantryShaft on real carriage tracking - #25
Merged
Merged
Conversation
The carriage-detection test was inverted. It required GantryCarriageBlock.FACING to point back at the shaft; in Create it points away from it — canSurvive looks for the shaft at pos.relative(FACING.getOpposite()), and checkAttachedCarriageBlocks, isCustomConnection and propagateRotationTo all match on FACING == d. The predicate never matched a carriage on the queried rail at all. What it did match was a carriage one block away belonging to a *different* rail two blocks away, and since direction iteration starts at DOWN the practical effect was "reports the carriage below the rail, never its own" — the exact geometry of a stacked gantry. Fixing that alone still leaves the readback dead during travel, which is when it matters. Assembly anchors the contraption at the carriage's own position and removes its blocks from the world, so a moving gantry has no carriage block at all. Carriage resolution now checks for a GantryContraptionEntity first, deciding rail membership exactly as checkPinionShaft does, and only then falls back to a block scan. Positions are live end to end and fractional while moving. Two further defects in the same walk: it joined shafts on shared axis where Create requires exact facing equality (sneak-placing against a shaft deliberately yields the opposite facing, so abutting opposed rails are one keystroke away, not a corner case), and it called getBlockState up to 256 times per direction with no load check, where Level.getBlockState resolves its chunk with requireChunk = true — a rail pointing into ungenerated terrain would generate it, on the server thread, from a Lua call. Create's own rail walk guards the same way. The surface grows to match the other contraption controllers, which all had assembly state and an error readback already: getCarriage (one coherent read, nil plus reason when empty), getState distinguishing empty/parked/moving/stalled/failed where everything used to answer an undifferentiated nil, isAssembled, isStalled, getRemainingMovement, getLastAssemblyError, getRailStart, and disassemble as an emergency stop. Rail events are queued off a tick hook, polled every five ticks and only on shafts that actually have a computer attached. Also corrects docs that described a redstone mechanic backwards: powering a shaft does not invert travel, it stops translation and transmits rotation into the carriage's output shaft instead. Topology and carriage location move to GantryRail so the membership predicate — the thing standing between us and this bug — can be unit tested, including the stacked-gantry geometry that made the phantom match possible. Refs #24
Rail.contains() computed its bound-check offset as a raw coordinate delta, which only lines up with the rail's own direction of travel when facing points in the positive axis direction. On a westward, northward, or downward rail, every real position past start produced a negative offset and failed the bounds check outright -- matching only within about half a block of index 0. Carriage.position() had the identical bug, since it built the same delta by hand. Caught by manually driving a carriage down an extended rail and watching the readback go dead for the entire transit except right at the start and end -- exactly the blind spot the sign error produces. Covers both directions with negative-axis contains()/coord() tests, horizontal (west) and vertical (down).
ContraptionSurface / ContraptionReadback share getContraption(), size(), list(), getItemDetail(), getItemLimit() and tanks() across every contraption-driving peripheral -- gantry shaft, mechanical piston, mechanical bearing, rope pulley, elevator pulley -- via the same default-method pattern KineticScadaSurface already uses. getContraption() reports block count, seat count, hasBlockBreakers, current velocity, local (anchor-relative, never world) bounding box and positions, every block and active MovementBehaviour actor, every right-click interactor, the Contraption Controls filter's currently disabled actor types, every seat and every non-seated rider (both anonymized to isPlayer + entity type, never a name or UUID), and mounted-storage counts. size()/list()/getItemDetail()/getItemLimit()/ tanks() delegate straight to CC:Tweaked's own InventoryMethods / FluidMethods against MountedStorageManager's combined item and fluid handlers, so they report the same shape a script already gets from an ordinary CC inventory peripheral. Deliberately no pushItems/pullItems/pushFluid/pullFluid: vanilla Create only ever exposes a moving contraption's storage for transfer through a docked Portable Storage/Fluid Interface, and that stationary block is already a normal CC inventory/fluid_storage peripheral once connected. Reimplementing transfer here would let a script reach into any moving contraption's storage from anywhere, without the alignment or redstone gating vanilla requires. MechanicalBearingBlockEntity's movedContraption field is protected upstream (unlike the public field LinearActuatorBlockEntity gives piston/pulley, or PulleyBlockEntity#getAttachedContraption, which also resolves rope-pulley mirror children), so MechanicalBearingExt / MechanicalBearingBlockEntityMixin gained a matching @Accessor.
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
1 participant
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.
The carriage-detection test was inverted. It required GantryCarriageBlock.FACING to point back at the shaft; in Create it points away from it — canSurvive looks for the shaft at pos.relative(FACING.getOpposite()), and checkAttachedCarriageBlocks, isCustomConnection and propagateRotationTo all match on FACING == d. The predicate never matched a carriage on the queried rail at all. What it did match was a carriage one block away belonging to a different rail two blocks away, and since direction iteration starts at DOWN the practical effect was "reports the carriage below the rail, never its own" — the exact geometry of a stacked gantry.
Fixing that alone still leaves the readback dead during travel, which is when it matters. Assembly anchors the contraption at the carriage's own position and removes its blocks from the world, so a moving gantry has no carriage block at all. Carriage resolution now checks for a GantryContraptionEntity first, deciding rail membership exactly as checkPinionShaft does, and only then falls back to a block scan. Positions are live end to end and fractional while moving.
Two further defects in the same walk: it joined shafts on shared axis where Create requires exact facing equality (sneak-placing against a shaft deliberately yields the opposite facing, so abutting opposed rails are one keystroke away, not a corner case), and it called getBlockState up to 256 times per direction with no load check, where Level.getBlockState resolves its chunk with requireChunk = true — a rail pointing into ungenerated terrain would generate it, on the server thread, from a Lua call. Create's own rail walk guards the same way.
The surface grows to match the other contraption controllers, which all had assembly state and an error readback already: getCarriage (one coherent read, nil plus reason when empty), getState distinguishing empty/parked/moving/stalled/failed where everything used to answer an undifferentiated nil, isAssembled, isStalled, getRemainingMovement, getLastAssemblyError, getRailStart, and disassemble as an emergency stop. Rail events are queued off a tick hook, polled every five ticks and only on shafts that actually have a computer attached.
Also corrects docs that described a redstone mechanic backwards: powering a shaft does not invert travel, it stops translation and transmits rotation into the carriage's output shaft instead.
Topology and carriage location move to GantryRail so the membership predicate — the thing standing between us and this bug — can be unit tested, including the stacked-gantry geometry that made the phantom match possible.
Refs #24