-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix incompatibility with RenderLib 1.3.3+ #59
Conversation
WalkthroughThis update brings a more flexible and configurable approach to customizing the Minecraft client, particularly in how the window title and depth buffer settings are managed. By shifting from a direct overwrite to utilizing Mixin annotations like Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/dev/redstudio/valkyrie/mixin/MinecraftMixin.java (1 hunks)
Additional Context Used
Additional comments not posted (3)
src/main/java/dev/redstudio/valkyrie/mixin/MinecraftMixin.java (3)
46-49
: The change to use@ModifyConstant
for modifying the window title based on configuration is a good approach for enhancing flexibility and compatibility. Ensure thatValkyrieConfig.general.windowTitle
is properly sanitized to prevent potential security vulnerabilities.
55-57
: The modification to allow configurable depth buffer settings through@ModifyConstant
is a straightforward and effective way to enhance rendering capabilities based on user preferences. This change aligns well with the PR's objectives.
46-49
: While the use of@Overwrite
for setting window icons provides a flexible way to customize the game's appearance, it's important to carefully consider the implications of completely replacing the original method. Verify that this is the most appropriate approach and assess potential compatibility issues with other mods or future updates.
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.
LGTM
📝 Description
Replaces the mixin that overwrites the
Minecraft#createDisplay
method with two ModifyConstant mixins to fix compatibility with RenderLib 1.3.3+.🎯 Goals
Fix compatibility with RenderLib 1.3.3+.
❌ Non Goals
Change any Valkyrie functionality or introduce new bugs.
🚦 Testing
Couldn't create a Valkyrie build and thus just created a test mod with the new ModifyConstant mixins.
⏮️ Backwards Compatibility
PR is backward compatible.
📚 Related Issues & Documents
#23
🖼️ Screenshots/Recordings
None
📖 Added to documentation?
😄 [optional] What gif best describes this PR or how it makes you feel?
😶
Summary by CodeRabbit