升級 Minecraft 26.1.2 + 更新 GitHub Actions SHA pin#476
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughUpgrades Java/Gradle/Fabric toolchains, migrates multiple Minecraft/Fabric APIs (click/tick/chunk/registry), retargets several mixin hook points, adds a ServerClockManager real-time tick mixin, updates CI action pins and submodule pointers, and adjusts messaging/networking and access-widener rules. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Server
participant ClockMgr as ServerClockManager
participant ClockInst as ClockInstance
Note over Server,ClockMgr: on each server tick
Server->>ClockMgr: tick()
alt GameRule ADVANCE_TIME enabled
ClockMgr->>Server: getRealTimeTicks()
Server-->>ClockMgr: realTimeTicks (n)
loop diff = n - 1 times
ClockMgr->>ClockInst: tick() / advanceTick()
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the project to Minecraft 26.1.2, Java 25, and Gradle 9.4.1, involving extensive API migrations across Mixins, networking, and GUI systems. Key changes include the transition from ClickType to ContainerInput, updates to ChunkPos utility methods, and refactoring ModelCustomBlock to use lazy initialization for item stacks. Review feedback highlighted a bug in CustomItem where disabling caching results in the loss of custom item data, and a regression in ModelCustomBlockEntity where removing entity tag filters could cause the accidental deletion of unrelated decorative entities.
| return this | ||
| } | ||
|
|
||
| open fun createItemStack(): ItemStack = if (cacheable) cacheItemStack.copy() else ItemStack(baseItem) |
There was a problem hiding this comment.
目前的 createItemStack 實作在 cacheable 為 false 時會回傳原始的 ItemStack(baseItem)。這會導致遺失所有自定義屬性（如名稱、模型、Galaxy 數據等），因為這些初始化邏輯目前僅存在於 cacheItemStack 的 lazy 初始化區塊中。建議將初始化邏輯提取到一個獨立的方法中，以便在 cacheable 為 false 時也能正確建立包含自定義數據的 ItemStack。
| // Kill leak entities | ||
| (level as ServerLevel).getEntities(EntityType.ITEM_DISPLAY) { it.blockPosition() == worldPosition && it.tags.contains("BLOCK") }.forEach { | ||
| // Kill leak entities (remove tag check - tags is now Set<TagKey<?>> not Set<String> in 26.1) | ||
| (level as ServerLevel).getEntities(EntityType.ITEM_DISPLAY) { it.blockPosition() == worldPosition }.forEach { |
There was a problem hiding this comment.
移除標籤檢查（tag check）是有風險的。這會導致此方塊實體在 tick 時刪除該位置的所有 ITEM_DISPLAY 實體，包括玩家或其他模組放置的裝飾性實體。雖然 Minecraft API 有所變動，但實體標籤（透過 addTag 添加的字串標籤）仍然存在且可用於過濾。建議保留 "BLOCK" 標籤的檢查，以確保僅刪除由本模組產生的實體。
| (level as ServerLevel).getEntities(EntityType.ITEM_DISPLAY) { it.blockPosition() == worldPosition }.forEach { | |
| (level as ServerLevel).getEntities(EntityType.ITEM_DISPLAY) { it.blockPosition() == worldPosition && it.tags.contains("BLOCK") }.forEach { |
…romTag
- Remove unnecessary `?.` safe calls after `childOrEmpty("galaxy_data")` in `loadAdditional` and `readCopyableData`
- Add `@Deprecated` annotation to `removeComponentsFromTag` method
- Remove deprecated setDayTime/shadow from ServerLevelMixin_RealTime - Add ServerClockManagerMixin_RealTime for real-time tick handling - Register new mixin in sponge.realtime.mixin.json - Expose ServerClockManager$ClockInstance in access widener Real-time feature now uses ServerClockManager.tick() instead of ServerLevel.tickTime(), adapting to Minecraft 26.x API changes where getDayTime()/setDayTime() were removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/kotlin/one/oktw/galaxy/block/entity/ModelCustomBlockEntity.kt (1)
83-89:⚠️ Potential issue | 🟡 MinorAdd missing
super.removeComponentsFromTag(view)call.All other overrides of
removeComponentsFromTagin this codebase (HarvestBlockEntityandTestGuiBlockEntity) callsuper.removeComponentsFromTag(view)before performing their custom logic, even though they are marked@Deprecated("Deprecated in Java"). This override breaks that convention by skipping the super call, which may result in the parent class's cleanup logic not running. Addsuper.removeComponentsFromTag(view)as the first line of this method.Additionally, the hard cast on line 85 (
view as TagValueOutput) assumes the framework always passesTagValueOutputinstances; if called with otherValueOutputtypes, this will throwClassCastException. Consider verifying the method contract with Mojang'sBlockEntity.removeComponentsFromTagdocumentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/block/entity/ModelCustomBlockEntity.kt` around lines 83 - 89, Add a call to super.removeComponentsFromTag(view) as the first statement in the override of removeComponentsFromTag so the parent cleanup runs, and avoid an unsafe cast by checking that view is a TagValueOutput (e.g., if (view !is TagValueOutput) return) before calling buildResult(); then cast safely or use the smart-cast to access buildResult(), read "galaxy_data", remove "model_entity" and "facing", and call view.discard("galaxy_data") if the CompoundTag is empty to preserve the current behavior.src/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.java (1)
40-50:⚠️ Potential issue | 🔴 CriticalCritical behavioral regression:
intoWaternow increments every tick regardless of water contact — arrows will be discarded ~10 ticks after spawn.The injection point changed from
updateInWaterStateAndDoWaterCurrentPushingat the INVOKE ofdoWaterSplashEffect()(which fires only during water interaction) toupdateFluidInteractionat RETURN (which is invoked every entity tick). BecauseintoWater++executes unconditionally on every callback, arrows are now discarded ~10 ticks after spawn regardless of fluid contact.The fix is to restore the water-contact guard using
isInWater()or theCallbackInfoReturnable<Boolean>return value before incrementing the counter.🛡️ Suggested guard so the counter only advances on actual water contact
`@Inject`(method = "updateFluidInteraction", at = `@At`(value = "RETURN")) private void countIntoWater(CallbackInfoReturnable<Boolean> ci) { + // Only count ticks the entity is actually in water; otherwise arrows would be + // discarded ~10 ticks after spawn regardless of fluid contact. + Entity self = (Entity) (Object) this; + if (!self.isInWater()) return; //noinspection ConstantConditions if (((Object) this) instanceof Arrow || ((Object) this) instanceof SpectralArrow) { if (intoWater > 10) { discard(); } } intoWater++; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.java` around lines 40 - 50, The injected method countIntoWater on updateFluidInteraction increments intoWater every tick causing arrows to be discarded even when not in water; change the logic to only advance the counter when the entity is actually in water by checking isInWater() (or by using ci.getReturnValue() from CallbackInfoReturnable<Boolean> which indicates water interaction) and only then increment intoWater and call discard() when > 10 for Arrow/SpectralArrow.
🧹 Nitpick comments (9)
src/main/kotlin/one/oktw/galaxy/player/Harvest.kt (1)
35-35: Optional: drop the unused lambda parameter.
serverWorldis unused in the cleanup body. Replacing it with_(or omitting the explicit name to use the implicitit) would be more idiomatic Kotlin and consistent with the style used inAngelBlock.kt. Functionally LGTM.♻️ Proposed nit
- ServerTickEvents.END_LEVEL_TICK.register { serverWorld -> justHarvested.clear() } + ServerTickEvents.END_LEVEL_TICK.register { _ -> justHarvested.clear() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/player/Harvest.kt` at line 35, The lambda passed to ServerTickEvents.END_LEVEL_TICK.register currently names an unused parameter `serverWorld`; change the lambda to omit the explicit parameter or replace it with `_` so the body simply calls justHarvested.clear() (i.e., update the registration at ServerTickEvents.END_LEVEL_TICK.register to use an unused-parameter underscore or implicit it, matching the style in AngelBlock.kt and removing the unused symbol).src/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.java (1)
38-43: Resolve the TODO and simplify the redundant ChunkPos allocations.The TODO at line 39 is justified: this code uses
chunkPos.pack()(instance method) while other mixins consistently useChunkPos.pack(x, z)(static method). The web search confirms thatpack()produces the canonical long encoding (chunkX | (chunkZ << 32)) consumed byisPositionTicking(), so the instance method is semantically correct. However, the inconsistency with the codebase pattern warrants resolving the TODO.The code also allocates
ChunkPosredundantly: once on line 38 forpack(), then again on line 43 for the fallback. Simplify by using the static form and eliminating both allocations:Suggested refactor
- ChunkPos chunkPos = new ChunkPos(x, z); - if (serverWorld.getChunkSource().isPositionTicking(chunkPos.pack())) { // TODO check this + if (serverWorld.getChunkSource().isPositionTicking(ChunkPos.pack(x, z))) { LevelChunk chunk = world.getChunkSource().getChunkNow(x, z); if (chunk != null) return chunk; } return new EmptyLevelChunk(world, new ChunkPos(x, z), world.registryAccess().lookupOrThrow(Registries.BIOME).getOrThrow(Biomes.PLAINS));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.java` around lines 38 - 43, Replace the instance ChunkPos allocation and pack() call with the static helper and avoid the redundant allocation: call ChunkPos.pack(x, z) when calling serverWorld.getChunkSource().isPositionTicking(...) and only construct a new ChunkPos(x, z) when creating the EmptyLevelChunk fallback; keep the existing LevelChunk retrieval via world.getChunkSource().getChunkNow(x, z) and preserve the registryAccess().lookupOrThrow(Registries.BIOME).getOrThrow(Biomes.PLAINS) usage.src/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.kt (1)
31-31: LGTM!
ContainerInput.PICKUPandContainerInput.CLONEchecks align with the newGUIClickEvent.actiontype. Behavior is preserved.Optional nit: since
ContainerInput.PICKUPis referenced in five places in this file, you could import it directly (e.g.,import net.minecraft.world.inventory.ContainerInput.PICKUP) to reduce verbosity, mirroring the wildcard pattern used inGUI.kt.Also applies to: 100-128, 163-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.kt` at line 31, Add a direct static import for ContainerInput.PICKUP to reduce verbosity where ContainerInput.PICKUP is referenced in HTCraftingTableBlockEntity; update the imports in HTCraftingTableBlockEntity.kt to include import net.minecraft.world.inventory.ContainerInput.PICKUP (you can leave ContainerInput.CLONE as-is or similarly import CLONE if desired) so the five occurrences use PICKUP directly, matching the import style used in GUI.kt and keeping behavior unchanged.src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.java (1)
36-40: Samelambda$register$1fragility asMixinPlayerChat_SayCommand.This
@Redirectrelies on the synthetic lambda index inEmoteCommands.register. The retarget is correct for 26.1.2, but identical caveats apply: future Mojang reorders/inserts insideregisterwill break this silently. Consider addingrequire = 1so a failed bind aborts mixin apply rather than silently no-oping the chat-event guard. Same recommendation as filed onMixinPlayerChat_SayCommand.java.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.java` around lines 36 - 40, The `@Redirect` in MixinPlayerChat_EmoteCommands that targets the synthetic method lambda$register$1 is fragile and may silently fail if Mojang reorders lambdas; update the `@Redirect` annotation (the one with target PlayerList.broadcastChatMessage and method = "lambda$register$1") to include require = 1 so mixin application will abort on a failed bind instead of silently no-oping the chat-event guard, matching the same fix applied to MixinPlayerChat_SayCommand and keeping EmoteCommands.register guarded reliably.src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.java (1)
37-45: Addrequire = 1to the@Injectannotation to fail-fast on lambda resolution failure.The synthetic
lambda$register$1target is indeed fragile across Minecraft updates. Since synthetic lambda names are derived from declaration order withinSayCommand.register, any future Mojang refactor that adds, removes, or reorders a lambda in that method will silently shift the index and cause the injection to fail at runtime when the command is invoked.Adding
require = 1to the@Injectannotation will make the mixin fail immediately at load time if the target cannot be found, rather than silently skipping the injection. This pattern is already used elsewhere in the codebase (e.g.,MixinSkipSyncRegistry.java).Additionally, verify that
lambda$register$1still targets the broadcastChatMessage invocation in the current 26.1.2 source.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.java` around lines 37 - 45, Update the `@Inject` on MixinPlayerChat_SayCommand targeting the synthetic method lambda$register$1 to include require = 1 (i.e., `@Inject`(..., require = 1, cancellable = true)) so the mixin fails fast if the lambda/broadcastChatMessage target cannot be resolved; keep the same target invocation (PlayerList.broadcastChatMessage) and after the change verify that lambda$register$1 still maps to the broadcastChatMessage invocation in the current 26.1.2 sources to ensure the injection point is correct.src/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.java (3)
46-48: Asymmetric null-handling onthis.server.
realTimeBridge$getRealTimeTicks()(Lines 61–66) defensively checksthis.server != null, but the@Injectat Line 48 dereferencesthis.server.getGlobalGameRules()without that guard. In vanilla 26.1,ServerClockManager.serveris set in the constructor and is effectively non-null duringtick(), so this is unlikely to NPE in practice — however the asymmetry is confusing for readers, and other mods could conceivably construct aServerClockManageroutside the normal lifecycle. Either tighten the bridge method (drop the null branch) or guard the inject for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.java` around lines 46 - 48, The inject method realTimeImpl$fixTimeOfDayForRealTime dereferences this.server without the null guard used in realTimeBridge$getRealTimeTicks, so add a defensive null check at the start of realTimeImpl$fixTimeOfDayForRealTime (i.e., return early if this.server == null) before calling this.server.getGlobalGameRules().get(GameRules.ADVANCE_TIME), mirroring the null-handling in realTimeBridge$getRealTimeTicks to keep behavior consistent and avoid potential NPEs from non-standard construction.
52-56: Invert the nested loop to advance each clock once withdiffiterations.The current shape iterates
clocks.values()difftimes, allocating a new iterator each pass. Iterating clocks once and advancing each bydiffgives identical semantics with a single iterator allocation and better cache locality — relevant during long pauses wherediffcan be large (skipped-tick recovery / debugger stalls).♻️ Proposed refactor
- if (diff > 0) { - for (int i = 0; i < diff; i++) { - this.clocks.values().forEach(ServerClockManager.ClockInstance::tick); - } - } + if (diff > 0) { + for (final ServerClockManager.ClockInstance clock : this.clocks.values()) { + for (long i = 0; i < diff; i++) { + clock.tick(); + } + } + }Note: also widening the
icounter tolongsincediffis along(tick deltas in nanos/50_000_000 can exceedInteger.MAX_VALUEonly after ~3.4 years of stall, but it removes the implicit narrowing risk and silences IDE warnings).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.java` around lines 52 - 56, The nested loops allocate a new iterator for clocks on each of the diff iterations; invert them so you iterate clocks once and advance each clock diff times to reduce iterator allocations and improve locality. Replace the outer for (int i = 0; i < diff; i++) this.clocks.values().forEach(ServerClockManager.ClockInstance::tick) with a single iteration over this.clocks.values() (e.g. for (ServerClockManager.ClockInstance ci : this.clocks.values()) { for (long i = 0; i < diff; i++) ci.tick(); }) and widen the loop counter to long to match diff.
45-45: Add method descriptor to@Injectfor robustness and clarity.The
method = "tick"parameter should include the explicit method descriptor()Vto prevent ambiguity ifServerClockManagergains an overloadedtick(...)method in the future. This is a Mixin best practice that makes the injection target explicit and fault-tolerant:♻️ Proposed change
- `@Inject`(method = "tick", at=@At("HEAD")) + `@Inject`(method = "tick()V", at = `@At`("HEAD"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.java` at line 45, The `@Inject` on ServerClockManagerMixin_RealTime currently targets method = "tick" without a descriptor; update the injection to use the explicit JVM descriptor by changing the annotation to target method = "tick()V" so the mixin binds unambiguously to the no-arg void tick method and remains correct if tick is later overloaded; locate the `@Inject` on ServerClockManagerMixin_RealTime and replace the method string accordingly.src/main/resources/fabric.mod.json (1)
32-32: Consider narrowing theminecraftrange to26.1.x.
"26.x"will satisfy any future 26.* (e.g., 26.2.0). Since this PR contains API migrations that are specific to 26.1 internals (ServerClockManager,ContainerInput, etc.), allowing 26.2+ at the manifest level risks Fabric Loader accepting the mod on a Minecraft version where these mixins/refs no longer resolve, producing harder-to-diagnose runtime/mixin errors instead of a clean dependency rejection.♻️ Proposed tightened range
- "minecraft": "26.x", + "minecraft": "~26.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/fabric.mod.json` at line 32, The manifest currently allows any 26.* Minecraft with "minecraft": "26.x", which could let the mod load on 26.2+ where internal APIs like ServerClockManager and ContainerInput may differ; update the fabric.mod.json minecraft field to a tightened range that only permits 26.1 series (e.g., "26.1.x") so Fabric Loader rejects incompatible 26.2+ versions and avoids mixin/runtime resolution errors referencing symbols such as ServerClockManager and ContainerInput.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.java`:
- Around line 40-50: The injected method countIntoWater on
updateFluidInteraction increments intoWater every tick causing arrows to be
discarded even when not in water; change the logic to only advance the counter
when the entity is actually in water by checking isInWater() (or by using
ci.getReturnValue() from CallbackInfoReturnable<Boolean> which indicates water
interaction) and only then increment intoWater and call discard() when > 10 for
Arrow/SpectralArrow.
In `@src/main/kotlin/one/oktw/galaxy/block/entity/ModelCustomBlockEntity.kt`:
- Around line 83-89: Add a call to super.removeComponentsFromTag(view) as the
first statement in the override of removeComponentsFromTag so the parent cleanup
runs, and avoid an unsafe cast by checking that view is a TagValueOutput (e.g.,
if (view !is TagValueOutput) return) before calling buildResult(); then cast
safely or use the smart-cast to access buildResult(), read "galaxy_data", remove
"model_entity" and "facing", and call view.discard("galaxy_data") if the
CompoundTag is empty to preserve the current behavior.
---
Nitpick comments:
In
`@src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.java`:
- Around line 36-40: The `@Redirect` in MixinPlayerChat_EmoteCommands that targets
the synthetic method lambda$register$1 is fragile and may silently fail if
Mojang reorders lambdas; update the `@Redirect` annotation (the one with target
PlayerList.broadcastChatMessage and method = "lambda$register$1") to include
require = 1 so mixin application will abort on a failed bind instead of silently
no-oping the chat-event guard, matching the same fix applied to
MixinPlayerChat_SayCommand and keeping EmoteCommands.register guarded reliably.
In `@src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.java`:
- Around line 37-45: Update the `@Inject` on MixinPlayerChat_SayCommand targeting
the synthetic method lambda$register$1 to include require = 1 (i.e.,
`@Inject`(..., require = 1, cancellable = true)) so the mixin fails fast if the
lambda/broadcastChatMessage target cannot be resolved; keep the same target
invocation (PlayerList.broadcastChatMessage) and after the change verify that
lambda$register$1 still maps to the broadcastChatMessage invocation in the
current 26.1.2 sources to ensure the injection point is correct.
In
`@src/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.java`:
- Around line 38-43: Replace the instance ChunkPos allocation and pack() call
with the static helper and avoid the redundant allocation: call ChunkPos.pack(x,
z) when calling serverWorld.getChunkSource().isPositionTicking(...) and only
construct a new ChunkPos(x, z) when creating the EmptyLevelChunk fallback; keep
the existing LevelChunk retrieval via world.getChunkSource().getChunkNow(x, z)
and preserve the
registryAccess().lookupOrThrow(Registries.BIOME).getOrThrow(Biomes.PLAINS)
usage.
In
`@src/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.java`:
- Around line 46-48: The inject method realTimeImpl$fixTimeOfDayForRealTime
dereferences this.server without the null guard used in
realTimeBridge$getRealTimeTicks, so add a defensive null check at the start of
realTimeImpl$fixTimeOfDayForRealTime (i.e., return early if this.server == null)
before calling this.server.getGlobalGameRules().get(GameRules.ADVANCE_TIME),
mirroring the null-handling in realTimeBridge$getRealTimeTicks to keep behavior
consistent and avoid potential NPEs from non-standard construction.
- Around line 52-56: The nested loops allocate a new iterator for clocks on each
of the diff iterations; invert them so you iterate clocks once and advance each
clock diff times to reduce iterator allocations and improve locality. Replace
the outer for (int i = 0; i < diff; i++)
this.clocks.values().forEach(ServerClockManager.ClockInstance::tick) with a
single iteration over this.clocks.values() (e.g. for
(ServerClockManager.ClockInstance ci : this.clocks.values()) { for (long i = 0;
i < diff; i++) ci.tick(); }) and widen the loop counter to long to match diff.
- Line 45: The `@Inject` on ServerClockManagerMixin_RealTime currently targets
method = "tick" without a descriptor; update the injection to use the explicit
JVM descriptor by changing the annotation to target method = "tick()V" so the
mixin binds unambiguously to the no-arg void tick method and remains correct if
tick is later overloaded; locate the `@Inject` on ServerClockManagerMixin_RealTime
and replace the method string accordingly.
In `@src/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.kt`:
- Line 31: Add a direct static import for ContainerInput.PICKUP to reduce
verbosity where ContainerInput.PICKUP is referenced in
HTCraftingTableBlockEntity; update the imports in HTCraftingTableBlockEntity.kt
to include import net.minecraft.world.inventory.ContainerInput.PICKUP (you can
leave ContainerInput.CLONE as-is or similarly import CLONE if desired) so the
five occurrences use PICKUP directly, matching the import style used in GUI.kt
and keeping behavior unchanged.
In `@src/main/kotlin/one/oktw/galaxy/player/Harvest.kt`:
- Line 35: The lambda passed to ServerTickEvents.END_LEVEL_TICK.register
currently names an unused parameter `serverWorld`; change the lambda to omit the
explicit parameter or replace it with `_` so the body simply calls
justHarvested.clear() (i.e., update the registration at
ServerTickEvents.END_LEVEL_TICK.register to use an unused-parameter underscore
or implicit it, matching the style in AngelBlock.kt and removing the unused
symbol).
In `@src/main/resources/fabric.mod.json`:
- Line 32: The manifest currently allows any 26.* Minecraft with "minecraft":
"26.x", which could let the mod load on 26.2+ where internal APIs like
ServerClockManager and ContainerInput may differ; update the fabric.mod.json
minecraft field to a tightened range that only permits 26.1 series (e.g.,
"26.1.x") so Fabric Loader rejects incompatible 26.2+ versions and avoids
mixin/runtime resolution errors referencing symbols such as ServerClockManager
and ContainerInput.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4a56a01-cf9d-4a75-94fd-4e5df3dfdb47
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (33)
.circleci/config.yml.github/workflows/workflow.ymlbuild.gradle.ktsdockergradle/wrapper/gradle-wrapper.propertiesgradlewsrc/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.javasrc/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinAsyncChunk_IOWorker.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinAsyncChunk_ServerGamePacketListener.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.javasrc/main/java/org/spongepowered/common/mixin/realtime/world/ServerClockManagerMixin_RealTime.javasrc/main/java/org/spongepowered/common/mixin/realtime/world/ServerLevelMixin_RealTime.javasrc/main/kotlin/one/oktw/galaxy/Main.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/CustomBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/ModelCustomBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/TestGuiBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/event/AngelBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/event/BlockEvents.ktsrc/main/kotlin/one/oktw/galaxy/chat/Exchange.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Home.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Join.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Spawn.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/admin/Creative.ktsrc/main/kotlin/one/oktw/galaxy/gui/GUI.ktsrc/main/kotlin/one/oktw/galaxy/gui/GUIClickEvent.ktsrc/main/kotlin/one/oktw/galaxy/item/recipe/CustomItemRecipe.ktsrc/main/kotlin/one/oktw/galaxy/player/Harvest.ktsrc/main/resources/fabric.mod.jsonsrc/main/resources/galaxy.accesswidenersrc/main/resources/sponge.realtime.mixin.json
💤 Files with no reviewable changes (1)
- src/main/java/org/spongepowered/common/mixin/realtime/world/ServerLevelMixin_RealTime.java
- Remove unused `serverLevelData` shadow field from `ServerLevelMixin_RealTime` - Convert Java `Duration`/`TimeUnit` delays to Kotlin `Duration.Companion.milliseconds` in `Join`, `Home`, and `Spawn` commands Aligns with Kotlin best practices for time-based delays.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/main/kotlin/one/oktw/galaxy/player/Harvest.kt (1)
35-35: Optional: drop the unusedserverWorldlambda parameter for consistency.
AngelBlock.ktuses the parameter-less form for the same cleanup. Aligning the style here is a minor readability nit.♻️ Proposed tweak
- ServerTickEvents.END_LEVEL_TICK.register { serverWorld -> justHarvested.clear() } + ServerTickEvents.END_LEVEL_TICK.register { justHarvested.clear() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/player/Harvest.kt` at line 35, Remove the unused lambda parameter by changing the registration of ServerTickEvents.END_LEVEL_TICK to use a parameter-less lambda; specifically update the call to ServerTickEvents.END_LEVEL_TICK.register so it invokes justHarvested.clear() without the unused serverWorld parameter to match the style used in AngelBlock.kt.src/main/kotlin/one/oktw/galaxy/command/commands/Spawn.kt (1)
64-64: Optional: simplify to1.secondsfor idiomatic Kotlin Duration usage.Since this commit migrates to the Kotlin
DurationAPI,TimeUnit.SECONDS.toMillis(1).millisecondscould be expressed more directly as1.seconds, dropping theTimeUnitindirection (and thejava.util.concurrent.TimeUnitimport if no other usage remains). Note: applying this here would diverge from the same pattern inHome.kt/Join.kt, so consider doing it consistently across all three commands or leaving as-is for parity.♻️ Proposed change
-import java.util.concurrent.TimeUnit -import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds @@ - delay(TimeUnit.SECONDS.toMillis(1).milliseconds) + delay(1.seconds)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/command/commands/Spawn.kt` at line 64, Replace the verbose Duration expression in Spawn.kt — the delay(TimeUnit.SECONDS.toMillis(1).milliseconds) call — with the idiomatic Kotlin Duration literal 1.seconds; update the import usage (remove java.util.concurrent.TimeUnit if it's no longer used) and consider applying the same change consistently to Home.kt and Join.kt to keep parity across the command implementations (look for delay(...) calls in the Spawn command handler or method where this line appears).src/main/kotlin/one/oktw/galaxy/item/CustomBlockItem.kt (1)
29-37: Optional: avoid repeatedas CustomBlockItemcasts.Since
CustomItem.registryis typed asCustomRegistry<CustomItem>,register()returns the supertype and forces a cast on every entry. You can sidestep this withalso, which preserves the subtype:♻️ Proposed refactor
- val HT_CRAFTING_TABLE = registry.register(CustomBlockItem("ht_crafting_table", "block.HT_CRAFTING_TABLE")) as CustomBlockItem - val ELEVATOR = registry.register(CustomBlockItem("elevator", "block.ELEVATOR")) as CustomBlockItem - val ANGEL_BLOCK = registry.register(CustomBlockItem("angel_block", "block.ANGEL_BLOCK")) as CustomBlockItem - val TRASHCAN = registry.register(CustomBlockItem("trashcan", "block.TRASHCAN")) as CustomBlockItem - val TELEPORTER_CORE_BASIC = registry.register(CustomBlockItem("teleporter_core_basic", "block.TELEPORTER")) as CustomBlockItem - val TELEPORTER_CORE_ADVANCE = registry.register(CustomBlockItem("teleporter_core_advance", "block.TELEPORTER_ADVANCED")) as CustomBlockItem - val TELEPORTER_FRAME = registry.register(CustomBlockItem("teleporter_frame", "block.TELEPORTER_FRAME")) as CustomBlockItem - val TEST_GUI = registry.register(CustomBlockItem("test_gui", "block.TEST_GUI")) as CustomBlockItem - val HARVEST = registry.register(CustomBlockItem("harvest", "block.HARVEST")) as CustomBlockItem + val HT_CRAFTING_TABLE = CustomBlockItem("ht_crafting_table", "block.HT_CRAFTING_TABLE").also { registry.register(it) } + val ELEVATOR = CustomBlockItem("elevator", "block.ELEVATOR").also { registry.register(it) } + val ANGEL_BLOCK = CustomBlockItem("angel_block", "block.ANGEL_BLOCK").also { registry.register(it) } + val TRASHCAN = CustomBlockItem("trashcan", "block.TRASHCAN").also { registry.register(it) } + val TELEPORTER_CORE_BASIC = CustomBlockItem("teleporter_core_basic", "block.TELEPORTER").also { registry.register(it) } + val TELEPORTER_CORE_ADVANCE = CustomBlockItem("teleporter_core_advance", "block.TELEPORTER_ADVANCED").also { registry.register(it) } + val TELEPORTER_FRAME = CustomBlockItem("teleporter_frame", "block.TELEPORTER_FRAME").also { registry.register(it) } + val TEST_GUI = CustomBlockItem("test_gui", "block.TEST_GUI").also { registry.register(it) } + val HARVEST = CustomBlockItem("harvest", "block.HARVEST").also { registry.register(it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/one/oktw/galaxy/item/CustomBlockItem.kt` around lines 29 - 37, The registrations repeatedly cast registry.register(...) to CustomBlockItem; instead instantiate the CustomBlockItem first and call registry.register on that instance using also to preserve the subtype (e.g. val HT_CRAFTING_TABLE = CustomBlockItem("ht_crafting_table","block.HT_CRAFTING_TABLE").also { registry.register(it) }), doing the same for ELEVATOR, ANGEL_BLOCK, TRASHCAN, TELEPORTER_CORE_BASIC, TELEPORTER_CORE_ADVANCE, TELEPORTER_FRAME, TEST_GUI, and HARVEST so you avoid the repeated as CustomBlockItem casts while keeping the same registry.register call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/kotlin/one/oktw/galaxy/command/commands/Spawn.kt`:
- Line 64: Replace the verbose Duration expression in Spawn.kt — the
delay(TimeUnit.SECONDS.toMillis(1).milliseconds) call — with the idiomatic
Kotlin Duration literal 1.seconds; update the import usage (remove
java.util.concurrent.TimeUnit if it's no longer used) and consider applying the
same change consistently to Home.kt and Join.kt to keep parity across the
command implementations (look for delay(...) calls in the Spawn command handler
or method where this line appears).
In `@src/main/kotlin/one/oktw/galaxy/item/CustomBlockItem.kt`:
- Around line 29-37: The registrations repeatedly cast registry.register(...) to
CustomBlockItem; instead instantiate the CustomBlockItem first and call
registry.register on that instance using also to preserve the subtype (e.g. val
HT_CRAFTING_TABLE =
CustomBlockItem("ht_crafting_table","block.HT_CRAFTING_TABLE").also {
registry.register(it) }), doing the same for ELEVATOR, ANGEL_BLOCK, TRASHCAN,
TELEPORTER_CORE_BASIC, TELEPORTER_CORE_ADVANCE, TELEPORTER_FRAME, TEST_GUI, and
HARVEST so you avoid the repeated as CustomBlockItem casts while keeping the
same registry.register call.
In `@src/main/kotlin/one/oktw/galaxy/player/Harvest.kt`:
- Line 35: Remove the unused lambda parameter by changing the registration of
ServerTickEvents.END_LEVEL_TICK to use a parameter-less lambda; specifically
update the call to ServerTickEvents.END_LEVEL_TICK.register so it invokes
justHarvested.clear() without the unused serverWorld parameter to match the
style used in AngelBlock.kt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb572651-8121-4192-991d-0a1c2178637c
📒 Files selected for processing (32)
src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.javasrc/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinAsyncChunk_IOWorker.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinAsyncChunk_ServerGamePacketListener.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.javasrc/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.javasrc/main/java/org/spongepowered/common/mixin/realtime/world/ServerLevelMixin_RealTime.javasrc/main/kotlin/one/oktw/galaxy/Main.ktsrc/main/kotlin/one/oktw/galaxy/block/CustomBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/HTCraftingTable.ktsrc/main/kotlin/one/oktw/galaxy/block/HarvestBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/ModelCustomBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/TestGuiBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/TrashcanBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/CustomBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/HarvestBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/ModelCustomBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/TestGuiBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/entity/TrashcanBlockEntity.ktsrc/main/kotlin/one/oktw/galaxy/block/event/AngelBlock.ktsrc/main/kotlin/one/oktw/galaxy/block/event/BlockEvents.ktsrc/main/kotlin/one/oktw/galaxy/chat/Exchange.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Home.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Join.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/Spawn.ktsrc/main/kotlin/one/oktw/galaxy/command/commands/admin/Creative.ktsrc/main/kotlin/one/oktw/galaxy/gui/GUI.ktsrc/main/kotlin/one/oktw/galaxy/gui/GUIClickEvent.ktsrc/main/kotlin/one/oktw/galaxy/item/CustomBlockItem.ktsrc/main/kotlin/one/oktw/galaxy/item/recipe/CustomItemRecipe.ktsrc/main/kotlin/one/oktw/galaxy/player/Harvest.kt
✅ Files skipped from review due to trivial changes (5)
- src/main/kotlin/one/oktw/galaxy/block/event/BlockEvents.kt
- src/main/kotlin/one/oktw/galaxy/block/entity/CustomBlockEntity.kt
- src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_SayCommand.java
- src/main/kotlin/one/oktw/galaxy/chat/Exchange.kt
- src/main/java/one/oktw/galaxy/mixin/tweak/MixinAsyncChunk_ServerGamePacketListener.java
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/one/oktw/galaxy/mixin/tweak/MixinMapExistingChunk_MapItem.java
- src/main/kotlin/one/oktw/galaxy/gui/GUIClickEvent.kt
- src/main/kotlin/one/oktw/galaxy/Main.kt
- src/main/java/one/oktw/galaxy/mixin/event/MixinPlayerChat_EmoteCommands.java
- src/main/java/one/oktw/galaxy/mixin/tweak/MixinThrownCountdown_Entity.java
- src/main/kotlin/one/oktw/galaxy/command/commands/Join.kt
- src/main/kotlin/one/oktw/galaxy/command/commands/admin/Creative.kt
- src/main/java/org/spongepowered/common/mixin/realtime/world/ServerLevelMixin_RealTime.java
- src/main/kotlin/one/oktw/galaxy/block/entity/HTCraftingTableBlockEntity.kt
- src/main/kotlin/one/oktw/galaxy/command/commands/Home.kt
1 participant
摘要
將 Galaxy 從 Minecraft 1.21.11 升級至 26.1.2，並更新 GitHub Actions CI 的版本。
建置變更
Minecraft 26.1.x 變更
loom.officialMojangMappings()）modImplementation改為implementationGitHub Actions CI（已更新為 SHA pin）
Docker 映像檔
其他更新
###破壞性變更