Feat: Use LazyInit to load LightEngine. #60
Conversation
…patibility on the PluginServer. - Allow server to accept any version of the client mod.
WalkthroughThe pull request introduces several changes, including an update to the project version in Changes
Sequence Diagram(s)sequenceDiagram
participant World as WorldMixin
participant Engine as LightingEngine
World->>Engine: Request LightingEngine
alt First Access
Engine-->>World: Initialize LightingEngine
end
World->>Engine: Use LightingEngine
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/main/java/dev/redstudio/alfheim/Alfheim.java (1)
17-17: Consider specifying a range of compatible versions.The change to add
acceptableRemoteVersions = "*"is correctly implemented and aligns with the PR objective of making the server not force the client to install the same version of the mod.However, setting
acceptableRemoteVersions = "*"is a broad compatibility rule that may lead to unexpected issues if the mod's functionality changes significantly between versions. It would be ideal to specify a range of compatible versions instead of allowing all versions.For example, you could specify a range like this:
-@Mod(modid = ID, version = VERSION, updateJSON = "https://forge.curseupdate.com/910715/" + ID, useMetadata = true, dependencies = "required-after:mixinbooter@[8.8,);required-after:redcore@[0.5,)", acceptableRemoteVersions = "*") +@Mod(modid = ID, version = VERSION, updateJSON = "https://forge.curseupdate.com/910715/" + ID, useMetadata = true, dependencies = "required-after:mixinbooter@[8.8,);required-after:redcore@[0.5,)", acceptableRemoteVersions = "[1.0,2.0)")This would allow compatibility with versions from 1.0 (inclusive) to 2.0 (exclusive). Adjust the range as per your mod's versioning scheme and compatibility expectations.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- build.gradle.kts (1 hunks)
- src/main/java/dev/redstudio/alfheim/Alfheim.java (1 hunks)
- src/main/java/dev/redstudio/alfheim/mixin/WorldMixin.java (2 hunks)
Additional comments not posted (3)
src/main/java/dev/redstudio/alfheim/mixin/WorldMixin.java (2)
72-75: LGTM!The lazy initialization of
alfheim$lightingEnginelooks good. It ensures that theLightingEngineis only created when it is first accessed, which aligns with the objective of deferring the initialization to improve compatibility and avoid immediate loading issues.
42-42: Verify that always returningtruedoes not introduce unintended side effects.The change looks good as it delegates the light update scheduling to the custom
LightingEngine. However, please ensure that always returningtruedoes not break any existing logic that relies on the return value ofcheckLightFor.To verify the impact of always returning
true, run the following script:Verification successful
Modification to always return
trueis safeAfter analyzing the usage of
checkLightForin the codebase, I can confirm that always returningtruedoes not introduce any unintended side effects. The method calls inChunkMixin.javado not use the return value, and the actual light update is scheduled through the customLightingEngine. This change aligns with the intended behavior of delegating light updates to the new system.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the return value of `checkLightFor`. # Test: Search for the method calls and analyze how the return value is used. # Expect: The return value should not be used in a way that introduces side effects. rg --type java -A 5 $'checkLightFor\('Length of output: 2940
build.gradle.kts (1)
14-14: Version update looks good!The version has been bumped from
1.4to1.5, which adheres to the Ragnarök versioning convention. This update likely corresponds to the changes introduced in this PR.
KasumiNova
changed the title
|
Could you send the full debug log of a crash? because it feels weird that the init inject would fail |
📝 Description
This PR fixes plugin server compatibility.
Changed WorldMixin to LazyInit at alfheim$getLightingEngine() instead of loading immediately at because the plugin server added a new method, but the mixin could not modify the newly added method properly (Like this: https://mclo.gs/Timk8ND).
Another feature, modified Alfheim.java, added a annotation content, so that the server can accept any version of the client's mod, because there is no relevant code involved in the dual-end interaction, so that the client does not need to force the installation of the same version of the mod with the server.
🚦 Testing
Works fine at SinglePlayer, Dedicated Server, Dedicated Plugin Server.
⏮️ Backwards Compatibility
Works fine.
📚 Related Issues & Documents
🖼️ Screenshots/Recordings
📖 Added to documentation?
😄 [optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
New Features
Bug Fixes