-
Notifications
You must be signed in to change notification settings - Fork 1
feat: improve detections #3
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: main
Are you sure you want to change the base?
Conversation
GalvinPython
commented
Jan 11, 2026
- chore: update gradle
- improve world detections with mixins
- open up to lan using world settings ([FR]: Change default behaviour when LAN opens #2)
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 refactors the AutoLan mod to improve world detection performance by replacing the client tick event listener with a more efficient mixin-based approach. It also updates the mod to Minecraft 1.21.11 and upgrades Gradle to version 9.2.1.
Changes:
- Refactored from ClientTickEvents polling to mixin-based world join detection for better CPU efficiency
- Updated to Minecraft 1.21.11 with corresponding Fabric API, Yarn mappings, and loader version updates
- Enhanced functionality to use the world's game mode setting instead of hardcoded Creative mode
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/fabric.mod.json | Updated entrypoint to new package structure, changed Minecraft version to 1.21.11, and registered mixin configuration |
| src/main/resources/autolan.mixins.json | Added new mixin configuration file for AutoLanMixin |
| src/main/java/me/imgalvin/autolan/mixin/AutoLanMixin.java | New mixin implementation that injects into onGameJoin for automatic LAN opening with world-specific game mode |
| src/main/java/me/imgalvin/autolan/Autolan.java | Simplified main class moved to new package with basic initialization |
| src/main/java/me/imgalvin/Autolan.java | Removed old implementation using ClientTickEvents |
| gradle/wrapper/gradle-wrapper.properties | Updated Gradle from 8.14.3 to 9.2.1 |
| gradle.properties | Updated Minecraft, Fabric API, Yarn mappings, loader versions, and mod version to 1.1.0 |
| build.gradle | Removed extraneous blank line |
| README.md | Added notes about cheats being enabled, updated supported versions, and added changelog for version 1.1.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Changelog | ||
| ## 1.1.0 | ||
| - Optimised the CPU usage of the mod |
Copilot
AI
Jan 11, 2026
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 spelling of "Optimised" should be "Optimized" (with a 'z') to maintain consistency with American English spelling conventions used throughout the codebase and standard in software development.
| - Optimised the CPU usage of the mod | |
| - Optimized the CPU usage of the mod |
| public class AutoLanMixin { | ||
|
|
||
| @Unique | ||
| private boolean autolan$opened = false; |
Copilot
AI
Jan 11, 2026
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 state variable 'autolan$opened' is stored per-instance of ClientPlayNetworkHandler, which means it could be reset to false when transitioning between worlds. This could lead to the LAN server being opened multiple times if the player switches between worlds in the same game session. Consider using a static field or a more global state management approach to ensure the flag persists across ClientPlayNetworkHandler instances, or reset the flag intentionally when switching worlds if the behavior should be per-world.
| private boolean autolan$opened = false; | |
| private static boolean autolan$opened = false; |
| server.openToLan(gameMode, true, port); | ||
| autolan$opened = true; | ||
| client.player.sendMessage( | ||
| Text.literal("Opened to LAN on port " + port + " in mode: " + gameMode + ". Note: LAN worlds automatically open with cheats enabled. This will be addressed in a future update.").formatted(net.minecraft.util.Formatting.GREEN), |
Copilot
AI
Jan 11, 2026
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 success message is excessively long and includes implementation notes that should be in the README or changelog, not shown to users every time they open a world. Consider shortening this message to something like "Opened to LAN on port [port] in [gameMode] mode" and move the note about cheats to documentation only.
| Text.literal("Opened to LAN on port " + port + " in mode: " + gameMode + ". Note: LAN worlds automatically open with cheats enabled. This will be addressed in a future update.").formatted(net.minecraft.util.Formatting.GREEN), | |
| Text.literal("Opened to LAN on port " + port + " in " + gameMode + " mode.").formatted(net.minecraft.util.Formatting.GREEN), |
| public class Autolan implements ClientModInitializer { | ||
|
|
||
| @Override | ||
| public void onInitializeClient() { | ||
| System.out.println("Autolan initialised!"); |
Copilot
AI
Jan 11, 2026
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 class name "Autolan" does not follow Java naming conventions. Class names should use PascalCase with all significant words capitalized. It should be "AutoLan" to properly represent "Auto LAN" (where LAN is an acronym), making the class name more readable and consistent with naming conventions.
| public class Autolan implements ClientModInitializer { | |
| @Override | |
| public void onInitializeClient() { | |
| System.out.println("Autolan initialised!"); | |
| public class AutoLan implements ClientModInitializer { | |
| @Override | |
| public void onInitializeClient() { | |
| System.out.println("AutoLan initialised!"); |
| } catch (Throwable ignored) { | ||
| } | ||
| } | ||
|
|
Copilot
AI
Jan 11, 2026
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.
If retrieving the game mode fails (line 37), the gameMode variable remains null and is passed to server.openToLan(null, true, port) on line 50. While this may work with a fallback to a default game mode in the Minecraft API, the behavior is unclear. Consider either: 1) returning early if gameMode retrieval fails, or 2) providing an explicit fallback game mode, or 3) adding a comment explaining that passing null is intentional and will use a default.
| // Fallback to a sensible default if game mode could not be retrieved | |
| if (gameMode == null) { | |
| gameMode = GameMode.SURVIVAL; | |
| } |
| # Changelog | ||
| ## 1.1.0 | ||
| - Optimised the CPU usage of the mod | ||
| - Open up to lan using world settings for difficulty |
Copilot
AI
Jan 11, 2026
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 changelog mentions "Open up to lan using world settings for difficulty" but the code actually uses the game mode, not difficulty. The game mode is retrieved from server.getSaveProperties().getGameMode() which returns the world's game mode (Creative, Survival, Adventure, Spectator), not its difficulty setting. The changelog should be updated to accurately reflect that game mode is being used, not difficulty.
| - Open up to lan using world settings for difficulty | |
| - Open up to LAN using world settings for game mode |
|
|
||
| @Override | ||
| public void onInitializeClient() { | ||
| System.out.println("Autolan initialised!"); |
Copilot
AI
Jan 11, 2026
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 spelling of "initialised" should be "initialized" (with a 'z') to maintain consistency with American English spelling conventions used throughout the codebase and standard in software development. For example, the method implemented is "onInitializeClient" (with a 'z'), not "onInitialiseClient".
| System.out.println("Autolan initialised!"); | |
| System.out.println("Autolan initialized!"); |