Skip to content

Commit

Permalink
Fix pulse unlock mode being broken by a constant redstone signal (#8172)
Browse files Browse the repository at this point in the history 
With this change pattern providers will check if they already have a
redstone signal when locking, and if so they will wait for the signal to
depower and then power again. This fixes odd behavior with a constant
redstone signal and block updates leading to unlocking without a
'pulse'.

![2024-08-28_17 50
27](https://github.com/user-attachments/assets/f9725586-6202-42b8-bd6c-71414059f551)

Fixes #7179

---------

Co-authored-by: Sebastian Hartte <[email protected]>
(cherry picked from commit a1211bc)
(cherry picked from commit 7de2565)
  • Loading branch information
@jpenilla @shartte
jpenilla authored and shartte committed Dec 8, 2024
1 parent 6f86ce0 commit 81edee6
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 40 deletions.
24 changes: 19 additions & 5 deletions src/main/java/appeng/helpers/patternprovider/PatternProviderLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void writeToNBT(CompoundTag tag) {
this.configManager.writeToNBT(tag);
this.patternInventory.writeToNBT(tag, NBT_MEMORY_CARD_PATTERNS);
tag.putInt(NBT_PRIORITY, this.priority);
if (unlockEvent == UnlockCraftingEvent.PULSE) {
if (unlockEvent == UnlockCraftingEvent.REDSTONE_POWER) {
tag.putByte(NBT_UNLOCK_EVENT, (byte) 1);
} else if (unlockEvent == UnlockCraftingEvent.RESULT) {
if (unlockStack != null) {
Expand All @@ -169,6 +169,8 @@ public void writeToNBT(CompoundTag tag) {
} else {
LOGGER.error("Saving pattern provider {}, locked waiting for stack, but stack is null!", host);
}
} else if (unlockEvent == UnlockCraftingEvent.REDSTONE_PULSE) {
tag.putByte(NBT_UNLOCK_EVENT, (byte) 3);
}

ListTag sendListTag = new ListTag();
Expand All @@ -191,8 +193,9 @@ public void readFromNBT(CompoundTag tag) {
var unlockEventType = tag.getByte(NBT_UNLOCK_EVENT);
this.unlockEvent = switch (unlockEventType) {
case 0 -> null;
case 1 -> UnlockCraftingEvent.PULSE;
case 1 -> UnlockCraftingEvent.REDSTONE_POWER;
case 2 -> UnlockCraftingEvent.RESULT;
case 3 -> UnlockCraftingEvent.REDSTONE_PULSE;
default -> {
LOGGER.error("Unknown unlock event type {} in NBT for pattern provider: {}", unlockEventType, tag);
yield null;
Expand Down Expand Up @@ -377,7 +380,14 @@ private void onPushPatternSuccess(IPatternDetails pattern) {
var lockMode = configManager.getSetting(Settings.LOCK_CRAFTING_MODE);
switch (lockMode) {
case LOCK_UNTIL_PULSE -> {
unlockEvent = UnlockCraftingEvent.PULSE;
if (getRedstoneState()) {
// Already have signal, wait for no signal before switching to REDSTONE_POWER
unlockEvent = UnlockCraftingEvent.REDSTONE_PULSE;
} else {
// No signal, wait for signal
unlockEvent = UnlockCraftingEvent.REDSTONE_POWER;
}
redstoneState = YesNo.UNDECIDED; // Check redstone state again next update
saveChanges();
}
case LOCK_UNTIL_RESULT -> {
Expand All @@ -403,7 +413,7 @@ public LockCraftingMode getCraftingLockedReason() {
} else if (unlockEvent != null) {
// Crafting locked by waiting for unlock event
switch (unlockEvent) {
case PULSE -> {
case REDSTONE_POWER, REDSTONE_PULSE -> {
return LockCraftingMode.LOCK_UNTIL_PULSE;
}
case RESULT -> {
Expand Down Expand Up @@ -754,9 +764,13 @@ public IGrid getGrid() {

public void updateRedstoneState() {
// If we're waiting for a pulse, update immediately
if (unlockEvent == UnlockCraftingEvent.PULSE && getRedstoneState()) {
if (unlockEvent == UnlockCraftingEvent.REDSTONE_POWER && getRedstoneState()) {
unlockEvent = null; // Unlocked!
saveChanges();
} else if (unlockEvent == UnlockCraftingEvent.REDSTONE_PULSE && !getRedstoneState()) {
unlockEvent = UnlockCraftingEvent.REDSTONE_POWER; // Wait for re-power
redstoneState = YesNo.UNDECIDED; // Need to re-check signal on next update
saveChanges();
} else {
// Otherwise, just reset back to undecided
redstoneState = YesNo.UNDECIDED;
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/appeng/helpers/patternprovider/UnlockCraftingEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* The types of event that the pattern provider is waiting for to unlock crafting again.
*/
public enum UnlockCraftingEvent {
PULSE,
RESULT
REDSTONE_POWER, // Waiting for redstone to be on (pulse blocking mode)
REDSTONE_PULSE, // Waiting for redstone to turn off and back on (pulse blocking mode, already powered when craft
// started)
RESULT // Waiting for result
}
107 changes: 74 additions & 33 deletions src/main/java/appeng/server/testplots/PatternProviderLockModePlots.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ public final class PatternProviderLockModePlots {
private PatternProviderLockModePlots() {
}

/**
* Regression test for #7179 When there was a high redstone signal to begin with, pulse should wait until that
* signal is low before unlocking due to a high signal.
*/
@TestPlot("pp_block_lockmode_pulse_starting_high")
public static void testBlockLockModePulseStartingHigh(PlotBuilder plot) {
setup(plot, false, LockCraftingMode.LOCK_UNTIL_PULSE);
testLockModePulse(plot, true);
}

@TestPlot("pp_block_lockmode_pulse")
public static void testBlockLockModePulse(PlotBuilder plot) {
setup(plot, false, LockCraftingMode.LOCK_UNTIL_PULSE);
Expand All @@ -51,43 +61,74 @@ public static void testPartLockModePulse(PlotBuilder plot) {
}

private static void testLockModePulse(PlotBuilder plot) {
testLockModePulse(plot, false);
}

private static void testLockModePulse(PlotBuilder plot, boolean startHigh) {
plot.test(helper -> {
var host = getHost(helper);
var pp = host.getLogic();

helper.startSequence()
.thenExecuteAfter(1, () -> {
// Initially it should be unlocked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.NONE, pp.getCraftingLockedReason());
// Pushing pattern should succeed
helper.check(pushPattern(host), "Pushing pattern failed");
// Now it should be immediately locked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
// Pushing another pattern should fail
helper.check(!pushPattern(host), "Pushing pattern should fail");
})
.thenExecuteAfter(1, () -> {
// Turn the lever on to trigger the pulse
helper.pullLever(LEVER_POS);

// That should immediately unlock the provider
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.NONE, pp.getCraftingLockedReason());

// Pushing a pattern should succeed now
helper.check(pushPattern(host), "Pushing pattern failed");

// But that should lock it again, even though the signal is still high
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
helper.check(!pushPattern(host), "Pushing pattern should fail");

// Turning the lever off should not trigger the pulse
helper.pullLever(LEVER_POS);
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
helper.check(!pushPattern(host), "Pushing pattern should fail");
})
var sequence = helper.startSequence();

if (startHigh) {
sequence.thenExecute(() -> {
// Toggle the lever on initially if we should start in high
helper.pullLever(LEVER_POS);
});
}

sequence.thenExecuteAfter(1, () -> {
// Initially it should be unlocked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.NONE, pp.getCraftingLockedReason());
// Pushing pattern should succeed
helper.check(pushPattern(host), "Pushing pattern failed");
// Now it should be immediately locked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
// Pushing another pattern should fail
helper.check(!pushPattern(host), "Pushing pattern should fail");
});

if (startHigh) {
sequence.thenExecuteAfter(1, () -> {
// Instead of pulling the lever, trigger a block update
helper.getLevel().updateNeighborsAt(helper.absolutePos(BlockPos.ZERO.above()), Blocks.CHEST);

// The pattern provider should still be locked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());

// Now turn the lever off
helper.pullLever(LEVER_POS);

// The pattern provider should still be locked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
});
}

sequence.thenExecuteAfter(1, () -> {
// Turn the lever on to trigger the pulse
helper.pullLever(LEVER_POS);

// That should immediately unlock the provider
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.NONE, pp.getCraftingLockedReason());

// Pushing a pattern should succeed now
helper.check(pushPattern(host), "Pushing pattern failed");

// But that should lock it again, even though the signal is still high
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
helper.check(!pushPattern(host), "Pushing pattern should fail");

// Turning the lever off should not trigger the pulse
helper.pullLever(LEVER_POS);
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
pp.getCraftingLockedReason());
helper.check(!pushPattern(host), "Pushing pattern should fail");
})
.thenExecuteAfter(1, () -> {
// Precondition is that it's still locked
helper.assertEquals(BlockPos.ZERO, LockCraftingMode.LOCK_UNTIL_PULSE,
Expand Down

0 comments on commit 81edee6

Please sign in to comment.