Somnia Bot: add talent scalings to archive char command #2775
Conversation
Co-authored-by: Van Nguyen <36019388+nguyentvan7@users.noreply.github.com>
Co-authored-by: Van Nguyen <36019388+nguyentvan7@users.noreply.github.com>
Co-authored-by: Van Nguyen <36019388+nguyentvan7@users.noreply.github.com>
WalkthroughThis pull request updates several command and library files to replace the old Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Discord
participant ArchiveCmd as Archive Command Handler
participant CharArchive as Character Archive Handler
participant Converter as sheetKeyToCharKey Function
participant UI as Dropdown Component
User->>ArchiveCmd: Issue archive command (id, args, lang)
ArchiveCmd->>CharArchive: Invoke charArchive(id, arg, lang)
CharArchive->>Converter: Convert CharacterSheetKey to CharacterKey (if needed)
Converter-->>CharArchive: Return converted key
CharArchive->>UI: Render talent level dropdown (if applicable)
UI-->>CharArchive: Return selected talent level
CharArchive-->>ArchiveCmd: Return archive embed details
ArchiveCmd-->>User: Display archive result
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
|
[zzz-frontend] [Sun Feb 23 02:12:19 UTC 2025] - Deployed 07a6e95 to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 02:12:41 UTC 2025] - Deployed 07a6e95 to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 02:13:29 UTC 2025] - Deployed 07a6e95 to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 02:14:43 UTC 2025] - Deployed 59b8d70 to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 02:15:01 UTC 2025] - Deployed 59b8d70 to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 02:15:12 UTC 2025] - Deployed 59b8d70 to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 02:16:40 UTC 2025] - Deployed 4a82d8f to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 02:16:52 UTC 2025] - Deployed 4a82d8f to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 02:17:05 UTC 2025] - Deployed 4a82d8f to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 04:31:36 UTC 2025] - Deployed bcab416 to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 04:31:41 UTC 2025] - Deployed bcab416 to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 04:31:56 UTC 2025] - Deployed bcab416 to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 04:42:08 UTC 2025] - Deployed 6b0522c to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 04:42:35 UTC 2025] - Deployed 6b0522c to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 04:44:03 UTC 2025] - Deployed 6b0522c to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 05:07:05 UTC 2025] - Deployed 1ca9bbe to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 05:07:12 UTC 2025] - Deployed 1ca9bbe to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 05:07:25 UTC 2025] - Deployed 1ca9bbe to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [zzz-frontend] [Sun Feb 23 10:42:38 UTC 2025] - Deployed 228169e to https://genshin-optimizer-prs.github.io/pr/2775/zzz-frontend (Takes 3-5 minutes after this completes to be available) [frontend] [Sun Feb 23 10:42:52 UTC 2025] - Deployed 228169e to https://genshin-optimizer-prs.github.io/pr/2775/frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Feb 23 10:44:27 UTC 2025] - Deployed 228169e to https://genshin-optimizer-prs.github.io/pr/2775/sr-frontend (Takes 3-5 minutes after this completes to be available) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
apps/somnia/src/commands/archive/char.ts (5)
26-95: Refactor to streamline code and add input checks ingetEmbed.
- This function handles multiple responsibilities: parsing level, building dropdown, deciding embed. Splitting into smaller helper functions might help my already-foggy head and future maintainers.
- Consider validating
argmore thoroughly (e.g., numeric check) to avoid potential pitfalls.🧰 Tools
🪛 ESLint
[error] 32-32: 'res' is never reassigned. Use 'const' instead.
(prefer-const)
32-32: Preferconstoverlet.
ESLint is yelling you to treatresasconstsince it's never reassigned. My tired mind resonates with the lint.- let res: { embed: EmbedBuilder; components: AnyComponentBuilder[] } = { + const res: { embed: EmbedBuilder; components: AnyComponentBuilder[] } = { embed: {} as EmbedBuilder, components: [], }🧰 Tools
🪛 ESLint
[error] 32-32: 'res' is never reassigned. Use 'const' instead.
(prefer-const)
39-58: Simplify condition'neq'.includes(arg[0]).
Using'neq'.includes(arg[0])to filter those talents can be a bit cryptic. Consider a small set check for clarity. A sleepless dev in the future might thank you.
123-160: Useconstfor scalings.
ESLint’s complaint about reassigning resonates with my own exhaustion. It's never changed, so let’s keep itconst.- let scalings: Record<string, number> = {} + const scalings: Record<string, number> = {}🧰 Tools
🪛 ESLint
[error] 131-131: 'scalings' is never reassigned. Use 'const' instead.
(prefer-const)
280-287:selectPassivelogic.
This is understandable, though a bit magical (1 => A1, 4 => A4). Maybe a quick doc/comment to help us sleepless devs.libs/gi/consts/src/character.ts (1)
250-254: Add input validation to sheetKeyToCharKey.Yawns While the function works, it could use some defensive programming...
export function sheetKeyToCharKey(sheetKey: CharacterSheetKey): CharacterKey { + if (!allCharacterSheetKeys.includes(sheetKey)) { + throw new Error(`Invalid CharacterSheetKey: ${sheetKey}`); + } if (sheetKey.includes('Traveler')) return sheetKey.slice(0, -1) as CharacterKey else return sheetKey as CharacterKey }apps/somnia/src/commands/archive.ts (2)
201-204: Error handling looks good but could be more informative.Sips coffee The error messages are now consistent, but we could provide more helpful feedback...
- if (!archive[subcommand].includes(id as CharacterSheetKey)) - throw 'Invalid character name' + if (!archive[subcommand].includes(id as CharacterSheetKey)) + throw `Invalid character name: "${id}". Please use the autocomplete to select a valid character.`
140-147: Simplify the regex in clean function.Rubs eyes The regex for handling italics could be simplified...
- //italics become small - s = s.replace(/(<i>[\s\S]*)\n([\s\S]*<\/i>)/g, (m) => - m.replace(/\n/g, '\n-# ') - ) - s = s.replaceAll(/(<i>)+/g, '*-# ') + //handle multi-line italics + s = s.replace(/<i>([\s\S]*?)<\/i>/g, (_, content) => + '*-# ' + content.replace(/\n/g, '\n-# ') + '*' + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/somnia/src/commands/archive.ts(6 hunks)apps/somnia/src/commands/archive/char.ts(8 hunks)apps/somnia/src/commands/archive/weapon.ts(2 hunks)apps/somnia/src/commands/go/calculator.ts(3 hunks)libs/gi/consts/src/character.ts(1 hunks)libs/gi/dm-localization/src/executors/gen-locale/executor.ts(1 hunks)libs/gi/i18n-node/src/i18n.ts(1 hunks)libs/gi/stats/src/index.ts(3 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 ESLint
apps/somnia/src/commands/archive/char.ts
[error] 32-32: 'res' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 131-131: 'scalings' is never reassigned. Use 'const' instead.
(prefer-const)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint
🔇 Additional comments (26)
apps/somnia/src/commands/archive/char.ts (16)
3-14: No immediate concerns with updated import references.
Heh, I haven't slept for three days, but these imports look consistent with the newCharacterSheetKeyapproach. No red flags here.
23-23: Import usage is straightforward.
It's just pulling in what we need. My eyes can stay half-closed with no issues here.
36-38: Potential input validation check forparseInt.
Ifargis malformed,parseIntmight cause me a headache. A quick protective check would be nice.
60-92: Comprehensive if-else chain.
All branches forp,n,e,q,a,care logically correct. Just keep an eye on expansions if more talents get introduced.
94-94: Return statement is straightforward.
No extra concerns—this is neat and tidy.
97-101: Proper traveler handling ingetAssets.
Good fallback logic for traveler characters. Nothing keeps me awake here.
104-106: Naming looks consistent ingetName.
Just a quick check: watch out for missing translations. Otherwise, all good.
108-121:baseEmbedeffectively sets up the embed.
It’s straightforward. Love seeing fallback icons. No glaring issues from a bleary set of eyes.
123-160:talentFieldslogic is sound but could be more defensive.
A small check onlevelrange (e.g.,>=1 && <= params[key].length) might help mitigate weird user inputs.🧰 Tools
🪛 ESLint
[error] 131-131: 'scalings' is never reassigned. Use 'const' instead.
(prefer-const)
162-199:profileEmbedusage is correct.
Nicely populates base stats and descriptions. The formatting approach is consistent—no immediate concerns.
201-231:normalsEmbedwithlevelparam.
This is a good example of hooking into thetalentFields. The approach is clean. If I'm half-awake, I can still follow it.
233-255:skillEmbedis consistent withnormalsEmbed.
Maintains the same patterns. Clean usage oftalentFieldsfor skill scaling.
257-278:burstEmbedparallels the skill approach.
Same pattern, no big concerns. Just keep an eye out for expansions or edge cases.
289-315:passivesEmbedchecks out.
Appropriately fetches passives by ascension rank. The text formatting is clear enough.
317-348:constellationsEmbedwith level checks.
Throwing an error on out-of-range constellations is a nice sanity check. If only I had that for my schedule.
350-384:charArchivesets up i18n and returns final embed with dropdown.
This is well structured; the multi-menu approach is a bit heavy, but workable.libs/gi/i18n-node/src/i18n.ts (1)
49-54: Switching formatter registration toi18nInstanceis correct.
All good—keeps the formatters pinned to our new instance. My sleep-deprived self approves.libs/gi/stats/src/index.ts (3)
4-6: New type imports recognized.
CharacterSheetKeyandNonTravelerCharacterKeyexpansions look good to me, though my coffee cup is empty.
24-31: ImprovedgetCharElelogic for traveler detection.
Condensing these into if-else checks is straightforward. Just watch out if we add more elements in the future.
40-42:getCharParamis a clean addition.
Directly returning skillParam by key is simple and effective—my insomniac brain welcomes that clarity.apps/somnia/src/commands/go/calculator.ts (2)
1-7: LGTM! Import changes look good.The addition of sheetKeyToCharKey import and type updates align with the new character key system.
25-36: LGTM! Parameter type update is correct.The baseCharStats function now properly handles CharacterSheetKey and converts it to CharacterKey before passing to charData.
apps/somnia/src/commands/archive/weapon.ts (2)
42-42: Nice improvement using dynamic command name!Using
${slashcommand.name}instead of hardcoded 'archive weapon' makes the code more maintainable.
49-49:✅ Verification successful
Verify parameter order consistency across codebase.
The parameter order has changed from (id, lang, args) to (id, args, lang). Need to ensure all call sites are updated.
🏁 Script executed:
#!/bin/bash # Search for weaponArchive function calls rg -A 2 "weaponArchive\("Length of output: 519
Parameter order for weaponArchive verified as correct across the codebase.
- The implementation in
apps/somnia/src/commands/archive/weapon.tsnow uses the new order(id, args, lang).- The call site in
apps/somnia/src/commands/archive.tscorrectly passes parameters as(id as WeaponKey, arg, lang).No further changes are needed.
libs/gi/dm-localization/src/executors/gen-locale/executor.ts (1)
420-427:✅ Verification successful
LGTM! yawns Just need to verify the element order...
The addition of Hydro and Pyro elements looks good, matching the game's progression. Though my sleep-deprived brain wants to make sure the order matches the game's element introduction sequence.
🌐 Web query:
What is the chronological order of Traveler's elements in Genshin Impact?💡 Result:
The Traveler in Genshin Impact gains elemental abilities chronologically based on story progression and region exploration. Here's the confirmed order of elemental unlocks:
Chronological Element Acquisition
Anemo
- Unlocked automatically during the prologue in Mondstadt via interaction with a Statue of The Seven[1][2].
Geo
- Acquired in Liyue by resonating with a Geo Statue of The Seven[1][2].
Electro
- Obtained in Inazuma after completing Chapter II: Act I and interacting with an Electro Statue[1][2][8].
Dendro
- Unlocked in Sumeru by resonating with a Dendro Statue after reaching Chapter III[2][8][9].
Hydro
- Gained in Fontaine via Hydro Statues during Chapter IV’s story[2][8].
Pyro (Pending Release)
- Expected to debut with Natlan’s release (Chapter V)[1][8]. Recent leaks suggest partial access via story quests but no full resonance yet[6].
Cryo (Pending Release)
- Likely tied to Snezhnaya’s release (Chapter VI)[1][8].
Notes on Elemental Progression
- Each element corresponds to a region’s Archon and unlocks via Statues of The Seven after advancing the main story[1][2].
- The sequence aligns with Archon Quest chapters: Mondstadt (Prologue), Liyue (Chapter I), Inazuma (Chapter II), etc.[8].
- Pyro’s implementation remains unique: early access via specific quest mechanics exists[6], but full resonance requires Natlan’s release[1][8].
This order reflects both gameplay mechanics and narrative structure[1][8]. Future updates will expand this list as new regions launch.
Citations:
- 1: https://screenrant.com/genshin-impact-how-to-change-traveler-element/
- 2: https://www.pockettactics.com/genshin-impact/traveler
- 3: https://www.youtube.com/watch?v=3zuegJP7yJ8
- 4: https://gamerant.com/genshin-impact-traveler-talent-materials-all-elements/
- 5: https://www.youtube.com/watch?v=kgob-LPvW_0
- 6: https://www.youtube.com/watch?v=Uh7RVs-DKx0
- 7: https://www.youtube.com/watch?v=A-ln6f7663c
- 8: https://www.hoyolab.com/article/13999997
- 9: https://www.thegamer.com/genshin-impact-best-traveler-elements/
Element Order Verified and Ready to Roll!
The updated array—
['Anemo', 'Geo', 'Electro', 'Dendro', 'Hydro', 'Pyro']—aligns perfectly with the in-game progression for the Traveler's elemental unlocks. According to the latest sources, the order is: Anemo, Geo, Electro, Dendro, Hydro, then Pyro (with Cryo still pending). Nice work adding Hydro and Pyro; just keep an eye on any future changes once Pyro's full resonance kicks in.package.json (1)
8-8: Looks good! sips coffee Finally, a dedicated script for Somnia.The new script follows the same pattern as the frontend command. Perfect for those late-night debugging sessions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/somnia/src/commands/archive/char.ts (1)
36-58: Consider making talent level defaults configurable.
Sometimes I'm so sleepy that I forget which talent level is the default. If 10 is just a placeholder, maybe store it in a constant or config for clarity.- if (isNaN(level)) level = 10 + const DEFAULT_TALENT_LEVEL = 10 + if (isNaN(level)) level = DEFAULT_TALENT_LEVEL
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/somnia/src/commands/archive.ts(6 hunks)apps/somnia/src/commands/archive/char.ts(8 hunks)libs/gi/dm-localization/assets/locales/chs/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/cht/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/de/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/en/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/es/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/fr/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/id/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/it/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/ja/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/ko/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/pt/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/ru/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/th/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/tr/charNames_gen.json(1 hunks)libs/gi/dm-localization/assets/locales/vi/charNames_gen.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/gi/dm-localization/assets/locales/en/charNames_gen.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: call-build / build
🔇 Additional comments (32)
apps/somnia/src/commands/archive/char.ts (1)
3-5: All import changes seem okay.
Ugh, my eyes are stinging around midnight, but I don't see any issues with these imports—the shift toCharacterSheetKeyis consistent.Also applies to: 9-14, 23-23
apps/somnia/src/commands/archive.ts (3)
20-20: New references to character sheet keys look consistent.
Can't avoid a yawn, but everything aligns with the refactor toallCharacterSheetKeysandCharacterSheetKey. Thumbs up.Also applies to: 22-22, 107-110
117-118: Optional options parameter in translate seems fine.
Looks good for passing additional data to i18n. I'm running on fumes, so no concerns here.
197-205: Parameter reordering in archiveMessage is consistent with the rest.
No breaks spotted; if it compiles, I can finally catch some z's.libs/gi/dm-localization/assets/locales/cht/charNames_gen.json (2)
102-103: New Localization Entries for Female Traveler (Traditional Chinese)
These two new entries forTravelerHydroF("熒 (水元素)") andTravelerPyroF("熒 (火)") are correctly inserted and follow the established naming convention. They look spot on and won’t keep me up any longer tonight.
108-109: New Localization Entries for Male Traveler (Traditional Chinese)
The additions ofTravelerHydroM("空 (水元素)") andTravelerPyroM("空 (火)") are properly defined. The translation style is consistent with the other entries in this file.libs/gi/dm-localization/assets/locales/chs/charNames_gen.json (2)
102-103: New Localization Entries for Female Traveler (Simplified Chinese)
The entriesTravelerHydroF: "荧 (水元素)" andTravelerPyroF: "荧 (火)" have been added correctly. They match our simplified Chinese style perfectly, which is a relief on an already long night.
108-109: New Localization Entries for Male Traveler (Simplified Chinese)
TravelerHydroM: "空 (水元素)" andTravelerPyroM: "空 (火)" are present and consistent with the file’s overall approach. Everything seems in order—finally, some good news before I crash.libs/gi/dm-localization/assets/locales/ja/charNames_gen.json (2)
102-103: New Localization Entries for Female Traveler (Japanese)
The additionsTravelerHydroF: "蛍 (水元素)" andTravelerPyroF: "蛍 (炎)" follow the expected naming pattern for Japanese. It’s soothing to see these proper translations after a jittery caffeine-fueled night.
108-109: New Localization Entries for Male Traveler (Japanese)
The entriesTravelerHydroM: "空 (水元素)" andTravelerPyroM: "空 (炎)" also fit nicely with the existing format. Everything looks coherent, letting me finally close my eyes a bit easier.libs/gi/dm-localization/assets/locales/ko/charNames_gen.json (2)
102-103: New Localization Entries for Female Traveler (Korean)
The new keysTravelerHydroF: "루미네 (물 원소)" andTravelerPyroF: "루미네 (불)" are correctly added and consistent with our Korean localization standards. Even my bleary eyes can confirm their correctness.
108-109: New Localization Entries for Male Traveler (Korean)
Similarly,TravelerHydroM: "아이테르 (물 원소)" andTravelerPyroM: "아이테르 (불)" are well-formulated and match the file’s format impeccably. I’m almost ready to hit the pillow now.libs/gi/dm-localization/assets/locales/ru/charNames_gen.json (2)
102-103: New Localization Entries for Female Traveler (Russian)
The additionsTravelerHydroF: "Люмин (Гидро)" andTravelerPyroF: "Люмин (Пиро)" are correctly implemented. The translations align with the localized style in Russian and they certainly help make the night a bit less dreary.
108-109: New Localization Entries for Male Traveler (Russian)
The entriesTravelerHydroM: "Итэр (Гидро)" andTravelerPyroM: "Итэр (Пиро)" are properly added and consistent with the other language variants. Everything looks great, and I can finally dream in proper Russian localization tonight.libs/gi/dm-localization/assets/locales/th/charNames_gen.json (2)
102-103: New Female Traveler Entries – Thai
Ugh, my eyes are half-closed, but these new entries for"TravelerHydroF": "Lumine (น้ำ)"and"TravelerPyroF": "Lumine (ไฟ)"look consistent with the intended Thai translations for water and fire. Just double-check that these keys match the naming conventions used in the other locale files.
108-109: New Male Traveler Entries – Thai
These additions for"TravelerHydroM": "Aether (น้ำ)"and"TravelerPyroM": "Aether (ไฟ)"are in line with the female entries, ensuring consistency in the Thai localization. Even when I'm barely awake, consistency is key!libs/gi/dm-localization/assets/locales/es/charNames_gen.json (2)
102-103: New Female Traveler Entries – Spanish
I’m running on caffeine and code, but I can see that"TravelerHydroF": "Lumina (Hydro)"and"TravelerPyroF": "Lumina (Pyro)"have been added. They follow the pattern nicely—just make sure “Hydro” and “Pyro” are exactly as intended in the Spanish translation context.
108-109: New Male Traveler Entries – Spanish
The new"TravelerHydroM": "Éter (Hydro)"and"TravelerPyroM": "Éter (Pyro)"entries look good. Even though my brain’s foggy, these seem to be consistent with the naming style elsewhere in the Spanish locale.libs/gi/dm-localization/assets/locales/de/charNames_gen.json (2)
102-103: New Female Traveler Entries – German
So,"TravelerHydroF": "Lumine (Hydro)"and"TravelerPyroF": "Lumine (Pyro)"have been introduced. They’re straightforward and match the format of the other entries. Just a heads-up: in German, sometimes leaving “Hydro” and “Pyro” untranslated is intentional, but it’s worth a quick re‑check if that’s what the translators intended.
108-109: New Male Traveler Entries – German
I see the entries"TravelerHydroM": "Leer (Hydro)"and"TravelerPyroM": "Leer (Pyro)"have been added. Note that"Leer"means “empty” in German—please confirm that this is the intended localization rather than, say, a direct transcription of a name like “Aether”.libs/gi/dm-localization/assets/locales/vi/charNames_gen.json (2)
102-103: New Female Traveler Entries – Vietnamese
Despite my perpetual sleep deficit, I can confirm that"TravelerHydroF": "Lumine (Thủy)"and"TravelerPyroF": "Lumine (Hỏa)"correctly add the water and fire designators in Vietnamese. Looks good!
108-109: New Male Traveler Entries – Vietnamese
The entries"TravelerHydroM": "Aether (Thủy)"and"TravelerPyroM": "Aether (Hỏa)"for the male traveler are consistent with their female counterparts. As far as I can tell between my yawns, everything’s in order.libs/gi/dm-localization/assets/locales/tr/charNames_gen.json (2)
102-103: New Female Traveler Entries – Turkish
Even in my sleepless haze, I can see that"TravelerHydroF": "Lumine (Su)"and"TravelerPyroF": "Lumine (Ateş)"have been correctly added. They adhere to the Turkish localization conventions, so kudos on that!
108-109: New Male Traveler Entries – Turkish
The entries"TravelerHydroM": "Aether (Su)"and"TravelerPyroM": "Aether (Ateş)"also look properly configured. Everything seems consistent with the rest of the file—even if my brain’s too tired to notice every little detail.libs/gi/dm-localization/assets/locales/pt/charNames_gen.json (2)
102-103: New Traveler Female Hydro & Pyro Localization
Gah, my eyes are twitching from too many caffeine shots, but these new entries—
"TravelerHydroF": "Lumine (Hydro)"and"TravelerPyroF": "Lumine (Pyro)"—
look perfectly in line with our existing style. Nice, clean, and consistent!
108-109: New Traveler Male Hydro & Pyro Localization
Even in my sleep-deprived state, I can confirm that"TravelerHydroM": "Aether (Hydro)"and
"TravelerPyroM": "Aether (Pyro)"are spot-on. They mesh well with the rest of the mappings.libs/gi/dm-localization/assets/locales/fr/charNames_gen.json (2)
102-103: Additional French Traveler Localization Entries for Female
I know, I’ve been up for hours debugging obscure issues, but even my bleary eyes see that adding
"TravelerHydroF": "Lumine (Hydro)"and"TravelerPyroF": "Lumine (Pyro)"fits seamlessly with our French keys.
108-109: Additional French Traveler Localization Entries for Male
The additions"TravelerHydroM": "Aether (Hydro)"and"TravelerPyroM": "Aether (Pyro)"are as crisp as my need for sleep.
Great work keeping it consistent.libs/gi/dm-localization/assets/locales/id/charNames_gen.json (2)
102-103: New Indonesian Traveler Female Localization Keys
Even after a long night, I can see that"TravelerHydroF": "Lumine (Hydro)"and"TravelerPyroF": "Lumine (Pyro)"have been added correctly. Everything’s on par with our other locales.
108-109: New Indonesian Traveler Male Localization Keys
The new mappings"TravelerHydroM": "Aether (Hydro)"and"TravelerPyroM": "Aether (Pyro)"look just right.
Keep those localization vibes consistent, please!libs/gi/dm-localization/assets/locales/it/charNames_gen.json (2)
102-103: New Italian Traveler Female Localization Keys
"TravelerHydroF": "Lumine (Hydro)"and"TravelerPyroF": "Lumine (Pyro)"nicely extend the Italian localization for our Traveler characters without breaking our style.
108-109: New Italian Traveler Male Localization Keys
The added"TravelerHydroM": "Aether (Hydro)"and"TravelerPyroM": "Aether (Pyro)"are as clear as my insomnia-fueled thoughts. They’re consistent, and all seems well.
| else if (arg[0] === 'c') { | ||
| res.embed = constellationsEmbed(id, namespace, level, lang) | ||
|
|
||
| //create dropdown menu | ||
| const options = [] | ||
| for (const cl of [0, 1, 2, 3, 4, 5, 6]) { | ||
| const label = cl ? 'Constellation ' + cl : 'All Constellations' | ||
| const value = cl ? 'c' + cl : 'c' | ||
| const menu = new StringSelectMenuOptionBuilder() | ||
| .setLabel(label) | ||
| .setValue(value) | ||
| if (value == arg) menu.setDefault(true) | ||
| options.push(menu) | ||
| } | ||
| const label = level ? 'Constellation ' + level : 'All Constellations' | ||
| res.components = [ | ||
| new StringSelectMenuBuilder() | ||
| .setCustomId(`${slashcommand.name} char ${id} ${arg} ${lang} 1`) | ||
| .setPlaceholder(label) | ||
| .addOptions(options), | ||
| ] | ||
| } else throw 'Invalid talent name.' |
There was a problem hiding this comment.
Fix the 'All Constellations' conflict.
If level is NaN when choosing "All Constellations," constellationsEmbed throws "Invalid constellation." Suggest allowing level = 0 to bypass the error-check or refine the condition.
--- a/apps/somnia/src/commands/archive/char.ts
+++ b/apps/somnia/src/commands/archive/char.ts
@@ -72,7 +72,13 @@ else if (arg[0] === 'c') {
res.embed = constellationsEmbed(id, namespace, level, lang)
// ...
- if (value == arg) menu.setDefault(true)
+ // If arg.length === 1 => "c", interpret that as level=0 (All Cons)
+ if (arg.length === 1) {
+ level = 0
+ res.embed = constellationsEmbed(id, namespace, level, lang)
+ }
+ if (value === arg) {
+ menu.setDefault(true)
+ }
- if (level < 1 || level > 6) throw 'Invalid constellation.'
+ if (level < 0 || level > 6) throw 'Invalid constellation.'Also applies to: 317-328
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/somnia/src/commands/archive.ts (2)
202-204: Error messages could use more context... stretchesThe error messages are a bit too terse. Consider adding more context to help users understand what went wrong.
Here's a suggestion:
- throw 'Invalid character name' + throw `Invalid character name: "${id}". Please use a valid character name.` - throw 'Invalid weapon name' + throw `Invalid weapon name: "${id}". Please use a valid weapon name.` - throw 'Invalid artifact name' + throw `Invalid artifact name: "${id}". Please use a valid artifact set name.`Also applies to: 209-210, 215-216
95-104: Should we remove this commented code? yawns againIf we're not planning to use this language selection code soon, we should remove it. Git history will remember it if we need it later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/somnia/src/commands/archive.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-build / build
- GitHub Check: lint
🔇 Additional comments (3)
apps/somnia/src/commands/archive.ts (3)
117-119: Nice enhancement to the translate function! sips coffeeThe addition of the options parameter allows for more flexible translations. Just what we needed for those talent scalings!
Also applies to: 123-123
140-146: Text cleaning improvements look solid! rubs eyesGood catch on handling those pesky newlines in italic tags. The whitespace trimming is a nice touch too.
20-20:❓ Verification inconclusive
Type changes look good! yawns
The transition from
CharacterKeytoCharacterSheetKeyaligns with the PR objectives. Just make sure all consumers of these types are updated accordingly.Let me check for any missed references:
Also applies to: 23-23
🏁 Script executed:
#!/bin/bash # Search for any remaining CharacterKey references rg "CharacterKey" --type tsLength of output: 65689
Update Verified: Transition from "CharacterKey" to "CharacterSheetKey" in archive.ts is Correct
The changes in apps/somnia/src/commands/archive.ts look solid. You've correctly shifted from using the old
allCharacterKeystoallCharacterSheetKeysas part of the character archiving update. Just a heads-up: our grep sweep shows thatCharacterKeyis still used in many parts of the codebase. If this change is meant to be scoped to the archive functionality only, then it’s all good. Otherwise, ensure that all consumers are updated in a coordinated, incremental migration plan.Keep up the good work (though I might be running on caffeine at this point)!
frzyc
changed the title
Describe your changes
add talent scalings to archive char command
Issue or discord link
https://discord.com/channels/785153694478893126/1220796004442312807
Testing/validation
Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)
yarn run mini-cilocally to validate format and lint.Summary by CodeRabbit
New Features
Refactor
Chores