-
-
Notifications
You must be signed in to change notification settings - Fork 417
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
HealthTags improvements #801
base: master
Are you sure you want to change the base?
Conversation
does it also display max health? |
For mobs, always. For players, it is selectable with a checkbox. |
This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks. Pull requests should be closed if:
|
📝 WalkthroughWalkthroughThe recent changes involve significant enhancements to the 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/util/RenderUtils.java (2 hunks)
Additional comments (Suppressed): 7
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (7)
28-30: The
HealthTagsHack
class now implements theRenderListener
interface. This change allows the class to listen and respond to render events. Ensure that theonRender()
method is correctly implemented to handle these events.31-50: New settings have been added to the
HealthTagsHack
class. These settings allow for more customization of the health tags, such as displaying them for mobs, at unlimited range, showing maximum health for players, and rounding health values. These settings are added to the hack in the constructor.53-63: The
onEnable()
andonDisable()
methods have been overridden to add and remove theHealthTagsHack
class as aRenderListener
when the hack is enabled and disabled, respectively. This is a good practice as it ensures that the class only listens to render events when the hack is enabled.65-84: The
onRender()
method has been overridden to render the health tags. It checks if themobs
setting is enabled, and if so, it iterates over all entities in the world. If an entity is aMobEntity
, it calculates the health and maximum health (either rounded or not, based on theround
setting), creates a text tag with these values, and uses theRenderUtils.renderTag()
method to render the tag above the entity. The range at which the tags are rendered can be unlimited, based on theunlimitedRange
setting. This method is well-structured and makes good use of the new settings.86-99: The
addHealth()
method has been updated to accommodate the new settings. It now calculates the health and maximum health (either rounded or not, based on theround
setting), and appends these values to the nametag. If themax
setting is enabled, it appends both the health and maximum health; otherwise, it only appends the health. The health values are colored based on thegetColor()
method.101-113: The
getColor()
method has been updated to take the health and maximum health as parameters, and it now returns a color based on the percentage of health remaining, rather than fixed thresholds. This is a more flexible approach and allows for more accurate color representation of health.115-118: A new method
hasMobHealthTags()
has been added to check if the hack is enabled and if themobs
setting is checked. This method can be useful in other parts of the code to check if mob health tags should be displayed.
This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks. Pull requests should be closed if:
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/util/RenderUtils.java (2 hunks)
Additional comments: 10
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (8)
10-26: The addition of new import statements aligns with the new features and methods described in the summary. Ensure that these imports are used within the file and are not unnecessarily increasing the compilation time or memory footprint.
29-29: The
HealthTagsHack
class now implementsRenderListener
. This change is consistent with the summary stating that event handling methods for rendering have been added.31-43: The new settings added to the
HealthTagsHack
class are consistent with the PR objectives to allow more control over the display of health tags. Ensure that these settings are properly integrated into the UI and saved/loaded as part of the user's preferences.55-64: The
onEnable
andonDisable
methods correctly manage the addition and removal of this class as aRenderListener
. This is necessary for the proper functioning of the event-driven rendering logic.68-95: The
onRender
method has been modified to render health tags above mobs. The logic checks if themobs
setting is enabled and uses theRenderUtils.renderTag
method to draw the tags. Ensure that the rendering respects the game's performance and does not introduce any visual glitches or lag.98-115: The
addHealth
method has been updated to format health information and append it to the entity's name tag. The use ofDecimalFormat
and theround
setting to control the display format is consistent with the PR objectives. Ensure that the health information is displayed correctly and is readable in the game's UI.118-129: The
getColor
method's logic for determining the color of health tags based on the health percentage is straightforward and follows a clear pattern. This should make it easy for players to quickly assess health levels.132-135: The
hasMobHealthTags
method provides a way to check if mob health tags are enabled. This could be useful for avoiding tag collisions in the future, as mentioned in the PR objectives.src/main/java/net/wurstclient/util/RenderUtils.java (2)
17-35: The changes in import statements reflect the new functionality added to the
RenderUtils
class. Removed imports suggest a refactoring or removal of methods that used them. Ensure that the removed imports do not affect other methods in the class.897-942: The new
renderTag
method is added to handle rendering tags for entities. It includes logic for calculating the distance to the camera and scaling the tag based on that distance. The method also respects theNameTagsHack
settings, which is consistent with the PR objectives. Ensure that the method is called in the right context and that the tags are rendered correctly in the game.
Also reverts the non-standard 75 block range back to 64.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/util/RenderUtils.java (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java
- src/main/java/net/wurstclient/util/RenderUtils.java
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/mixin/EntityRendererMixin.java (2 hunks)
- src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java (1 hunks)
- src/main/java/net/wurstclient/settings/SliderSetting.java (1 hunks)
Additional comments: 4
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (2)
28-121: The implementation of the
HealthTagsHack
class with the new settings andRenderListener
interface is correct. TheonRender
method properly checks themobs
setting before rendering tags. TheaddHealth
method uses theprecision
setting to format health values. ThegetColor
method now calculates color based on health percentage. Ensure that theprecision
setting is properly initialized and that its value is within the expected range to prevent any runtime exceptions.118-121: The
hasMobHealthTags
method is a straightforward check that combines the enabled state of the hack with themobs
setting. This method is correctly implemented and follows best practices.src/main/java/net/wurstclient/mixin/EntityRendererMixin.java (1)
- 45-48: The change to cast
text
toMutableText
in theonRenderLabelIfPresent
method is correct and necessary for theaddHealth
method to work with mutable text. This change aligns with the modifications in theHealthTagsHack
class.src/main/java/net/wurstclient/settings/SliderSetting.java (1)
- 326-327: The addition of the
ROUNDING_PRECISION
constant in theSliderSetting
class is correct and follows the established pattern forValueDisplay
constants. This constant will allow for a display behavior that shows the number of decimal places based on the slider's value.
src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java
Outdated
Show resolved
Hide resolved
src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java
I'd prefer to add it back later when/if that other PR is merged.
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/util/RenderUtils.java (2 hunks)
Additional comments: 7
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (6)
10-25: Added multiple imports to support the new features and changes in the
HealthTagsHack
class. Ensure that all these imports are used within the class and there are no unused imports, which can lead to unnecessary dependencies.28-40: The class now implements
RenderListener
and has added settings for displaying health tags on mobs, showing maximum health, and setting rounding precision. Verify that the default values for these settings align with the intended functionality and user experience.51-60: The
onEnable
andonDisable
methods correctly manage the addition and removal of this class as aRenderListener
. This is a good practice to ensure that the class only listens for render events when it is enabled, which can help with performance.63-83: The
onRender
method now includes logic to render health tags for mob entities. Ensure that the rendering logic is optimized and does not cause performance issues, especially when there are many entities on screen. Also, verify that the color and scale of the tags are appropriate and consistent with the game's UI standards.85-101: The
addHealth
method now uses float values for health and max health, with updated formatting and color logic. Ensure that the rounding precision setting is working as expected and that the color logic accurately reflects the health status according to the game's design. Additionally, verify that the method handles entities without a custom name correctly, appending a space only when necessary.104-115: The
getColor
method has been updated to determine the color formatting based on the entity's current health relative to its max health. Verify that the thresholds for color changes (25%, 50%, 75%) are consistent with the game's design and that the colors used (DARK_RED, GOLD, YELLOW, GREEN) provide clear and intuitive visual cues for the player.src/main/java/net/wurstclient/util/RenderUtils.java (1)
- 896-941: The new
renderTag
method should be checked for:
- Correct usage of the
matrixStack
and proper push/pop encapsulation to avoid affecting the matrix stack elsewhere.- Proper handling of the
partialTicks
for smooth rendering.- Correct calculation of
scale
based on distance, ensuring it doesn't get too large or too small.- Appropriate use of
TextRenderer
andTextLayerType
for drawing text with background and see-through layers.- Ensuring that the magic number
15728880
used for light is appropriate or if it should be replaced with a named constant for clarity.- Confirming that the visibility check (
dist > 4096
) aligns with the game's rendering distance settings andnameTags.isUnlimitedRange()
logic.Verification successful
The search for the magic number
15728880
within thesrc/main/java/net/wurstclient/util/
directory confirms that this number is used exclusively in therenderTag
method of theRenderUtils.java
file. It appears twice, once for each text layer type:NORMAL
andSEE_THROUGH
. Since there are no other occurrences or named constants for this value, it suggests that this number is specific to the rendering context within this method.Given that the number is used in a specific rendering context and is not referenced elsewhere, it may be acceptable to use it as a literal if it represents a standard value for lighting in Minecraft's rendering system. However, if this value has a broader significance or is subject to change, it would be best practice to define it as a named constant for clarity and maintainability.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the magic number for light is standard or if it should be replaced with a named constant. rg --type java '15728880' src/main/java/net/wurstclient/util/Length of output: 255
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java (1 hunks)
Additional comments: 3
src/main/java/net/wurstclient/settings/RoundingPrecisionSetting.java (3)
21-29: The addition of a check to ensure
min
is non-negative in the constructor is a good safeguard against potentialArrayIndexOutOfBoundsException
.34-47: The
getFormat
method correctly initializesDecimalFormat
instances on demand using lazy initialization, and the constructor's bounds check formin
ensures that array access is safe.50-53: The
format
method is straightforward and correctly delegates togetFormat
for formatting the value.
This pull request has been open for a while with no recent activity. If you're still working on this or waiting for a review, please add a comment or commit within the next 7 days to keep it open. Otherwise, the pull request will be automatically closed to free up time for other tasks. Pull requests should be closed if:
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
Additional comments: 1
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1)
- 85-101: The
addHealth
method introduces decimal precision and color-based health representation, fulfilling the PR objectives. The use of theprecision
setting to format health values and the conditional logic for displaying maximum health are well-implemented. However, ensure that thegetColor
method, which determines the color based on health, is robust against edge cases, such as whenmaxHealth
is zero, to prevent division by zero errors or misleading color representations.Ensure the
getColor
method handles edge cases, such as zeromaxHealth
, appropriately.
NOTE: Rotating the tag only applies for 1.21 and above
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: 2
🧹 Outside diff range and nitpick comments (3)
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1)
93-95
: Ensure consistent formatting of health valuesWhen constructing the
healthString
, consider usingStringBuilder
for better performance, especially if the method is called frequently. Although the impact may be minimal here, it's a good practice for string concatenation in loops or frequently called methods.Apply this diff to use
StringBuilder
:String healthString = precision.format(health); if(showMaxHealth.isChecked()) - healthString += "/" + precision.format(maxHealth); +{ + StringBuilder sb = new StringBuilder(healthString); + sb.append("/").append(precision.format(maxHealth)); + healthString = sb.toString(); +}src/main/java/net/wurstclient/util/RenderUtils.java (2)
776-777
: Consider defining4096
as a constant for clarity.Defining the value
4096
as a named constant (e.g.,MAX_RENDER_DISTANCE_SQUARED
) enhances readability and makes future adjustments easier.Apply this diff to introduce a constant:
+ private static final double MAX_RENDER_DISTANCE_SQUARED = 4096; ... double dist = dispatcher.getSquaredDistanceToCamera(entity); - if(dist > 4096 && !nameTags.isUnlimitedRange()) + if(dist > MAX_RENDER_DISTANCE_SQUARED && !nameTags.isUnlimitedRange()) return;
789-795
: Use named constants for scaling factors.Defining
0.025F
and10
as constants improves code readability and maintainability.Apply this diff:
+ private static final float BASE_SCALE = 0.025F; + private static final double SCALE_DISTANCE_THRESHOLD = 10.0; ... float scale = BASE_SCALE; if(nameTags.isEnabled()) { double distance = WurstClient.MC.player.distanceTo(entity); - if(distance > 10) - scale *= distance / 10; + if(distance > SCALE_DISTANCE_THRESHOLD) + scale *= distance / SCALE_DISTANCE_THRESHOLD; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/HealthTagsHack.java (1 hunks)
- src/main/java/net/wurstclient/util/RenderUtils.java (2 hunks)
🧰 Additional context used
🪛 ast-grep
src/main/java/net/wurstclient/hacks/HealthTagsHack.java
[warning] 75-75: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (entity, Text.literal(""))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 97-97: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (Text.literal(" "))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 75-75: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (entity, Text.literal(""))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 97-97: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (Text.literal(" "))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/main/java/net/wurstclient/util/RenderUtils.java
[warning] 785-786: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: ((float)Math.PI,
new Quaternionf())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 785-786: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: ((float)Math.PI,
new Quaternionf())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🔇 Additional comments (5)
src/main/java/net/wurstclient/hacks/HealthTagsHack.java (4)
29-47
: Well-implemented settings enhance user customizationThe addition of the
mobs
,showMaxHealth
, andprecision
settings is well-executed and aligns perfectly with the PR objectives. These settings provide users with greater control over how health tags are displayed, enhancing the overall flexibility and usability of the feature.
53-59
: Proper registration of the RenderListenerThe implementation of the
onEnable
andonDisable
methods ensures that theHealthTagsHack
correctly registers and deregisters itself as aRenderListener
. This is crucial for managing the rendering lifecycle effectively.
103-114
: Thresholds ingetColor
method are well-definedThe health percentage thresholds used in the
getColor
method provide clear and intuitive color coding for health tags, enhancing visual feedback for users.
97-98
: 🛠️ Refactor suggestionSimplify nametag spacing logic
Since
nametag
could be empty when rendering health tags for mobs, the condition to append a space can be simplified to avoid unnecessary checks.Apply this diff to simplify the logic:
- if(!nametag.getString().isEmpty()) - nametag = nametag.append(Text.literal(" ")); + nametag = nametag.append(Text.literal(" "));This change ensures a space is always appended before the health string, which doesn't affect the display for empty
nametag
cases.Likely invalid or redundant comment.
🧰 Tools
🪛 ast-grep
[warning] 97-97: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (Text.literal(" "))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 97-97: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (Text.literal(" "))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/main/java/net/wurstclient/util/RenderUtils.java (1)
766-813
: Implementation ofrenderTag
method is correct.The new
renderTag
method correctly renders tags above entities with appropriate scaling and positioning based on distance and the state ofNameTagsHack
.🧰 Tools
🪛 ast-grep
[warning] 785-786: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: ((float)Math.PI,
new Quaternionf())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 785-786: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: ((float)Math.PI,
new Quaternionf())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
The PR has been updated. Note that 1.21 and above should use |
-Added an option to display health up to 2 decimal places. This may be useful in some cases.
-Player health colors are now based upon their actual maximum health.
-Added a nametag for display healthtags for mobs. The renderTag() is based upon Minecraft's code to render nametags in renderLabelIfPresent(). I use this method to display item nametags as well as the mob owner (#608).
Note: The hasMobHealthTags() method currently has no use, but if I make a pull request for MobOwner in the future, it will help prevent the tags from colliding with each other.