-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fixed detection of modded item groups in GUIs #295
Fixed detection of modded item groups in GUIs #295
Conversation
...java/com/github/khanshoaib3/minecraft_access/features/inventory_controls/GroupGenerator.java
Outdated
Show resolved
Hide resolved
...java/com/github/khanshoaib3/minecraft_access/features/inventory_controls/GroupGenerator.java
Outdated
Show resolved
Hide resolved
...java/com/github/khanshoaib3/minecraft_access/features/inventory_controls/GroupGenerator.java
Outdated
Show resolved
Hide resolved
...java/com/github/khanshoaib3/minecraft_access/features/inventory_controls/GroupGenerator.java
Outdated
Show resolved
Hide resolved
...java/com/github/khanshoaib3/minecraft_access/features/inventory_controls/GroupGenerator.java
Outdated
Show resolved
Hide resolved
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.
Forgive my stupid, I can't understand some details in the PR, can you make the code more clear?
I've moved that condition to a boolean and fixed the one indentation error I could see. Short of adding a bunch of comments I don't think there is much I can do to make it clearer but I might be able to have a better look and think of something if I have another look when it's not late afternoon after a party. |
Use method naming, variable naming to describe the purpose of logic, instead of comments. See https://github.com/FabricMC/yarn/blob/24w03b/CONVENTIONS.md |
Some refactor FYI: https://github.com/boholder/minecraft-access/commits/modded-gui-groups/ The or() can be replaced with orElse() thus the result can be directly List without Optional wrapper |
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 👍
/fast-forward |
[Fast Forward CI] modded-gui-groups merged into 1.20... Copied the changelogs from pull request body to doc/CHANGELOG.md... Pushed the changes to origin. |
Changelog
New Features
Feature Updates
Bug Fixes
Translation Changes
Others
Development Chores