Fix CCJ-229: implement new health bar styles for Junior #7761
Conversation
Allows us to set a new `healthBarStyle = 'pill-with-ticks', which shows rounded corners, a semi-transparent background, and discrete sections instead of displaying just a continuous rectangle. Also updates the colors used a little bit.
WalkthroughThe changes enhance the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
app/lib/surface/Lank.coffee (3)
567-574: LGTM! New health bar style implemented correctly.The changes to the
updateStatsmethod successfully implement the new 'pill-with-ticks' health bar style while maintaining compatibility with the existing style. This is a good approach for introducing new features.Consider extracting the health bar update logic into a separate method for better readability and maintainability.
You could refactor this part as follows:
updateHealthBar: -> if @thang.healthBarStyle is 'pill-with-ticks' @addHealthBar() else @scaleHealthBar() scaleHealthBar: -> healthPct = Math.max(@thang.health / @thang.maxHealth, 0) @healthBar.scaleX = healthPct / @options.floatingLayer.resolutionFactorThis refactoring would make the
updateStatsmethod cleaner and easier to maintain.
602-617: New health bar style implemented correctly.The changes to the
addHealthBarmethod successfully implement the new 'pill-with-ticks' health bar style with appropriate conditions. The use ofjuniorHealthColorsfor this style is a good way to visually differentiate junior health bars.Consider adding a comment explaining the significance of the max health <= 10 condition for clarity.
Add a comment to explain the max health condition:
# Use the 'pill-with-ticks' style only for entities with low max health (e.g., junior entities) if (@thang.healthBarStyle is 'pill-with-ticks') and @thang.maxHealth <= 10This will help other developers understand the reasoning behind this condition.
20-24: New junior health colors added successfully.The introduction of
juniorHealthColorssupports the new health bar style by providing specific colors for junior entities. This is consistent with the changes in theaddHealthBarmethod.For consistency and easier maintenance, consider using the same color for the 'neutral' team in both
healthColorsandjuniorHealthColors.Update the
juniorHealthColorsdefinition as follows:juniorHealthColors = ogres: [0, 240, 255] humans: [253, 20, 48] neutral: healthColors.neutralThis ensures that any future changes to the neutral team color will be automatically reflected in both color schemes.
app/lib/surface/sprite_utils.coffee (1)
73-78: Consider performance implications with highmaxTicksvaluesThe loop adding vertical tick marks:
for i in [1...maxTicks] tickX = i * (pieceWidth + TICK_WIDTH) - TICK_WIDTH/2 g.moveTo(tickX, -HEIGHT/2 - 5) g.lineTo(tickX, HEIGHT/2 + 5)could impact rendering performance if
maxTicksis large. If there is a possibility ofmaxTicksbeing a high number, consider limiting the maximum value or optimizing the rendering logic to maintain performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- app/lib/surface/Lank.coffee (3 hunks)
- app/lib/surface/sprite_utils.coffee (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
app/lib/surface/Lank.coffee (1)
Line range hint
1-1000: Summary: New health bar style successfully implementedThe changes in this file successfully implement a new 'pill-with-ticks' health bar style for junior entities (those with max health <= 10). The implementation includes:
- A new
juniorHealthColorsscheme for visual differentiation.- Updates to the
updateStatsmethod to handle the new style.- Modifications to the
addHealthBarmethod to create the new style when appropriate.These changes enhance the visual representation of health for certain entities while maintaining backward compatibility with the existing health bar system. The code is well-structured and consistent with the existing codebase.
To further improve the code:
- Consider extracting the health bar update logic in
updateStatsto a separate method.- Add a comment explaining the max health condition in
addHealthBar.- Use the same color for the 'neutral' team in both color schemes for consistency.
Overall, this is a solid implementation that adds value to the game's visual feedback system.
app/lib/surface/sprite_utils.coffee (7)
6-16: Confirm that default behavior is maintained whenmaxTicksis not providedIn the block beginning with
unless maxTicks, the code handles the simple rectangular bar style, which is the default behavior. Please verify that this section maintains the previous functionality without any regressions whenmaxTicksis undefined or null.
17-19: Verify handling whenticksis zero or undefinedThe condition
else if not ticksis intended to draw no bar whenticksis zero or not provided. Since0is a falsy value in JavaScript/CoffeeScript, this condition will trigger whenticksis zero. Please ensure this is the desired behavior and that it correctly represents a health value of zero.
22-29: Double-check calculations forWIDTHandpieceWidthThe calculations for
WIDTHandpieceWidthare crucial for correctly scaling the progress bar based onmaxTicks. Please verify that these calculations produce the expected dimensions across various values ofmaxTicks, especially at boundary conditions (e.g., whenmaxTicksis 1, 3, or exceeds 10).
38-43: Ensure the background shape is rendered correctlyThe drawing commands for the background shape use precise curvature to create the pill shape:
g.moveTo(radius, -HEIGHT/2) g.lineTo(WIDTH - radius, -HEIGHT/2) g.arc(WIDTH - radius, 0, radius, -Math.PI/2, Math.PI/2, false) g.lineTo(radius, HEIGHT/2) g.arc(radius, 0, radius, Math.PI/2, -Math.PI/2, false) g.closePath()Please verify that these commands produce the desired shape across different
WIDTHandHEIGHTvalues, and that the arcs connect smoothly with the lines.
57-69: Validate path drawing logic for filled piecesThe logic for drawing the filled portion of the progress bar involves conditional paths based on whether
ticksequalsmaxTicks. Please ensure that in both cases (full health bar and partial health bar), the paths are correctly formed, and the visual output aligns with the design specifications.
Line range hint
80-83: Set z-index and bounds appropriately for the shapeThe final steps set the z-index and bounds for the shape:
s = new createjs.Shape(g) s.z = 100 s.bounds = [0, -HEIGHT/2, WIDTH, HEIGHT]Confirm that the
zvalue aligns with rendering expectations in the broader context, and that the bounds accurately encompass the entire progress bar graphics.
3-3: Ensure all usages ofcreateProgressBarare updated with new parametersThe
createProgressBarfunction now includes additional parametersticksandmaxTicks. This change may affect existing calls elsewhere in the codebase. Please ensure that all calls to this function are updated accordingly to prevent potential runtime errors.Run the following script to find all usages of
createProgressBar:✅ Verification successful
All usages of
createProgressBarare updated with the new parameters.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all calls to createProgressBar and check parameter usage # Search for function calls to createProgressBar rg --type coffee -F 'createProgressBar(' -A 2Length of output: 78
Script:
#!/bin/bash # Description: Find all calls to createProgressBar in .coffee files and check parameter usage # Search for function calls to createProgressBar in .coffee files rg 'createProgressBar\(' --type-add 'coffee:*.coffee' --type coffee -A 2Length of output: 289
Script:
#!/bin/bash # Description: Alternative method to find all calls to createProgressBar in .coffee files # Find all .coffee files and search for createProgressBar calls fd --extension coffee --type f | xargs rg 'createProgressBar\(' -A 2Length of output: 285
| g.setStrokeStyle(0) # No stroke for filled pieces | ||
| g.beginFill(createjs.Graphics.getRGB(color...)) | ||
|
|
||
| filledWidth = Math.min(ticks * (pieceWidth + TICK_WIDTH) - TICK_WIDTH, WIDTH - STROKE_WIDTH) |
There was a problem hiding this comment.
Review filledWidth calculation for potential off-by-one error
The computation of filledWidth:
filledWidth = Math.min(ticks * (pieceWidth + TICK_WIDTH) - TICK_WIDTH, WIDTH - STROKE_WIDTH)may lead to an off-by-one error due to the subtraction of TICK_WIDTH. This could result in improper rendering of the filled portion of the progress bar. Please confirm that the formula accurately reflects the intended visual representation for all values of ticks.
Allows us to set a new
healthBarStyle = 'pill-with-ticks', which shows rounded corners, a semi-transparent background, and discrete sections instead of displaying just a continuous rectangle. Also updates the colors used a little bit.Design Exploration
Implementation Close-Up
The scaling/resolution isn't great, but it wasn't on the original health bars, either.
Implementation Zoomed Out
Summary by CodeRabbit
New Features
Bug Fixes
Documentation