Skip to content

[needs testing] Fix linear actuator offset logic and fix clamps to be of more logical value #8838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: mc1.20.1/dev
Choose a base branch
from

Conversation

nopeless
Copy link

@nopeless nopeless commented Jul 2, 2025

Known issues

none atm


This is a three part fix

  1. There exists 3 clamp(getSpeed(), -.49, .49) around the code base for
  • LinearActuatorBlockEntity
  • MechanicalPistonBlockEntity
  • GantryShaftBlockEntity

Collectively, these affect:

  • PulleyBlockEntity extends LinearActuatorBE
  • MechanicalPistonBlockEntity extends LinearActuatorBE#getMovementSpeed
  • GantryShaftBlockEntity has its own clamp

However, this clamp affects rpms above 250 because convertToLinear divides rpm by 512, which means 0.49 * 512 = 250.88.

Slightly related, but this 0.49 clamp is also responsible for the incorrect block speed in the fandom wiki as they used (0.49 * 20) / 256 = 0.03828125

https://create.fandom.com/wiki/Gantry_Carriage

However, this clamp value did not make sense, so I changed it to 1 (as in maximum movement speed is 1 blocks per tick), and any value above 1 was unstable from my testing.

  1. This clamp change made it so that blocks would not properly stop at the edge of other blocks, and it was because of a nonexistent offset correction at

protected void collided() {
if (level.isClientSide) {
waitingForSpeedChange = true;
return;
}
offset = getGridOffset(offset - getMovementSpeed());
resetContraptionToOffset();
tryDisassemble();
}

where the code uses a newOffset, offset pair

if (sequencedOffsetLimit > 0) {
sequencedOffsetLimit = Math.max(0, sequencedOffsetLimit - Math.abs(movementSpeed));
locked = sequencedOffsetLimit == 0;
}
float newOffset = offset + movementSpeed;
if ((int) newOffset != (int) offset)
visitNewPosition();

  1. There exists a movement direction check for anchors and I noticed this when contraptions were stopping early in only the positive axes.

// Blocks in the world
if (movementDirection.getAxisDirection() == AxisDirection.POSITIVE)
gridPos = gridPos.relative(movementDirection);
if (isCollidingWithWorld(world, contraption, gridPos, movementDirection))
return true;

Removing this check seems to produce correct behavior

This is incorrect behavior. Imagine a contraption that is perfectly aligned with the world. It intersects only one block, not two. this is because positive direction requires ceil while negative direction requires floor. This pr implements the ceil part


I have tested RPMS up to 512 by changing the game config and rope pulleys, mechanical pistons, and gantry carriages all worked fine up to 512RPM.

However, this PR includes a rather big line change in ContraptionCollider and I have not tested how the other contraptions will behave to this change.

TL;DR: fixed offset logic which allows linear actuator contraptions to stop at the correct block, especially at high rpms, adjust clamp so that it scales linearly up to 512RPM instead of 250RPM, removed possibly broken check for contraption colliding

TL;DR 2: there were more bugs related to integer alignments, specifically when contraptions were exactly aligned with the block, causing 256rpm to not behave correctly with linear actuators. That has also been addressed

@vercte vercte added pr type: fix PR fixes a bug status: needs testing Issue needs testing labels Jul 2, 2025
@nopeless
Copy link
Author

nopeless commented Jul 2, 2025

this change broke behavior at low rpm

im looking into it

@nopeless
Copy link
Author

nopeless commented Jul 3, 2025

If anyone does want to test this, please check the following entries

  • low rpm: (1~255rpm)
  • high rpm: (256~512rpm) (you need to edit config to test above 256)
  • sequenced gearshift

against

  • rope pulleys
  • elevator pulleys
  • mechanical pistons
  • sticky mechanical pistons
  • mechanical bearing block
  • clockwork bearing block

for pulleys and pistons, make sure to also check behavior when it does not fully extend nor retract

@nopeless
Copy link
Author

nopeless commented Jul 4, 2025

I am pretty sure this pr also fixes the "up" bias that exists with rope pulleys but I only empirically verified this

@adaliszk
Copy link
Collaborator

adaliszk commented Jul 4, 2025

Hmm, would probably be also good to double-check technical hacks around them and see how those are affected. Would not be happy to say goodbye to the 2gt cobblegen :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr type: fix PR fixes a bug status: needs testing Issue needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants