Repository navigation
fix: detect sculk sensor clocks - #328
Merged
Merged
Conversation
The middleware read check.sculkSensor while config.yml and the feature command use check.sculk, so sculk events were always skipped. The tracking services only counted every second non redstone event and the listener passed no state, so the counter never grew. Paper fires BlockReceiveGameEvent for every vibration listener and pre-cancelled when the sensor cannot receive the vibration, including vibrations caused by players. The listener now ignores cancelled events, accepts only sculk sensors and calibrated sculk sensors, and ignores vibrations with a source entity. Every accepted activation counts as a trigger. Claude-Session: https://claude.ai/code/session_01D82BKRq4pNe6vLvbaFgR1G
Contributor
Unit Test Results 11 files 11 suites 4s ⏱️ Results for commit 438a707. ♻️ This comment has been updated with latest results. |
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.
Root causes
SkipEventTypeRedstoneClockMiddlewarereadcheck.sculkSensor, whileconfig.ymland/arcm feature check sculkusecheck.sculk. Sculk events were therefore always skipped.SculkListenerpassed no state.StaticTrackingServiceandDynamicTrackingServicecounted non redstone/hopper events only through anactivetoggle armed by the state, so nothing was ever counted. Even with a state it would have counted every second event.Paper event semantics (verified in Paper 26.2 bytecode,
VibrationSystem$Listener.handleGameEvent)!isValidVibration.BlockReceiveGameEvent(gameEvent, CraftBlock.at(listener position), sourceEntity)and callssetCancelled(!user.canReceiveVibration(...))before the occlusion check and before firing. So the event fires pre-cancelled while the sensor is active or cooling down (canReceiveVibrationrequires phase INACTIVE).getEntity()isGameEvent.Context.sourceEntity(). Block caused vibrations have none:PistonBaseBlockusesContext.of(BlockState)(null entity), and a redstone drivenNoteBlockcallsplayNote(null, ...). A real sculk clock (sensor -> redstone -> piston/note block -> sensor) is therefore not filtered out.Design
SculkListener:@EventHandler(priority = LOWEST, ignoreCancelled = true); onlySCULK_SENSORandCALIBRATED_SCULK_SENSOR(resolved by material name so apiVersion 1.19 stays supported); events with a source entity are ignored; statetrue.check.sculk.SCULK_SENSORevent likeREDSTONE_AND_REPEATERandHOPPER.check.sculkknown issue removed, command description, one explanation paragraph).Tests
SculkListenerTestandSkipEventTypeRedstoneClockMiddlewareTestwere written first and failed againstmain(6 of 8 failing: middleware skipped enabled sculk, shrieker and player events forwarded,ignoreCancelledfalse, no state, clock never reported). Green after the change;./gradlew buildpasses.ignoreCancelledis enforced by Bukkit's event bus, not by calling the method, so it is asserted via reflection on the annotation. A limit test confirms a clock is reported aftermaxCount + 2events (first event only starts observation and the limit is checked before counting, same as redstone), i.e. every activation counts rather than every second one.Threshold analysis (report only, defaults unchanged)
From the patched jar:
SculkSensorBlock.ACTIVE_TICKS = 30,COOLDOWN_TICKS = 10;CalibratedSculkSensorBlock.getActiveTicks() = 10(cooldown shared, 10). A sensor only accepts a vibration in phase INACTIVE.clock.endDelayof 300 s (6000 ticks), 151 including one at t=0 with zero travel delay.clock.maxCount = 150needs 152 events in the window, so the fastest plain sculk sensor clock (at most ~151 events, less with vibration travel time and redstone delay) does not reach it at defaults. Only calibrated sensors can. Server owners who want plain sensor clocks caught need a largerclock.endDelayor a smallerclock.maxCount.https://claude.ai/code/session_01D82BKRq4pNe6vLvbaFgR1G