-
Notifications
You must be signed in to change notification settings - Fork 30
fix: add string data for archimedea, calendar challenge, and prime vault #730
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
Conversation
WalkthroughAdded two faction entries (Scaldra, Techrot) to faction data, numerous new localization entries in languages.json, and a new translator function Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Translator as tools/translation.ts
Note right of Translator #f0f8ff: archimedeaTypes mapping (new)
Caller->>Translator: translateArchimedeaType(type)
Translator->>Translator: lookup type in archimedeaTypes
alt found
Translator-->>Caller: mapped display value
else not found
Translator-->>Caller: original input (fallback)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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: 3
🧹 Nitpick comments (3)
tools/translation.ts (2)
360-363: Add semicolon for consistency.The
archimedeaTypesobject is missing a semicolon after the closing brace. All similar mapping objects in this file (focusMapat line 314,polarityMapat line 330,eventTypeMapat line 344) include semicolons.Apply this diff:
const archimedeaTypes = { CT_LAB: "Deep Archimedea", CT_HEX: "Temporal Archimedea" -} +};
365-365: Add explicit return type annotation.The function is missing an explicit return type annotation. All similar translator functions in this file (
translateFocusat line 320,translatePolarityat line 337,translateCalendarEventat line 351,translateSeasonat line 358) include: stringreturn type annotations.Apply this diff:
-export const translateArchimedeaType = (type: string) => valMapping(type, archimedeaTypes); +export const translateArchimedeaType = (type: string): string => valMapping(type, archimedeaTypes);data/languages.json (1)
19434-19437: Ensure description formatting consistency.Line 19436 lacks a trailing period while similar descriptions in this section have them (e.g., line 19247 "Max health reduced each time health damage is taken."). Consider adding a period for consistency.
Apply this diff if adopting a consistent period-ending style:
"Undersupplied": { "value": "Undersupplied", - "desc": "Max ammo on all weapons reduced by 75%" + "desc": "Max ammo on all weapons reduced by 75%." },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
data/factionsData.json(1 hunks)data/languages.json(20 hunks)tools/translation.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-17T20:05:33.832Z
Learnt from: SlayerOrnstein
PR: WFCD/warframe-worldstate-data#588
File: data/languages.json:18862-18865
Timestamp: 2024-12-17T20:05:33.832Z
Learning: In `data/languages.json`, upgrade descriptions should remain as provided by Digital Extremes (DE) without modifying formatting inconsistencies.
Applied to files:
data/languages.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (3)
data/factionsData.json (1)
25-31: LGTM! Faction entries added correctly.The new faction entries for Scaldra and Techrot follow the established structure and maintain proper JSON syntax.
tools/translation.ts (1)
365-365: Verify iftranslateArchimedeaTypeshould be included in the default export.The new
translateArchimedeaTypefunction follows the established pattern of other translator functions but is not included in:
- The default export object (lines 399-430)
- The JSDoc documentation (lines 367-398)
Every other
translate*function in this file (translateFocus,translatePolarity,translateCalendarEvent,translateSeason) is included in both. This inconsistency may indicate incomplete implementation, or it could be intentional if this function is meant for named-export-only usage.If this function should be publicly available via the default export (consistent with other translator functions), apply these changes:
1. Update the JSDoc (after line 397):
* @property {function} translateCalendarEvent - Translate the given event key * @property {function} translateSeason - Translate the given season name to a non-unique string + * @property {function} translateArchimedeaType - Translate Archimedea challenge types */2. Update the default export object (after line 429):
translateCalendarEvent, translateSeason, + translateArchimedeaType, };data/languages.json (1)
29-34: Approve data additions (aside from identified text errors).The remaining entries follow consistent JSON structure and localization patterns. Prime Vault items, armor sets, weapons, and challenge modifiers are properly formatted with appropriate display names and descriptions.
Verify that all new item IDs match those present in the game data and that no existing entries were inadvertently modified.
Also applies to: 53-85, 152-166, 326-328, 383-385, 440-451, 460-463, 597-599, 609-620, 700-703, 19227-19444
db49b3a to
c558765
Compare
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/translation.ts (1)
399-430: AddtranslateArchimedeaTypeto the default export object.All other translate functions (
translateFocus,translatePolarity,translateCalendarEvent,translateSeason) are included in the default export object, buttranslateArchimedeaTypeis missing. This inconsistency could break existing code patterns that rely on the default export.Apply this diff to add the new function to the default export:
translateCalendarEvent, translateSeason, + translateArchimedeaType, };Also consider updating the JSDoc typedef comment (lines 367-398) to document the new function:
* @property {function} translateCalendarEvent - Translate the given event key * @property {function} translateSeason - Translate the given season name to a non-unique string + * @property {function} translateArchimedeaType - Translate the given Archimedea type */
🧹 Nitpick comments (1)
tools/translation.ts (1)
360-365: Add explicit return type annotation for consistency.The implementation follows the established pattern correctly. However, add an explicit return type annotation (
: string) to match TypeScript best practices and maintain consistency with similar functions in the codebase.Apply this diff:
-export const translateArchimedeaType = (type: string) => valMapping(type, archimedeaTypes); +export const translateArchimedeaType = (type: string): string => valMapping(type, archimedeaTypes);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
data/factionsData.json(1 hunks)data/languages.json(20 hunks)tools/translation.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-data PR: 588
File: data/languages.json:18862-18865
Timestamp: 2024-12-17T20:05:33.832Z
Learning: In `data/languages.json`, upgrade descriptions should remain as provided by Digital Extremes (DE) without modifying formatting inconsistencies.
📚 Learning: 2024-12-17T19:46:12.851Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-data PR: 588
File: tools/translation.js:326-331
Timestamp: 2024-12-17T19:46:12.851Z
Learning: In `tools/translation.js`, the `translateSeason` function always receives a valid season string starting with `'CST_'`, so additional error handling is unnecessary.
Applied to files:
tools/translation.ts
📚 Learning: 2024-12-17T19:46:33.387Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-data PR: 588
File: tools/translation.js:312-317
Timestamp: 2024-12-17T19:46:33.387Z
Learning: In `tools/translation.js` (JavaScript), the `eventTypeMap` should match the in-game event type translations exactly (one-to-one), and should not be modified to ensure consistency with the game's terminology.
Applied to files:
tools/translation.ts
📚 Learning: 2024-12-17T20:05:33.832Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-worldstate-data PR: 588
File: data/languages.json:18862-18865
Timestamp: 2024-12-17T20:05:33.832Z
Learning: In `data/languages.json`, upgrade descriptions should remain as provided by Digital Extremes (DE) without modifying formatting inconsistencies.
Applied to files:
data/languages.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (5)
data/factionsData.json (1)
26-30: LGTM!The new faction entries follow the established pattern correctly and maintain consistent JSON structure. The additions align with the PR objective of adding Archimedea-related data.
data/languages.json (4)
700-703: Calendar upgrade entry looks good.The
GenerateOmniOrbsOnWeakKillentry is properly structured with descriptive text. No issues identified.
19287-19290: Previous typo fix verified: "gits" → "hits" correction is present.The
FortifiedFoesdescription correctly reads "non-weak point hits" and matches the correction flagged in past review comments.
19363-19366: Previous typo fix verified: "Spped" → "Speed" correction is present.The
Quicksanddescription correctly reads "reduced Movement Speed" and matches the correction flagged in past review comments.
19245-19248: Add period to end of description.The description for
DecayingFleshis missing ending punctuation, which is inconsistent with most other description entries in the file.Apply this diff:
"DecayingFlesh": { "value": "Permanent Injury", - "desc": "Max health reduced each time health damage is taken" + "desc": "Max health reduced each time health damage is taken." },Note: If this text is sourced directly from Digital Extremes without modification, please disregard this comment per the project learnings.
⛔ Skipped due to learnings
Learnt from: SlayerOrnstein Repo: WFCD/warframe-worldstate-data PR: 588 File: data/languages.json:18862-18865 Timestamp: 2024-12-17T20:05:33.832Z Learning: In `data/languages.json`, upgrade descriptions should remain as provided by Digital Extremes (DE) without modifying formatting inconsistencies.
| "Undersupplied": { | ||
| "value": "Undersupplied", | ||
| "desc": "Max ammo on all weapons reduced by 75%" | ||
| }, |
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.
Add period to end of description.
The description for Undersupplied is missing ending punctuation, which is inconsistent with most other description entries in the file.
Apply this diff:
"Undersupplied": {
"value": "Undersupplied",
- "desc": "Max ammo on all weapons reduced by 75%"
+ "desc": "Max ammo on all weapons reduced by 75%."
},Note: If this text is sourced directly from Digital Extremes without modification, please disregard this comment per the project learnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Undersupplied": { | |
| "value": "Undersupplied", | |
| "desc": "Max ammo on all weapons reduced by 75%" | |
| }, | |
| "Undersupplied": { | |
| "value": "Undersupplied", | |
| "desc": "Max ammo on all weapons reduced by 75%." | |
| }, |
🤖 Prompt for AI Agents
In data/languages.json around lines 19434 to 19437, the description string for
the "Undersupplied" entry is missing a terminating period; update the "desc"
value to add a period at the end (i.e., "Max ammo on all weapons reduced by
75%.") so it matches punctuation style used elsewhere.
|
🎉 This PR is included in version 3.1.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you fix?
Added risk strings for both existing archimedeas, calendar challenge, and current prime vault items
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit