-
-
Notifications
You must be signed in to change notification settings - Fork 71
支持 Folia #1059
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
base: dev
Are you sure you want to change the base?
支持 Folia #1059
Conversation
# Conflicts: # src/main/java/city/norain/slimefun4/utils/TaskUtil.java # src/main/java/com/xzavier0722/mc/plugin/slimefun4/storage/controller/BlockDataController.java
# Conflicts: # src/main/java/com/xzavier0722/mc/plugin/slimefun4/storage/controller/BlockDataController.java
# Conflicts: # src/main/java/com/xzavier0722/mc/plugin/slimefun4/storage/controller/BlockDataController.java
Folia remove support for player respawn related event
src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java
Show resolved
Hide resolved
… machine refactor(ticker): adjust thread pool argument for better load balanced performance
# Conflicts: # src/main/java/me/mrCookieSlime/CSCoreLibPlugin/general/Inventory/ChestMenu.java
|
是鸽了吗 |
没有,只是需要自己编译,反馈的相关问题也会处理的 |
# Conflicts: # src/main/java/com/xzavier0722/mc/plugin/slimefun4/storage/controller/BlockDataController.java # src/main/java/com/xzavier0722/mc/plugin/slimefun4/storage/util/StorageCacheUtils.java # src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java # src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/TeleportationManager.java # src/main/java/io/github/thebusybiscuit/slimefun4/core/services/holograms/HologramsService.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/geo/GEOMiner.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/AsyncRecipeChoiceTask.java # src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds Folia support to Slimefun, a major architectural change that migrates the plugin from Bukkit's traditional scheduler to a region-based scheduler using the FoliaLib library. The PR addresses multi-threading concerns and fixes issue #1082 related to multi-block structures not working properly on Folia.
Key changes:
- Migrates all scheduler calls from BukkitScheduler to FoliaLib's PlatformScheduler with location/entity context
- Introduces concurrent execution support for BlockTickers with a thread pool architecture
- Replaces thread-unsafe data structures with concurrent alternatives (ConcurrentHashMap, CopyOnWriteArraySet, etc.)
- Implements a custom TaskQueue for Folia-compatible sequential task execution
Reviewed changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin.yml | Adds folia-supported: true flag to declare Folia compatibility |
| config.yml | Introduces thread pool configuration for async ticker system |
| Slimefun.java | Integrates FoliaLib and adds location/entity-aware scheduling methods |
| TickerTask.java | Implements concurrent ticker execution with thread pool and fallback mechanism |
| TaskQueue.java | New Folia-compatible task queue with location/entity context support |
| BlockTicker.java | Adds concurrent safety API (isConcurrent(), useUniversalData()) |
| Multiple task files | Updates all scheduler calls to use PlatformScheduler with proper context |
| Multiple data structures | Replaces HashMap/HashSet with ConcurrentHashMap/CopyOnWriteArraySet |
| ChestMenu.java | Replaces AtomicBoolean lock with ReentrantLock for proper mutex behavior |
| pom.xml | Adds FoliaLib dependency and relocation configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Slimefun.isFolia()) { | ||
| final CompletableFuture<T> result = new CompletableFuture<>(); | ||
|
|
||
| System.out.println("Sync task created, hashcode = " + result.hashCode()); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statement should be removed before merging to production. This will spam the console with synchronization task information.
| change = true; | ||
| } | ||
|
|
||
| var poolSize = Slimefun.getCfg().getInt("URID.custom-async-ticker.pool-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key used here ('URID.custom-async-ticker.pool-size') does not match the key defined in config.yml ('URID.custom-async-ticker.queue-size'). This inconsistency will cause the configuration value to be read incorrectly, potentially using a default value of 0 and triggering the validation warning.
| Slimefun.logger().log(Level.WARNING, "当前设置的 Ticker 线程池初始大小过大,已被重设至 {0},建议修改为小于 {1} 的值。", new Object[] { | ||
| maxSize, maxSize - 1 | ||
| }); | ||
| Slimefun.getCfg().setValue("URID.custom.async-ticker.init-size", initSize); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key used for saving ('URID.custom.async-ticker.init-size') has a dot between 'custom' and 'async-ticker', which doesn't match the key used for reading ('URID.custom-async-ticker.init-size' without the dot). This will cause the corrected value to be saved to a different location than where it's read from.
| "URID.custom.async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | ||
| pluginConfig.setDefaultValue( | ||
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | ||
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | ||
|
|
||
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | ||
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | ||
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom.async-ticker.init-size' contains an extra dot between 'custom' and 'async-ticker'. This should be 'URID.custom-async-ticker.init-size' to match the key used in config.yml and when reading the configuration value.
| "URID.custom.async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | |
| pluginConfig.setDefaultValue( | |
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); | |
| "URID.custom-async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | |
| pluginConfig.setDefaultValue( | |
| "URID.custom-async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom-async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom-async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom-async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom-async-ticker.queue-size"); |
| "URID.custom.async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | ||
| pluginConfig.setDefaultValue( | ||
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | ||
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | ||
|
|
||
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | ||
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | ||
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom.async-ticker.init-size' contains an extra dot between 'custom' and 'async-ticker'. This should be 'URID.custom-async-ticker.init-size' to match the key defined as the default and used elsewhere in the codebase.
| "URID.custom.async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | |
| pluginConfig.setDefaultValue( | |
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); | |
| "URID.custom-async-ticker.init-size", Runtime.getRuntime().availableProcessors() / 2); | |
| pluginConfig.setDefaultValue( | |
| "URID.custom-async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom-async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom-async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom-async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom-async-ticker.queue-size"); |
| if (poolSize < 0) { | ||
| Slimefun.logger().log(Level.WARNING, "当前设置的 Ticker 线程池任务队列大小异常,已自动设置为 1024,请修改为一个正常的大小"); | ||
| poolSize = 512; | ||
| Slimefun.getCfg().setValue("URID.custom-async-ticker.pool-size", poolSize); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom-async-ticker.pool-size' used for saving doesn't match the key 'URID.custom-async-ticker.queue-size' defined in config.yml and used in SlimefunConfigManager. This inconsistency will save the value to the wrong location.
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | ||
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | ||
|
|
||
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | ||
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom.async-ticker.max-size' contains an extra dot between 'custom' and 'async-ticker'. This should be 'URID.custom-async-ticker.max-size' to match the key used in config.yml and when reading the configuration value in TickerTask.
| "URID.custom.async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| "URID.custom-async-ticker.max-size", Runtime.getRuntime().availableProcessors()); | |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom-async-ticker.max-size"); |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | ||
|
|
||
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | ||
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | ||
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom.async-ticker.queue-size' contains an extra dot between 'custom' and 'async-ticker'. This should be 'URID.custom-async-ticker.queue-size' to match the key used in config.yml.
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); | |
| pluginConfig.setDefaultValue("URID.custom-async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom-async-ticker.queue-size"); |
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | ||
|
|
||
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | ||
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | ||
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration key 'URID.custom.async-ticker.queue-size' contains an extra dot between 'custom' and 'async-ticker'. This should be 'URID.custom-async-ticker.queue-size' to match the key defined as the default and used elsewhere in the codebase.
| pluginConfig.setDefaultValue("URID.custom.async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom.async-ticker.queue-size"); | |
| pluginConfig.setDefaultValue("URID.custom-async-ticker.queue-size", 1024); | |
| asyncTickerInitSize = pluginConfig.getInt("URID.custom.async-ticker.init-size"); | |
| asyncTickerMaxSize = pluginConfig.getInt("URID.custom.async-ticker.max-size"); | |
| asyncTickerQueueSize = pluginConfig.getInt("URID.custom-async-ticker.queue-size"); |
| var poolSize = Slimefun.getCfg().getInt("URID.custom-async-ticker.pool-size"); | ||
|
|
||
| if (poolSize < 0) { | ||
| Slimefun.logger().log(Level.WARNING, "当前设置的 Ticker 线程池任务队列大小异常,已自动设置为 1024,请修改为一个正常的大小"); |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message mentions 'queue-size' but should say 'pool-size' based on the variable name and context. However, note that there's also a configuration key mismatch issue where 'pool-size' is used in code but 'queue-size' is defined in config.yml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Slimefun.isFolia()) { | ||
| final CompletableFuture<T> result = new CompletableFuture<>(); | ||
|
|
||
| System.out.println("Sync task created, hashcode = " + result.hashCode()); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debug print statement should be removed before merging to production. Debug statements left in production code can clutter logs and potentially expose sensitive information.
| } | ||
|
|
||
| if (asyncTickerQueueSize < 0) { | ||
| asyncTickerInitSize = 1024; |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name 'asyncTickerInitSize' is being incorrectly assigned when 'asyncTickerQueueSize' validation fails. This should set 'asyncTickerQueueSize' to 1024, not 'asyncTickerInitSize'. This is a copy-paste error that will cause incorrect configuration behavior.
| asyncTickerInitSize = 1024; | |
| asyncTickerQueueSize = 1024; |
| public boolean locked() { | ||
| return lock.get(); | ||
| return lock.isLocked(); | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChestMenu lock mechanism has been changed from AtomicBoolean to ReentrantLock, but the 'locked()' method only checks if the lock is currently held by any thread. This doesn't match the previous semantics where the lock was a boolean flag. If no thread holds the lock, 'isLocked()' returns false even if lock() was called. Consider using a boolean field alongside the ReentrantLock if you need to track lock state independently of actual acquisition, or ensure callers understand that 'locked()' only indicates current lock ownership.
| public void shutdown() { | ||
| setPaused(true); | ||
| halt(); | ||
|
|
||
| try { | ||
| asyncTickerService.shutdown(); | ||
| if (!asyncTickerService.awaitTermination(10, TimeUnit.SECONDS)) { | ||
| asyncTickerService.shutdownNow(); | ||
| } | ||
| } catch (InterruptedException e) { | ||
| asyncTickerService.shutdownNow(); | ||
| } finally { | ||
| asyncTickerService = null; | ||
| } | ||
|
|
||
| try { | ||
| fallbackTickerService.shutdown(); | ||
| if (!fallbackTickerService.awaitTermination(10, TimeUnit.SECONDS)) { | ||
| fallbackTickerService.shutdownNow(); | ||
| } | ||
| } catch (InterruptedException e) { | ||
| fallbackTickerService.shutdownNow(); | ||
| } finally { | ||
| fallbackTickerService = null; | ||
| } | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shutdown logic in TickerTask may cause issues during server shutdown. After calling 'setPaused(true)' and 'halt()', the running flag is set but tasks might still be executing in the thread pools. The 10-second timeout for awaitTermination may not be sufficient for all tasks to complete, especially if they're waiting on I/O or other operations. Consider adding proper task cancellation mechanisms or increasing the timeout. Also, setting the executor service references to null in finally blocks could cause NullPointerExceptions if shutdown is called multiple times.
| this.asyncTickerService = new SlimefunPoolExecutor( | ||
| "Slimefun-Ticker-Pool", | ||
| initSize - 1, | ||
| maxSize - 1, | ||
| 1, | ||
| TimeUnit.MINUTES, | ||
| new LinkedBlockingQueue<>(poolSize), | ||
| tickerThreadFactory, | ||
| (r, e) -> { | ||
| // 任务队列已满,使用备用的单线程池执行该任务 | ||
| fallbackTickerService.submit(r); | ||
| }); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the TickerTask initialization, when creating the asyncTickerService, the code uses 'initSize - 1' and 'maxSize - 1' for the thread pool sizes. This means if a user configures initSize=2 and maxSize=8, they'll actually get a pool with corePoolSize=1 and maximumPoolSize=7. This doesn't match the configuration documentation which states these values represent the actual thread counts. Either remove the '- 1' or update the documentation to clarify this behavior.
|
|
||
| /** | ||
| * 声明当前 {@link BlockTicker} 是否线程安全 | ||
| * </br> |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The javadoc comment contains HTML tags (</br>) which should be proper JavaDoc tags. The line break should be written as '<br>' (without the closing tag) in JavaDoc, or use '<p>' tags for paragraph separation. The current syntax may not render correctly in generated documentation.
| Runnable func = () -> { | ||
| try { | ||
| if (Slimefun.isFolia()) { | ||
| Slimefun.getPlatformScheduler() | ||
| .runAtLocation(l, task -> tickBlock(l, item, data, timestamp)); | ||
| } else { | ||
| tickBlock(l, item, data, timestamp); | ||
| } | ||
| } catch (Exception x) { | ||
| reportErrors(l, item, x); | ||
| } | ||
| }; | ||
|
|
||
| if (item.getBlockTicker().isConcurrent()) { | ||
| asyncTickerService.execute(func); | ||
| } else { | ||
| fallbackTickerService.execute(func); | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a nested Folia check that might cause double-scheduling. When 'isConcurrent()' is true, the task is submitted to 'asyncTickerService', which executes the 'func' runnable. Inside 'func', there's another check for 'Slimefun.isFolia()' at line 272 that schedules another task with 'runAtLocation'. This means on Folia, the task will be scheduled twice - once by the executor service and once by runAtLocation. This could cause the same tick to execute multiple times or in the wrong order. Consider restructuring the logic to avoid double-scheduling.
| private void scheduleGeneral(@Nonnull TaskNode node, @Nonnull Runnable runnable) { | ||
| if (node.isAsynchronous()) { | ||
| if (node.getDelay() > 0) { | ||
| Slimefun.getPlatformScheduler().runLater(runnable, node.getDelay()); | ||
| } else { | ||
| Slimefun.getPlatformScheduler().runNextTick((task) -> runnable.run()); | ||
| } | ||
| } else { | ||
| if (node.getDelay() > 0) { | ||
| Slimefun.getPlatformScheduler().runLaterAsync(runnable, node.getDelay()); | ||
| } else { | ||
| Slimefun.getPlatformScheduler().runAsync((task) -> runnable.run()); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditions in the scheduleGeneral method appear to be inverted. When 'isAsynchronous()' returns true, the code schedules the task asynchronously (lines 92-95), but when it returns false, it schedules synchronously (lines 84-89). This is backwards - an asynchronous task should be scheduled with async methods, and a synchronous task should be scheduled with sync methods.
| public <T> T runSyncMethod(@Nonnull Callable<T> callable) { | ||
| if (Slimefun.isFolia()) { | ||
| throw new IllegalArgumentException("Location must be provided when executing sync task on Folia!"); | ||
| } | ||
|
|
||
| return runSyncMethod(callable, null, null); | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| public <T> T runSyncMethod(@Nonnull Callable<T> callable, @Nonnull Location l) { | ||
| return runSyncMethod(callable, l, null); | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| public <T> T runSyncMethod(@Nonnull Callable<T> callable, @Nonnull Entity entity) { | ||
| if (Slimefun.isFolia()) { | ||
| throw new IllegalArgumentException("Entity must be provided when executing sync task on Folia!"); | ||
| } | ||
|
|
||
| return runSyncMethod(callable, null, entity); | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| public <T> T runSyncMethod(@Nonnull Callable<T> callable, @Nullable Location l, @Nullable Entity entity) { | ||
| if (Bukkit.isPrimaryThread()) { | ||
| return callable.call(); | ||
| } else { | ||
| try { | ||
| return Bukkit.getScheduler() | ||
| .callSyncMethod(Slimefun.instance(), callable) | ||
| .get(1, TimeUnit.SECONDS); | ||
| if (Slimefun.isFolia()) { | ||
| final CompletableFuture<T> result = new CompletableFuture<>(); | ||
|
|
||
| System.out.println("Sync task created, hashcode = " + result.hashCode()); | ||
|
|
||
| if (l != null) { | ||
| Slimefun.getPlatformScheduler().runAtLocation(l, task -> { | ||
| try { | ||
| result.complete(callable.call()); | ||
| } catch (Exception e) { | ||
| result.completeExceptionally(e); | ||
| } | ||
| }); | ||
| } else { | ||
| if (entity != null) { | ||
| Slimefun.getPlatformScheduler().runAtEntity(entity, task -> { | ||
| try { | ||
| result.complete(callable.call()); | ||
| } catch (Exception e) { | ||
| result.completeExceptionally(e); | ||
| } | ||
| }); | ||
| } else { | ||
| throw new IllegalArgumentException( | ||
| "Location or entity must be provided when executing sync task on Folia!"); | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception handling logic in TaskUtil.runSyncMethod appears problematic. When Folia is enabled and the location/entity parameter is null, the code throws IllegalArgumentException with message about requiring location or entity. However, at line 31-32, the method already throws an exception if Folia is enabled without a location. This creates inconsistent error handling - the signature doesn't enforce non-null parameters for Folia, but the implementation requires them. Consider adding proper parameter validation annotations or creating separate method overloads for Folia vs non-Folia environments.
| synchronized (tickingLocations) { | ||
| loc = new HashSet<>(tickingLocations.entrySet()); | ||
| } | ||
| loc = new HashSet<>(tickingLocations.entrySet()); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the synchronized block around 'new HashSet<>(tickingLocations.entrySet())' at line 154 could cause issues. While ConcurrentHashMap's entrySet() is thread-safe for iteration, creating a new HashSet from it during concurrent modifications might miss or duplicate entries if the map is being modified during the copy operation. The original synchronized block ensured a consistent snapshot. Consider whether this change could lead to missed or duplicate ticks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
sub region slimefun ticker(由于 Folia 没有显式获取所有 Region 的方法,故取消)*: BlockTicker#isConcurrentSafe() 用于标记当前机器 Ticker 是并发安全的,Slimefun 默认认为附属的机器是并发不安全的,并移至异步单线程处理这些机器。