Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package io.github.thebusybiscuit.slimefun4.api.events;

import io.github.bakedlibs.dough.blocks.BlockPosition;
import io.github.thebusybiscuit.slimefun4.core.machines.MachineOperation;
import io.github.thebusybiscuit.slimefun4.core.machines.MachineProcessor;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.bukkit.Bukkit;
import org.bukkit.event.Cancellable;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;

/**
* This {@link Event} is fired whenever an {@link MachineProcessor} wants to start a {@link MachineOperation} when invoking any of the {@link MachineProcessor#startOperation} method
* The event is cancellable, if the event is cancelled, the operation will not be added to the operation map and {@link MachineProcessor#startOperation} will return false
*
* @author m1919810
*
*/
public class AsyncMachineOperationStartEvent extends Event implements Cancellable {
private static final HandlerList handlers = new HandlerList();

private final BlockPosition position;
private final MachineProcessor<?> machineProcessor;
private final MachineOperation machineOperation;
private boolean cancel;

public <T extends MachineOperation> AsyncMachineOperationStartEvent(
BlockPosition pos, MachineProcessor<T> processor, T operation) {
super(!Bukkit.isPrimaryThread());

this.position = pos;
this.machineProcessor = processor;
this.machineOperation = operation;
}

/**
* This returns the {@link BlockPosition} of the machine.
*
* @return The {@link BlockPosition} of the machine
*/
@Nonnull
public BlockPosition getPosition() {
return position;
}

/**
* The {@link MachineProcessor} instance of the machine.
*
* @return The {@link MachineProcessor} instance of the machine
*/
@Nullable public MachineProcessor<?> getProcessor() {
return machineProcessor;
}

/**
* This returns the used {@link MachineOperation} in the process.
*
* @return The {@link MachineOperation} of the process
*/
@Nullable public MachineOperation getOperation() {
return machineOperation;
}

@Nonnull
public static HandlerList getHandlerList() {
return handlers;
}

@Nonnull
@Override
public HandlerList getHandlers() {
return getHandlerList();
}

/**
* This returns whether the event is cancelled
*
* @return cancel flag
*/
@Override
public boolean isCancelled() {
return this.cancel;
}

/**
* This sets the cancel flag of the event
* If the event is cancelled, the operation will not be added to the operation map and {@link MachineProcessor#startOperation} will return false
*
* @param b new flag
*/
@Override
public void setCancelled(boolean b) {
this.cancel = b;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package io.github.thebusybiscuit.slimefun4.core.attributes;

import io.github.bakedlibs.dough.blocks.BlockPosition;
import io.github.thebusybiscuit.slimefun4.core.machines.MachineOperation;

public interface MachineProcessSerializable<T extends MachineOperation> extends MachineProcessHolder<T> {
String KEY_PROGRESS_LEFT = "p-tick-left";
String KEY_OPERATION_INFO = "p-op-info";

/**
* this called when a MachineProcessor trys to load a MachineOperation from a blockData by using the value under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO} as the operation information and {@link MachineProcessSerializable#KEY_PROGRESS_LEFT} as the progress left
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Multiple grammatical issues:

  1. "trys" should be "tries"
  2. Missing "is" before "called" at the start

Copilot uses AI. Check for mistakes.
* @param position
* @param output
* @return
*/
T deserialize(BlockPosition position, String output);

/**
* this called when a MachineProcessor trys to save a MachineOperation when startOperation called, it will store the return value under key {@link MachineProcessSerializable#KEY_OPERATION_INFO}
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Multiple grammatical issues:

  1. "trys" should be "tries"
  2. Missing "is" before "called" at the start
  3. "startOperation called" should be "startOperation is called"

Copilot uses AI. Check for mistakes.
*
* @param position
* @param operation
* @return
Comment on lines +11 to +23
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JavaDoc has incomplete parameter and return tags. The @param and @return tags are present but have no descriptions. These should be completed with proper descriptions.

Suggested change
* this called when a MachineProcessor trys to load a MachineOperation from a blockData by using the value under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO} as the operation information and {@link MachineProcessSerializable#KEY_PROGRESS_LEFT} as the progress left
* @param position
* @param output
* @return
*/
T deserialize(BlockPosition position, String output);
/**
* this called when a MachineProcessor trys to save a MachineOperation when startOperation called, it will store the return value under key {@link MachineProcessSerializable#KEY_OPERATION_INFO}
*
* @param position
* @param operation
* @return
* Called when a MachineProcessor tries to load a MachineOperation from block data,
* using the value under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO} as the operation information
* and {@link MachineProcessSerializable#KEY_PROGRESS_LEFT} as the progress left.
*
* @param position The position of the block from which to load the operation.
* @param output The serialized operation information stored in the block data.
* @return The deserialized MachineOperation instance, or {@code null} if deserialization fails.
*/
T deserialize(BlockPosition position, String output);
/**
* Called when a MachineProcessor tries to save a MachineOperation after {@code startOperation} is called.
* The return value will be stored under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO}.
*
* @param position The position of the block where the operation is being saved.
* @param operation The MachineOperation instance to serialize.
* @return A String representing the serialized operation information.

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +23
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JavaDoc has incomplete parameter and return tags. The @param and @return tags are present but have no descriptions. These should be completed with proper descriptions.

Suggested change
* this called when a MachineProcessor trys to load a MachineOperation from a blockData by using the value under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO} as the operation information and {@link MachineProcessSerializable#KEY_PROGRESS_LEFT} as the progress left
* @param position
* @param output
* @return
*/
T deserialize(BlockPosition position, String output);
/**
* this called when a MachineProcessor trys to save a MachineOperation when startOperation called, it will store the return value under key {@link MachineProcessSerializable#KEY_OPERATION_INFO}
*
* @param position
* @param operation
* @return
* Called when a MachineProcessor tries to load a MachineOperation from block data,
* using the value under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO} as the operation information
* and {@link MachineProcessSerializable#KEY_PROGRESS_LEFT} as the progress left.
*
* @param position The position of the block from which the operation is being loaded.
* @param output The serialized operation information stored in the block data.
* @return The deserialized MachineOperation instance for the given block position and output.
*/
T deserialize(BlockPosition position, String output);
/**
* Called when a MachineProcessor tries to save a MachineOperation after startOperation is called.
* The return value will be stored under the key {@link MachineProcessSerializable#KEY_OPERATION_INFO}.
*
* @param position The position of the block where the operation is being saved.
* @param operation The MachineOperation instance to be serialized.
* @return A String representing the serialized operation information to be stored.

Copilot uses AI. Check for mistakes.
*/
String serialize(BlockPosition position, T operation);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import io.github.bakedlibs.dough.blocks.BlockPosition;
import io.github.thebusybiscuit.slimefun4.core.attributes.MachineProcessHolder;
import io.github.thebusybiscuit.slimefun4.implementation.operations.CraftingOperation;
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import of CraftingOperation in the MachineOperation interface creates a circular dependency concern and breaks abstraction. The base interface should not depend on its concrete implementation. This import is only used in the serializeOperation method which itself has design issues (see other comment).

Copilot uses AI. Check for mistakes.
import org.bukkit.configuration.ConfigurationSection;

/**
* This represents a {@link MachineOperation} which is handled
Expand Down Expand Up @@ -63,4 +65,10 @@ default boolean isFinished() {
* Implement to specify behaviour that should happen in this case.
*/
default void onCancel(BlockPosition position) {}

public static String TOTAL_TICKS = "total-ticks";
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constant TOTAL_TICKS should follow Java naming conventions for constants and be declared as public static final instead of just public static. Constants should be explicitly marked as final.

Suggested change
public static String TOTAL_TICKS = "total-ticks";
public static final String TOTAL_TICKS = "total-ticks";

Copilot uses AI. Check for mistakes.

default void serializeOperation(ConfigurationSection yaml, CraftingOperation operation) {
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method has an incorrect signature. It's defined in the MachineOperation interface but takes CraftingOperation as a parameter, which is a specific implementation. This breaks the abstraction and makes the method unusable for other MachineOperation implementations.

The parameter should be generic (e.g., use this implicitly or make it generic), or this method should be moved to CraftingOperation specifically.

Suggested change
default void serializeOperation(ConfigurationSection yaml, CraftingOperation operation) {
default void serializeOperation(ConfigurationSection yaml, MachineOperation operation) {

Copilot uses AI. Check for mistakes.
yaml.set(TOTAL_TICKS, operation.getTotalTicks());
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package io.github.thebusybiscuit.slimefun4.core.machines;

import com.xzavier0722.mc.plugin.slimefun4.storage.controller.SlimefunBlockData;
import com.xzavier0722.mc.plugin.slimefun4.storage.util.StorageCacheUtils;
import io.github.bakedlibs.dough.blocks.BlockPosition;
import io.github.thebusybiscuit.slimefun4.api.events.AsyncMachineOperationFinishEvent;
import io.github.thebusybiscuit.slimefun4.api.events.AsyncMachineOperationStartEvent;
import io.github.thebusybiscuit.slimefun4.core.attributes.MachineProcessHolder;
import io.github.thebusybiscuit.slimefun4.core.attributes.MachineProcessSerializable;
import io.github.thebusybiscuit.slimefun4.utils.ChestMenuUtils;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -32,6 +37,7 @@ public class MachineProcessor<T extends MachineOperation> {

private final Map<BlockPosition, T> machines = new ConcurrentHashMap<>();
private final MachineProcessHolder<T> owner;
private final MachineProcessSerializable<T> optionalSerializer;

private ItemStack progressBar;

Expand All @@ -45,6 +51,8 @@ public MachineProcessor(@Nonnull MachineProcessHolder<T> owner) {
Validate.notNull(owner, "The MachineProcessHolder cannot be null.");

this.owner = owner;
this.optionalSerializer =
this.owner instanceof MachineProcessSerializable<T> serializable ? serializable : null;
}

/**
Expand Down Expand Up @@ -123,13 +131,47 @@ public boolean startOperation(@Nonnull Block b, @Nonnull T operation) {
* The {@link MachineOperation} to start
*
* @return Whether the {@link MachineOperation} was successfully started. This will return false if another
* {@link MachineOperation} has already been started at that {@link BlockPosition}.
* {@link MachineOperation} has already been started at that {@link BlockPosition} or the StartEvent is cancelled.
*/
public boolean startOperation(@Nonnull BlockPosition pos, @Nonnull T operation) {
Validate.notNull(pos, "The BlockPosition must not be null");
Validate.notNull(operation, "The machine operation cannot be null");
// async Machine Operation Start Event

var currentOperation = machines.computeIfAbsent(pos, (ps) -> {
// only if the current operation if absent and Event is not cancelled can we put the new operation in the
// map
// other wise it will keep null
var event = new AsyncMachineOperationStartEvent(ps, this, operation);
if (event.callEvent()) {
return operation;
} else {
return null;
}
});
Comment on lines +141 to +151
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda in computeIfAbsent has side effects (firing an event) which makes the code harder to reason about. The event is only fired when the key is absent, which is correct, but mixing computation with side effects in computeIfAbsent is a code smell. Consider checking for absence explicitly, firing the event, and then using putIfAbsent if the event is not cancelled for clearer intent.

Suggested change
var currentOperation = machines.computeIfAbsent(pos, (ps) -> {
// only if the current operation if absent and Event is not cancelled can we put the new operation in the
// map
// other wise it will keep null
var event = new AsyncMachineOperationStartEvent(ps, this, operation);
if (event.callEvent()) {
return operation;
} else {
return null;
}
});
var currentOperation = machines.get(pos);
if (currentOperation == null) {
var event = new AsyncMachineOperationStartEvent(pos, this, operation);
if (event.callEvent()) {
var prev = machines.putIfAbsent(pos, operation);
currentOperation = (prev == null) ? operation : prev;
}
}

Copilot uses AI. Check for mistakes.
// if the current Operation is successfully put into the map, returns true
if (currentOperation == operation) {
// serialize and save to blockData
if (optionalSerializer != null) {
SlimefunBlockData blockData = StorageCacheUtils.getBlock(pos.toLocation());
if (blockData != null) {
StorageCacheUtils.executeAfterLoad(
blockData,
() -> {
blockData.setData(
MachineProcessSerializable.KEY_OPERATION_INFO,
optionalSerializer.serialize(pos, operation));
blockData.setData(
MachineProcessSerializable.KEY_PROGRESS_LEFT,
String.valueOf(operation.getRemainingTicks()));
},
false);
}
}
return true;
}

return machines.putIfAbsent(pos, operation) == null;
return false;
}

/**
Expand Down Expand Up @@ -171,7 +213,55 @@ public boolean startOperation(@Nonnull BlockPosition pos, @Nonnull T operation)
@Nullable public T getOperation(@Nonnull BlockPosition pos) {
Validate.notNull(pos, "The BlockPosition must not be null");

return machines.get(pos);
T value = machines.get(pos);
if (value != null) {
if (optionalSerializer != null) {
// try update the progressLeft field
SlimefunBlockData sfdata = StorageCacheUtils.getBlock(pos.toLocation());
if (sfdata != null) {
StorageCacheUtils.executeAfterLoad(
sfdata,
() -> sfdata.setData(
MachineProcessSerializable.KEY_PROGRESS_LEFT,
String.valueOf(value.getRemainingTicks())),
false);
}
}
} else {
if (optionalSerializer != null) {
// try load if operation is absent
SlimefunBlockData sfdata = StorageCacheUtils.getBlock(pos.toLocation());
if (sfdata != null && sfdata.isDataLoaded()) {
// this may not be multithread-safe, but who cares?
String infoYaml = sfdata.getData(MachineProcessSerializable.KEY_OPERATION_INFO);
if (infoYaml != null) {
T operationLoaded;
try {
operationLoaded = Objects.requireNonNull(optionalSerializer.deserialize(pos, infoYaml));
} finally {
sfdata.removeData(MachineProcessSerializable.KEY_OPERATION_INFO);
}
Comment on lines +238 to +243
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try-finally block is misused here. The finally block always executes, which means KEY_OPERATION_INFO will be removed from block data even if deserialization fails (when deserialize returns null). This will cause data loss.

Consider using a try-catch block instead and only remove the data if deserialization succeeds, or handle the null case before removing the data.

Suggested change
T operationLoaded;
try {
operationLoaded = Objects.requireNonNull(optionalSerializer.deserialize(pos, infoYaml));
} finally {
sfdata.removeData(MachineProcessSerializable.KEY_OPERATION_INFO);
}
T operationLoaded = null;
try {
operationLoaded = Objects.requireNonNull(optionalSerializer.deserialize(pos, infoYaml));
} catch (Throwable e) {
// Deserialization failed, do not remove KEY_OPERATION_INFO
// Optionally log the error here
return;
}
sfdata.removeData(MachineProcessSerializable.KEY_OPERATION_INFO);

Copilot uses AI. Check for mistakes.
String progress = sfdata.getData(MachineProcessSerializable.KEY_PROGRESS_LEFT);
int progressTickLeft;
try {
progressTickLeft =
progress == null ? operationLoaded.getTotalTicks() : Integer.parseInt(progress);
} catch (Throwable e) {
progressTickLeft = operationLoaded.getTotalTicks();
}
operationLoaded.addProgress(operationLoaded.getTotalTicks() - progressTickLeft);
sfdata.setData(
MachineProcessSerializable.KEY_OPERATION_INFO,
optionalSerializer.serialize(pos, operationLoaded));
sfdata.setData(
MachineProcessSerializable.KEY_PROGRESS_LEFT,
String.valueOf(operationLoaded.getRemainingTicks()));
Comment on lines +253 to +258
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After deserializing the operation and restoring its progress, the operation info is immediately re-serialized and saved back to block data (lines 253-258). This defeats the purpose of removing it in the finally block at line 242, and creates unnecessary overhead. The operation should be loaded into memory and the serialized data should be removed from persistent storage, not re-saved. Consider removing lines 253-258.

Suggested change
sfdata.setData(
MachineProcessSerializable.KEY_OPERATION_INFO,
optionalSerializer.serialize(pos, operationLoaded));
sfdata.setData(
MachineProcessSerializable.KEY_PROGRESS_LEFT,
String.valueOf(operationLoaded.getRemainingTicks()));

Copilot uses AI. Check for mistakes.
machines.put(pos, operationLoaded);
}
}
}
Comment on lines +230 to +262
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition: Between checking value == null at line 230 and loading the operation (lines 236-260), another thread could have already loaded and added the operation to the map. This could result in duplicate operations being created or inconsistent state. Consider using computeIfAbsent or proper synchronization to ensure atomic check-and-load behavior.

Copilot uses AI. Check for mistakes.
}
return value;
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the old value (which is null) instead of the newly loaded operationLoaded. After successfully deserializing and adding an operation to the machines map at line 259, the method should return the loaded operation, not the initially null value. Change this to return machines.get(pos); or track the loaded operation properly.

Suggested change
return value;
return machines.get(pos);

Copilot uses AI. Check for mistakes.
}

/**
Expand Down Expand Up @@ -215,6 +305,19 @@ public boolean endOperation(@Nonnull Block b) {
*/
public boolean endOperation(@Nonnull BlockPosition pos) {
Validate.notNull(pos, "The BlockPosition cannot be null");
// remove the serialized data from the blockData
if (optionalSerializer != null) {
SlimefunBlockData sfdata = StorageCacheUtils.getBlock(pos.toLocation());
if (sfdata != null) {
StorageCacheUtils.executeAfterLoad(
sfdata,
() -> {
sfdata.removeData(MachineProcessSerializable.KEY_PROGRESS_LEFT);
sfdata.removeData(MachineProcessSerializable.KEY_OPERATION_INFO);
},
false);
}
}

T operation = machines.remove(pos);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItemStack;
import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType;
import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems;
import io.github.thebusybiscuit.slimefun4.implementation.operations.CraftingOperationSerializable;
import io.github.thebusybiscuit.slimefun4.utils.SlimefunUtils;
import me.mrCookieSlime.Slimefun.Objects.SlimefunItem.abstractItems.AContainer;
import me.mrCookieSlime.Slimefun.Objects.SlimefunItem.abstractItems.MachineRecipe;
Expand All @@ -20,7 +21,7 @@
* @author TheBusyBiscuit
*
*/
public class AutoAnvil extends AContainer {
public class AutoAnvil extends AContainer implements CraftingOperationSerializable {

private final int repairFactor;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.github.thebusybiscuit.slimefun4.api.items.settings.IntRangeSetting;
import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType;
import io.github.thebusybiscuit.slimefun4.implementation.Slimefun;
import io.github.thebusybiscuit.slimefun4.implementation.operations.CraftingOperationSerializable;
import java.util.Collections;
import java.util.List;
import javax.annotation.Nonnull;
Expand All @@ -29,7 +30,7 @@
* @see AutoDisenchanter
*
*/
abstract class AbstractEnchantmentMachine extends AContainer {
abstract class AbstractEnchantmentMachine extends AContainer implements CraftingOperationSerializable {

private final ItemSetting<Boolean> useLevelLimit = new ItemSetting<>(this, "use-enchant-level-limit", false);
private final IntRangeSetting levelLimit = new IntRangeSetting(this, "enchant-level-limit", 0, 10, Short.MAX_VALUE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItemStack;
import io.github.thebusybiscuit.slimefun4.api.items.settings.IntRangeSetting;
import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType;
import io.github.thebusybiscuit.slimefun4.implementation.operations.CraftingOperationSerializable;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nonnull;
Expand All @@ -24,7 +25,7 @@
*
* @author ProfElements
*/
public class BookBinder extends AContainer {
public class BookBinder extends AContainer implements CraftingOperationSerializable {

private final ItemSetting<Boolean> bypassVanillaMaxLevel =
new ItemSetting<>(this, "bypass-vanilla-max-level", false);
Expand Down
Loading