Conversation
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on improving code documentation by adding Javadoc comments to numerous mixin classes across the codebase. This is a valuable change for maintainability. The secondary focus, as indicated by the title, is fixing and enhancing the 'disable helmet auto-eat' feature. This includes updating a mixin to use mapped method names for better readability and adding logic to hide the corresponding tooltip when the feature is disabled. My review identifies a potential NullPointerException in the new tooltip-hiding logic and suggests a fix. Overall, the changes are positive, especially the widespread addition of documentation.
| private boolean gtexpert$hideAutoEatTooltip(List<String> list, Object element) { | ||
| if (GTEConfigHolder.ceuOverride.disableHelmetAutoEat && | ||
| element.equals(I18n.format("metaarmor.tooltip.autoeat"))) { | ||
| return false; | ||
| } | ||
| return list.add((String) element); | ||
| } |
There was a problem hiding this comment.
This redirect method is susceptible to a NullPointerException if element is null, as element.equals(...) would be called. It's safer to invoke equals on the string literal, which is known to be non-null.
Additionally, the cast (String) element is unsafe and will throw a ClassCastException if a non-String object is ever added to the tooltip list. While this might be unlikely in the context of tooltips, it's a potential runtime error. The current logic would attempt the cast for any object that doesn't match the auto-eat tooltip.
The suggested change below fixes the potential NullPointerException.
| private boolean gtexpert$hideAutoEatTooltip(List<String> list, Object element) { | |
| if (GTEConfigHolder.ceuOverride.disableHelmetAutoEat && | |
| element.equals(I18n.format("metaarmor.tooltip.autoeat"))) { | |
| return false; | |
| } | |
| return list.add((String) element); | |
| } | |
| private boolean gtexpert$hideAutoEatTooltip(List<String> list, Object element) { | |
| if (GTEConfigHolder.ceuOverride.disableHelmetAutoEat && | |
| I18n.format("metaarmor.tooltip.autoeat").equals(element)) { | |
| return false; | |
| } | |
| return list.add((String) element); | |
| } |
No description provided.