feat: add support for mod sets#161
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR introduces a new styling module consolidating font and color constants, adds header rendering for mod sets with image tinting, refactors the generator API to accept a props object, migrates styling exports from utils to the new module, and adds utility functions for header fetching and hex-to-RGB color conversion. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
README.md (1)
31-31: Updated function call structureUpdating the function call to use an object parameter structure improves code clarity by making parameter names explicit at the call site.
However, there's a typo in the comment: "defualt" should be "default".
-const image = generateBasicMod({mod, rank: 3}); // You can set rank to whatever rank you want by defualt it's 0 +const image = generateBasicMod({mod, rank: 3}); // You can set rank to whatever rank you want by default it's 0src/utils.ts (2)
111-115: Consider adding JSDoc to document parameter reorderingThe
modDescriptionfunction parameters have been reordered and defaults added, which is a breaking change to the function signature.Consider adding JSDoc to clearly document the new parameter order:
+/** + * Generates a formatted description for a mod based on rank, description, and level stats + * @param rank The rank of the mod (defaults to 0) + * @param description Optional description text + * @param levelStats Optional level statistics + * @returns Formatted description string or undefined + */ export const modDescription = ( rank: number = 0, description?: string | undefined, levelStats?: LevelStat[] | undefined ): string | undefined => {
194-198: Add error handling for malformed uniqueNamesThe
setHeaderfunction assumes a specific format for uniqueNames, but doesn't handle potential errors.Consider adding error handling for uniqueNames that don't follow the expected format:
export const setHeader = async (uniqueName: string) => { const parts = uniqueName.split('/'); + if (parts.length < 2) { + throw new Error(`Invalid uniqueName format: ${uniqueName}`); + } const name = parts.reverse()[1]; return fetchModPiece(`${name}Header.png`); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/readme/Augur Message.pngis excluded by!**/*.png
📒 Files selected for processing (6)
README.md(2 hunks)src/data.ts(1 hunks)src/drawers.ts(6 hunks)src/generator.ts(4 hunks)src/utils.ts(3 hunks)tests/generator.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/generator.ts (2)
src/utils.ts (3)
CanvasOutput(166-170)setHeader(194-198)exportCanvas(172-192)src/drawers.ts (2)
horizontalPad(8-8)drawHeader(27-54)
tests/generator.spec.ts (1)
src/utils.ts (1)
Format(164-164)
🔇 Additional comments (21)
src/data.ts (2)
1-3: Type definition looks goodThe
UniquenameTypetype definition provides a clear structure for the mapping objects that follow. Using a custom type enhances code readability and maintainability.
5-11: Properly structured mapping for mod raritiesGood organization of mod rarity mapping with clear key-value pairs. This modular approach of moving constants to a dedicated data file improves code organization.
README.md (1)
7-7: Good example additionAdding the Augur Message example image enriches the documentation by showing a mod set example, which aligns with the new feature being implemented.
tests/generator.spec.ts (2)
24-24: Good test coverage for mod setsAdding a mod set example (
WarframeAugurMessageMod) to the test suite ensures the new feature is properly tested.
39-39: Updated function call aligns with new interfaceThe function call has been properly updated to use the new object parameter structure, which aligns with the changes in the generator interface.
src/generator.ts (6)
4-6: Improved imports organizationThe imports have been reorganized to better group related functionality. Good practice to include
horizontalPadfrom 'drawers.js' and add the import formodRarityMapfrom the new data file.
8-14: Well-structured interface for propsThe new
GenerateModPropsinterface provides a clear structure for the parameters, making the function more maintainable and the API more consistent. The optional parameters are appropriately marked.
24-24: Documentation and function signature updatedThe function documentation and signature have been updated to reflect the new interface. The destructuring of props makes the code cleaner.
Also applies to: 30-30, 33-33, 37-38
65-72: Improved code organization with variable extractionExtracting
topFrameHeightinto a variable improves readability. The consistent use ofhorizontalPadalso fixes a naming inconsistency.
74-84: New support for mod setsThe implementation for mod sets looks good. It checks if
mod.modSetexists and renders the appropriate header with the correct tint based on the tier.
107-107: Improved default handling for output parameterUsing the nullish coalescing operator
??to provide a default value for the output parameter is a good practice. This ensures a consistent default without overriding explicitundefinedvalues.src/drawers.ts (7)
8-8: Fixed variable name typoThe variable name has been correctly changed from "horizantalPad" to "horizontalPad" for consistency and accuracy.
27-54: Good implementation of the new drawHeader functionThe new
drawHeaderfunction properly implements image tinting based on tier color. The brightness calculation and weighting approach provides a good balance between the original image and the tint color.
155-155: Interface update supports mod setsThe addition of the optional
setBonusproperty to theBackgroundPropsinterface properly supports the new mod set functionality.
201-201: Added strokeStyle for visual consistencySetting the stroke style to match the text color ensures visual consistency when drawing set bonus rectangles.
210-210: Updated modDescription function callThe function call has been updated to match the new parameter order in utils.ts.
227-257: Good implementation of mod set visualizationThe code properly implements the visualization of mod sets with:
- Rectangle indicators for each set piece
- Filled rectangles based on the setBonus value
- Display of the active set bonus stat
This provides clear visual feedback about set completion status and benefits.
174-178: Fixed thumbnail positioning with new variable nameThe thumbnail positioning and dimensions have been updated to use the corrected
horizontalPadvariable, maintaining consistent padding.src/utils.ts (3)
8-8: Good refactoring of constantsMoving
modRarityMapandtierColorto a dedicated data file improves code organization by centralizing game-specific data.
200-205: Good implementation of hexToRgb utilityThe
hexToRgbfunction properly handles hex color conversion with a fallback for invalid formats.
183-187: Improved default handling in exportCanvasThe changes ensure that default values are used when quality or config parameters are not provided, preventing potential errors.
SlayerOrnstein
force-pushed
the
feat-headers
branch
from
068d5c6 to
1797cea
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/drawers.ts (1)
193-258:⚠️ Potential issue | 🟠 Major
modTextHeightis calculated fromstats[0]but a different stat is rendered whensetBonus > 0
modTextHeightdrives the thumbnail's height budget (thumbHeight = thumb.height - modTextHeight). It is sized for the line count ofstats[0], but the drawn text comes frommodSet.stats[Math.min(setBonus ?? 0, stats.length - 1)](line 235/255). WhensetBonus > 0and the active stat has more wrapped lines thanstats[0], the description overflows the image area; when it has fewer, there is unnecessary blank space.Fix: compute the active stat index before calling
textHeight, then use the same index for both the height measurement and the rendering:🐛 Proposed fix
if (mod.modSet) { modSet = find.findItem(mod.modSet) as ModSet; + const setStatsIndex = Math.min(setBonus ?? 0, (modSet?.stats.length ?? 1) - 1); modTextHeight = - modTextHeight + textHeight(context, maxWidth, undefined, modSet?.stats[0].split('\n')) + modSetProgressHeight; + modTextHeight + textHeight(context, maxWidth, undefined, modSet?.stats[setStatsIndex]?.split('\n')) + modSetProgressHeight; }And below, when rendering, replace the duplicated
Math.minderivation:- const stats = Math.min(setBonus ?? 0, modSet.stats.length - 1); + // setStatsIndex already matches the height budget ... - wrapText(context, modSet.stats[stats], maxWidth).forEach(...) + wrapText(context, modSet.stats[setStatsIndex], maxWidth).forEach(...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/drawers.ts` around lines 193 - 258, The mod thumbnail height is computed using textHeight for modSet.stats[0] but rendering uses Math.min(setBonus ?? 0, modSet.stats.length - 1), causing mismatched heights; compute the active stat index once (e.g., const activeStat = Math.min(setBonus ?? 0, modSet.stats.length - 1)) immediately after resolving modSet and use that activeStat when calling textHeight (replace the current stats[0] usage) and again when rendering the mod set text (replace the duplicated Math.min expression), ensuring you null-check modSet before accessing modSet.stats.
♻️ Duplicate comments (1)
tests/generator.spec.ts (1)
30-38:i <= mods.lengthintentionally over-iterates — prefer< mods.lengthThe loop runs one extra iteration (
mods[mods.length]→undefined) relying on theif (!mod) returnguard to no-op. The intent is cleaner as a standard<bound.- for (let i = 0; i <= mods.length; i += 1) { + for (let i = 0; i < mods.length; i += 1) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/generator.spec.ts` around lines 30 - 38, The for-loop uses "for (let i = 0; i <= mods.length; i += 1)" which iterates one past the end and relies on the "if (!mod) return" guard; change the loop to stop at the correct bound ("i < mods.length") to avoid an extra iteration and remove reliance on the guard. Update the loop controlling expression where "i" is declared; keep the body using "const mod = find.findItem(mods[i]) as Mod" and the existing early return logic unchanged.
🧹 Nitpick comments (2)
src/drawers.ts (2)
241-244: Hoist loop-invariantbonusout of the loop
(setBonus ?? 0) - 1is constant across all iterations of theforloop.♻️ Proposed fix
+ const bonus = (setBonus ?? 0) - 1; for (let i = 0; i < modSet.stats.length; i++) { - // default to negative to imply disabled - const bonus = (setBonus ?? 0) - 1; if (bonus < i) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/drawers.ts` around lines 241 - 244, Compute the loop-invariant bonus once before iterating instead of inside the for loop: move the expression (setBonus ?? 0) - 1 into a const named bonus defined just before the for loop that iterates over modSet.stats, then use that bonus variable inside the loop (replace the in-loop declaration and comparison if (bonus < i) accordingly).
159-170:modSetRankinBackgroundPropsis never usedThe field is declared in the interface but is neither destructured in
backgroundImagenor accessed elsewhere in the function.♻️ Proposed fix
- modSetRank?: number; setBonus?: number;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/drawers.ts` around lines 159 - 170, BackgroundProps declares a modSetRank field that is never used; remove modSetRank from the BackgroundProps interface to eliminate dead API surface (or if it was intended to affect rendering, add it to the destructured parameters of backgroundImage and apply it where rank/mod/set bonus are used). Locate the BackgroundProps interface and the backgroundImage function to either delete the modSetRank property from the type or add it to the backgroundImage parameter list and incorporate it into the existing logic that uses rank/mod/modSet/setBonus so callers and tests remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/drawers.ts`:
- Around line 193-197: The code calls modSet.stats[0].split('\n') without
guarding against an empty stats array which throws if stats[0] is undefined;
update the mod.modSet handling so after retrieving modSet (via find.findItem)
you check that modSet.stats exists and has at least one entry and only then
split the first stat, otherwise pass undefined or an empty array into
textHeight; modify the modTextHeight computation (using symbols mod.modSet,
modSet, modSet.stats, textHeight, modSetProgressHeight, modTextHeight) to use
this guarded value.
In `@src/generator.ts`:
- Around line 8-14: The interface GenerateModProps is currently module-private
but the public entrypoint generate expects that shape; export GenerateModProps
so consumers can import and type their props. Modify the declaration of
GenerateModProps to be exported (export interface GenerateModProps) and ensure
any dependent types or re-exports used by generate (e.g., Mod, CanvasOutput if
referenced) are also exported or imported where needed so external callers can
construct typed props matching generate(modProps).
In `@src/utils.ts`:
- Around line 168-170: The code currently uses the falsy-coalescing pattern
quality || 100 when calling canvas.encode (in the 'webp' and 'jpeg' branches),
which treats an explicit quality of 0 as absent; change those calls to use a
nullish-coalescing or explicit undefined check (e.g., quality ?? 100 or quality
=== undefined ? 100 : quality) so that a caller passing quality: 0 is preserved
and only undefined/null falls back to 100.
---
Outside diff comments:
In `@src/drawers.ts`:
- Around line 193-258: The mod thumbnail height is computed using textHeight for
modSet.stats[0] but rendering uses Math.min(setBonus ?? 0, modSet.stats.length -
1), causing mismatched heights; compute the active stat index once (e.g., const
activeStat = Math.min(setBonus ?? 0, modSet.stats.length - 1)) immediately after
resolving modSet and use that activeStat when calling textHeight (replace the
current stats[0] usage) and again when rendering the mod set text (replace the
duplicated Math.min expression), ensuring you null-check modSet before accessing
modSet.stats.
---
Duplicate comments:
In `@tests/generator.spec.ts`:
- Around line 30-38: The for-loop uses "for (let i = 0; i <= mods.length; i +=
1)" which iterates one past the end and relies on the "if (!mod) return" guard;
change the loop to stop at the correct bound ("i < mods.length") to avoid an
extra iteration and remove reliance on the guard. Update the loop controlling
expression where "i" is declared; keep the body using "const mod =
find.findItem(mods[i]) as Mod" and the existing early return logic unchanged.
---
Nitpick comments:
In `@src/drawers.ts`:
- Around line 241-244: Compute the loop-invariant bonus once before iterating
instead of inside the for loop: move the expression (setBonus ?? 0) - 1 into a
const named bonus defined just before the for loop that iterates over
modSet.stats, then use that bonus variable inside the loop (replace the in-loop
declaration and comparison if (bonus < i) accordingly).
- Around line 159-170: BackgroundProps declares a modSetRank field that is never
used; remove modSetRank from the BackgroundProps interface to eliminate dead API
surface (or if it was intended to affect rendering, add it to the destructured
parameters of backgroundImage and apply it where rank/mod/set bonus are used).
Locate the BackgroundProps interface and the backgroundImage function to either
delete the modSetRank property from the type or add it to the backgroundImage
parameter list and incorporate it into the existing logic that uses
rank/mod/modSet/setBonus so callers and tests remain consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/readme/Augur Message.pngis excluded by!**/*.pngassets/readme/Primed Flow.pngis excluded by!**/*.pngassets/readme/Steel Charge.pngis excluded by!**/*.png
📒 Files selected for processing (6)
package.jsonsrc/drawers.tssrc/generator.tssrc/styling.tssrc/utils.tstests/generator.spec.ts
2 participants
What did you fix?
Add support for creating set mods
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Refactor