Skip to content
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

Add Lan Yan #2617

Merged
merged 4 commits into from
Jan 25, 2025
Merged

Add Lan Yan #2617

merged 4 commits into from
Jan 25, 2025

Conversation

nguyentvan7
Copy link
Collaborator

@nguyentvan7 nguyentvan7 commented Jan 25, 2025

Describe your changes

Issue or discord link

Testing/validation

Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)

  • I have commented my code in hard-to understand areas.
  • I have made corresponding changes to README or wiki.
  • For front-end changes, I have updated the corresponding English translations.
  • I have run yarn run mini-ci locally to validate format and lint.
  • If I have added a new library or app, I have updated the deployment scripts to ignore changes as needed

Summary by CodeRabbit

Release Notes: New Character - Lan Yan

  • New Features

    • Introduced a new playable character, Lan Yan, a 4-star Anemo Catalyst from Liyue.
    • Added character details including skills, talents, and constellation abilities.
    • Implemented localization support across multiple languages.
  • Localization

    • Added character name and ability descriptions for Chinese, English, Japanese, Korean, and other supported languages.
  • Character Details

    • Elemental Type: Anemo
    • Weapon Type: Catalyst
    • Rarity: 4-star
    • Region: Liyue
    • Birthday: January 6th

@nguyentvan7 nguyentvan7 requested a review from frzyc January 25, 2025 17:45
Copy link
Contributor

coderabbitai bot commented Jan 25, 2025

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

/usr/local/bin/yarn: 3: dirname: not found
/usr/local/bin/yarn: 7: dirname: not found
node:internal/modules/cjs/loader:1251
throw err;
^

Error: Cannot find module '/yarn.js'
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:166:5)
at node:internal/main/run_main_module:30:49 {
code: 'MODULE_NOT_FOUND',
requireStack: []
}

Node.js v22.9.0

Walkthrough

A new character named "LanYan" has been comprehensively added to the game's ecosystem. This addition spans multiple libraries and localization assets, introducing a 4-star Anemo Catalyst character from the Liyue region. The implementation includes detailed character data such as stats, skills, constellations, and localization across multiple languages. The character's integration involves updating various configuration files, character sheets, and statistical data to fully incorporate her into the game's character roster.

Changes

File Path Change Summary
libs/gi/assets-data/src/AssetsData_gen.json Added new character entry for "LanYan" with icon and asset references
libs/gi/char-cards/src/index.ts Imported character card image and updated charCards object
libs/gi/consts/src/character.ts Added "LanYan" to nonTravelerCharacterKeys array
libs/gi/dm/src/mapping/character.ts Added character ID mapping for LanYan
libs/gi/mats/src/allCharacterMats_gen.json Added talent and ascension materials for LanYan
libs/gi/sheets/src/Characters/ Created new character sheet for LanYan
libs/gi/stats/Data/Characters/LanYan/ Added character data and skill parameter JSON files
libs/gi/stats/src/allStat_gen.json Integrated LanYan's statistical profile
Localization Files Added character name and detailed localization entries across multiple languages (CHS, CHT, DE, EN, ES, FR, ID, IT, JA, KO, PT, RU, TH, TR, VI)

Sequence Diagram

sequenceDiagram
    participant Game
    participant CharacterSystem
    participant LocalizationSystem
    participant StatsSystem
    
    Game->>CharacterSystem: Request Character Details
    CharacterSystem->>LocalizationSystem: Fetch Localized Name
    CharacterSystem->>StatsSystem: Retrieve Character Stats
    StatsSystem-->>CharacterSystem: Return Base Stats
    LocalizationSystem-->>CharacterSystem: Return Localized Name
    CharacterSystem->>Game: Provide Complete Character Profile
Loading

Poem

🐰 A Rabbit's Ode to Lan Yan 🍃

From Liyue's misty vale she springs,
With Anemo's grace, her catalyst sings!
Swallow-wisp dance, a moonlit art,
A new hero to steal my rabbit heart!
Welcome, Lan Yan, your journey's just begun! 🌙


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (12)
libs/gi/sheets/src/Characters/LanYan/index.tsx (3)

29-79: Consider using explicit indices instead of counter variables.

The current implementation uses counter variables (a, s, b) to index into skillParam arrays. This approach is error-prone as it relies on the exact order of parameters and makes the code harder to maintain.

Consider using explicit indices for better maintainability:

-let a = 0, s = 0, b = 0
 const dm = {
   normal: {
     hitArr: [
-      skillParam_gen.auto[a++], // 1
-      skillParam_gen.auto[a++], // 2.1
-      skillParam_gen.auto[a++], // 2.2
+      skillParam_gen.auto[0], // 1
+      skillParam_gen.auto[1], // 2.1
+      skillParam_gen.auto[2], // 2.2
       // ... rest of the indices
     ],
   },
   // Update other sections similarly
 }

88-97: Reduce code duplication in A4 passive implementation.

The A4 passive calculations for skill and burst damage increases use identical logic with different multipliers.

Consider creating a utility function:

+const createA4DmgInc = (multiplier: number) =>
+  greaterEq(
+    input.asc,
+    4,
+    prod(input.total.eleMas, percent(multiplier))
+  )
+
-const a4_skill_dmgInc = greaterEq(
-  input.asc,
-  4,
-  prod(input.total.eleMas, percent(dm.passive2.skill_dmgInc))
-)
-const a4_burst_dmgInc = greaterEq(
-  input.asc,
-  4,
-  prod(input.total.eleMas, percent(dm.passive2.burst_dmgInc))
-)
+const a4_skill_dmgInc = createA4DmgInc(dm.passive2.skill_dmgInc)
+const a4_burst_dmgInc = createA4DmgInc(dm.passive2.burst_dmgInc)

162-177: Consider using constants for magic numbers in auto attack helpers.

The functions use magic numbers which could make maintenance difficult.

Consider using constants:

+const MULTI_HIT_START_INDEX = 3
+const FIRST_HIT_INDICES = [1, 3]
+const SECOND_HIT_INDICES = [2, 4]

 function autoIndex(index: number) {
-  if (index > 3) {
+  if (index > MULTI_HIT_START_INDEX) {
     return index - 2
   } else if (index > 1) {
     return index - 1
   }
   return index
 }
 function autoSuffix(index: number) {
-  if (index === 1 || index === 3) {
+  if (FIRST_HIT_INDICES.includes(index)) {
     return '(1)'
-  } else if (index === 2 || index === 4) {
+  } else if (SECOND_HIT_INDICES.includes(index)) {
     return '(2)'
   }
   return undefined
 }
libs/gi/stats/src/allStat_gen.json (1)

51459-51523: LGTM! Character profile is well-defined.

The base stats, growth curves, and ascension bonuses are appropriately balanced for a 4-star Anemo Catalyst user. Consider adding comments to document the significance of the growth curve types (e.g., "GROW_CURVE_HP_S4") for future reference.

libs/gi/dm-localization/assets/locales/en/char_LanYan_gen.json (3)

10-11: Maintain consistent HTML tag style in ability descriptions.

Consider standardizing the HTML tag format. Some descriptions use a period at the end while others don't.

Apply this diff for consistency:

-        "1": "Wields her Feathermoon Rings to perform up to 4 attacks, dealing <anemo>Anemo DMG</anemo>."
+        "1": "Wields her Feathermoon Rings to perform up to 4 attacks, dealing <anemo>Anemo DMG</anemo>"

Also applies to: 14-15, 18-20


31-38: Clean up empty parameter fields.

Consider removing unused parameter fields (8-15) to improve maintainability.

Also applies to: 59-70, 84-96


122-123: Enhance prerequisite visibility in constellation descriptions.

Consider making the prerequisite more visible by moving it before the effect description or highlighting it.

Apply this diff:

-      "0": "After triggering the Elemental Absorption from the Passive Talent \"Four Sealing Divination Charms,\" this instance of Lan Yan's Elemental Skill <strong>Swallow-Wisp Pinion Dance</strong> will produce another Feathermoon Ring when they are thrown at opponents.",
-      "1": "You must first unlock the Passive Talent \"Four Sealing Divination Charms.\""
+      "0": "<strong>Prerequisite:</strong> Passive Talent \"Four Sealing Divination Charms\" must be unlocked.",
+      "1": "After triggering the Elemental Absorption from the Passive Talent \"Four Sealing Divination Charms,\" this instance of Lan Yan's Elemental Skill <strong>Swallow-Wisp Pinion Dance</strong> will produce another Feathermoon Ring when they are thrown at opponents."
libs/gi/dm-localization/assets/locales/th/char_LanYan_gen.json (1)

42-43: Consider localizing skill names in Thai.

Some skill names are left in English while others are translated. Consider maintaining consistency by translating all skill names to Thai.

Also applies to: 74-75

libs/gi/dm-localization/assets/locales/ru/char_LanYan_gen.json (1)

10-11: Fix inconsistent punctuation in Russian ability descriptions.

The Russian translation adds a colon after "Обычная атака" while other languages don't use this punctuation.

Apply this diff:

-        "0": "<strong>Обычная атака:</strong>",
+        "0": "<strong>Обычная атака</strong>"
libs/gi/dm-localization/assets/locales/it/char_LanYan_gen.json (3)

22-39: Consider removing unused skill parameters.

The skillParams array contains several empty strings at the end (indexes 8-15). Consider removing these unused parameters to maintain a cleaner configuration file.

     "skillParams": {
       "0": "DAN da 1º colpo",
       "1": "DAN da 2º colpo",
       "2": "DAN da 3º colpo",
       "3": "DAN da 4º colpo",
       "4": "DAN da ATT caricato",
       "5": "Costo vigore ATT caricato",
       "6": "DAN da ATT in picchiata",
       "7": "DAN da ATT in picchiata basso/alto"
-      "8": "",
-      "9": "",
-      "10": "",
-      "11": "",
-      "12": "",
-      "13": "",
-      "14": "",
-      "15": ""
     }

54-71: Consider removing unused skill parameters.

Similar to the auto attack parameters, the skill parameters contain unused entries that could be removed.

     "skillParams": {
       "0": "DAN da Anello Lunapiumata",
       "1": "Assorbimento DAN scudo",
       "2": "Durata scudo",
       "3": "TdR"
-      "4": "",
-      "5": "",
-      "6": "",
-      "7": "",
-      "8": "",
-      "9": "",
-      "10": "",
-      "11": "",
-      "12": "",
-      "13": "",
-      "14": "",
-      "15": ""
     }

80-97: Consider removing unused burst parameters.

The burst parameters also contain unused entries that could be removed for better maintainability.

     "skillParams": {
       "0": "DAN da abilità",
       "1": "TdR",
       "2": "Costo energia"
-      "3": "",
-      "4": "",
-      "5": "",
-      "6": "",
-      "7": "",
-      "8": "",
-      "9": "",
-      "10": "",
-      "11": "",
-      "12": "",
-      "13": "",
-      "14": "",
-      "15": ""
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 234d0cd and 68082dc.

⛔ Files ignored due to path filters (18)
  • libs/gi/assets/src/gen/chars/LanYan/Skill_E_Lanyan_01_HD.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/Skill_S_Lanyan_01.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_AvatarIcon_Lanyan.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_AvatarIcon_Side_Lanyan.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_NameCardPic_Lanyan_Alpha.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_NameCardPic_Lanyan_P.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_01.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_02.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_03.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_04.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_05.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Lanyan_06.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_S_Sayu_07.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_U_Lanyan_01.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/UI_Talent_U_Lanyan_02.png is excluded by !**/*.png, !**/gen/**
  • libs/gi/assets/src/gen/chars/LanYan/index.ts is excluded by !**/gen/**
  • libs/gi/assets/src/gen/chars/index.ts is excluded by !**/gen/**
  • libs/gi/char-cards/src/Character_LanYan_Card.jpg is excluded by !**/*.jpg
📒 Files selected for processing (40)
  • libs/gi/assets-data/src/AssetsData_gen.json (1 hunks)
  • libs/gi/char-cards/src/index.ts (2 hunks)
  • libs/gi/consts/src/character.ts (1 hunks)
  • libs/gi/dm-localization/assets/locales/chs/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/chs/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/cht/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/cht/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/de/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/de/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/en/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/en/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/es/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/es/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/fr/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/fr/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/id/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/id/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/it/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/it/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ja/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ja/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ko/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ko/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/pt/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/pt/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ru/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/ru/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/th/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/th/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/tr/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/tr/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/vi/charNames_gen.json (1 hunks)
  • libs/gi/dm-localization/assets/locales/vi/char_LanYan_gen.json (1 hunks)
  • libs/gi/dm/src/mapping/character.ts (1 hunks)
  • libs/gi/mats/src/allCharacterMats_gen.json (1 hunks)
  • libs/gi/sheets/src/Characters/LanYan/index.tsx (1 hunks)
  • libs/gi/sheets/src/Characters/index.ts (2 hunks)
  • libs/gi/stats/Data/Characters/LanYan/data.json (1 hunks)
  • libs/gi/stats/Data/Characters/LanYan/skillParam.json (1 hunks)
  • libs/gi/stats/src/allStat_gen.json (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • libs/gi/stats/Data/Characters/LanYan/data.json
🔇 Additional comments (57)
libs/gi/dm/src/mapping/character.ts (1)

162-162: LGTM! Character ID mapping follows conventions.

The new character ID mapping for LanYan (10000108) follows the established sequential pattern and naming conventions.

libs/gi/sheets/src/Characters/index.ts (1)

57-57: LGTM! Character sheet integration is consistent.

The LanYan character sheet import and registration follow the established patterns and maintain alphabetical ordering.

Also applies to: 165-165

libs/gi/consts/src/character.ts (1)

117-117: LGTM! Character key constant follows conventions.

The addition of 'LanYan' to nonTravelerCharacterKeys maintains alphabetical ordering and follows the naming convention.

libs/gi/char-cards/src/index.ts (1)

49-49: LGTM! Character card integration is consistent.

The LanYan character card import and registration follow the established patterns for image format, naming convention, and maintain alphabetical ordering.

Also applies to: 147-147

libs/gi/sheets/src/Characters/LanYan/index.tsx (6)

1-28: LGTM! Imports and initial setup are well-structured.

The necessary utilities are properly imported and the character setup follows the established pattern.


106-143: LGTM! Damage formulas are comprehensive and well-implemented.

The implementation handles all attack types correctly, including elemental absorption mechanics for passive1.


145-146: LGTM! Constellation talent boosts are correctly implemented.


148-160: LGTM! Data object properly includes all character modifiers and team buffs.


341-341: LGTM! Character sheet is properly exported.


276-280: Verify the burst damage multiplier.

The burst damage is set to multiply by 3 (multi: 3). Please verify if this matches the actual in-game behavior.

libs/gi/mats/src/allCharacterMats_gen.json (5)

60740-61360: LGTM! The overall structure follows the standard pattern.

The data structure for LanYan's material requirements is well-organized and follows the established pattern for character data in the game.


60741-60904: LGTM! Normal attack talent requirements follow the standard progression.

The material and cost progression for normal attack talents follows the expected pattern for a 4-star character:

  • Correct material tier progression (Teachings → Guide → Philosophies)
  • Proper cost scaling
  • Crown of Insight requirement at level 10

60905-61067: LGTM! Skill talent requirements are consistent with normal attack talents.

The material and cost progression for skill talents correctly mirrors the normal attack talent requirements.


61068-61230: LGTM! Burst talent requirements are consistent with other talents.

The material and cost progression for burst talents correctly mirrors the other talent requirements.


61232-61359: LGTM! Ascension requirements follow the standard progression.

The ascension requirements are correctly structured with:

  • Proper Anemo gem progression (Sliver → Fragment → Chunk → Gemstone)
  • Expected material quantities and types
  • Standard cost scaling
libs/gi/stats/src/allStat_gen.json (3)

44518-44551: Verify duplicate scaling values in auto-attack arrays.

Arrays 4 and 5 in the auto-attack sequence have identical scaling values. Please verify if this is intentional or if one of these arrays should have different values.


44742-44794: LGTM! Burst scaling values are well-balanced.

The burst damage multipliers and fixed values are appropriate for a 4-star character.


44808-44809: Verify empty passive and constellation arrays.

Several arrays are empty:

  • passive3
  • constellation3
  • constellation5

Please confirm if these are intentionally empty or if they need to be populated with values.

Also applies to: 44814-44815, 44819-44820

libs/gi/dm-localization/assets/locales/cht/charNames_gen.json (1)

96-96: LGTM! Traditional Chinese localization is accurate.

The translation "藍硯" (Lán Yàn) is correctly implemented using Traditional Chinese characters, maintaining consistency with other Liyue character names.

libs/gi/dm-localization/assets/locales/chs/charNames_gen.json (1)

96-96: LGTM! Simplified Chinese localization is accurate.

The translation "蓝砚" (Lán Yàn) correctly uses Simplified Chinese characters, properly converting from the Traditional form while maintaining the same meaning.

libs/gi/dm-localization/assets/locales/ja/charNames_gen.json (1)

96-96: LGTM! Japanese localization maintains consistency with Liyue character naming conventions.

The translation "藍硯" preserves the Chinese characters, which is consistent with how other Liyue character names are handled in the Japanese localization.

libs/gi/dm-localization/assets/locales/ko/charNames_gen.json (1)

96-96: LGTM! Korean localization follows established conventions.

The translation "남연" (Nam-yeon) correctly uses the Korean readings of the Chinese characters, consistent with the localization pattern for Liyue character names.

libs/gi/dm-localization/assets/locales/ru/charNames_gen.json (1)

96-96: LGTM! Russian transliteration is accurate.

The translation "Лань Янь" correctly follows Russian transliteration rules for Chinese names, maintaining consistency with other Liyue character names in the Russian localization.

libs/gi/dm-localization/assets/locales/th/charNames_gen.json (1)

96-96: LGTM! Character name is consistently maintained.

The character name "Lan Yan" is correctly preserved in its original form, which is appropriate for names of Chinese origin.

libs/gi/dm-localization/assets/locales/es/charNames_gen.json (1)

96-96: LGTM! Character name is consistently maintained.

The character name "Lan Yan" is correctly preserved in its original form, maintaining consistency with other language files.

libs/gi/dm-localization/assets/locales/de/charNames_gen.json (1)

96-96: LGTM! Character name is consistently maintained.

The character name "Lan Yan" is correctly preserved in its original form, aligning with the localization approach for character names.

libs/gi/dm-localization/assets/locales/vi/charNames_gen.json (1)

96-96: LGTM! Character name is consistently maintained.

The character name "Lan Yan" is correctly preserved in its original form, following the established pattern for character names.

libs/gi/dm-localization/assets/locales/fr/charNames_gen.json (1)

96-96: LGTM! Character name is consistently maintained.

The character name "Lan Yan" is correctly preserved in its original form, maintaining consistency across all localization files.

Let's verify that all required localization files have been updated:

✅ Verification successful

Character name "Lan Yan" is properly maintained across all localization files

  • Present in all 15 language files
  • Consistently preserved as "Lan Yan" in Latin-script languages
  • Appropriately localized in CJK and Cyrillic scripts
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistency of the new character entry across all localization files

# Find all charNames_gen.json files
echo "Checking for LanYan entry in all localization files..."
fd charNames_gen.json libs/gi/dm-localization/assets/locales/ --exec grep -l "LanYan" {} \;

# Verify the value is consistent
echo "\nVerifying consistent value across all files..."
fd charNames_gen.json libs/gi/dm-localization/assets/locales/ --exec grep "LanYan" {} \;

Length of output: 1655

libs/gi/dm-localization/assets/locales/pt/charNames_gen.json (1)

96-96: Verify localization coverage across all language files.

The addition of "Lan Yan" is consistent with the existing naming patterns for Chinese characters. Let's ensure comprehensive coverage across all supported languages.

Run the following script to verify the character's presence in all localization files:

✅ Verification successful

Localization coverage verified: "LanYan" is present across all language files

The character name has been properly added to all supported language files, ensuring comprehensive localization coverage.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if LanYan is consistently added across all localization files
# Expected: All locale files should contain the LanYan entry

# Find all charNames_gen.json files
echo "Checking LanYan entry in all locale files:"
fd charNames_gen.json libs/gi/dm-localization/assets/locales -x grep -l "LanYan"

# Verify if any locale file is missing the entry
echo -e "\nFiles missing LanYan entry:"
fd charNames_gen.json libs/gi/dm-localization/assets/locales -x grep -L "LanYan"

Length of output: 1244

libs/gi/dm-localization/assets/locales/en/charNames_gen.json (1)

96-96: LGTM! Consistent with English localization patterns.

The English localization maintains the original name format, which is the standard practice for character names from Liyue.

libs/gi/dm-localization/assets/locales/id/charNames_gen.json (1)

96-96: LGTM! Consistent with Indonesian localization patterns.

The Indonesian localization maintains the original name format, following the established pattern for character names.

libs/gi/dm-localization/assets/locales/it/charNames_gen.json (1)

96-96: LGTM! Consistent with Italian localization patterns.

The Italian localization maintains the original name format, which aligns with the existing pattern for Liyue character names.

libs/gi/dm-localization/assets/locales/tr/charNames_gen.json (2)

96-96: LGTM! Consistent with Turkish localization patterns.

The Turkish localization maintains the original name format while correctly localizing the element names in the Traveler entries (e.g., "Rüzgar" for Anemo).


96-96: Verify naming convention consistency with other Liyue characters.

Let's ensure the key format "LanYan" follows the established pattern for other Liyue characters.

✅ Verification successful

Naming convention is consistent with other Liyue characters

The key format "LanYan" follows the established CamelCase pattern used for two-word Liyue character names, consistent with "YunJin". Single-word names use a simple capitalized format.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check naming pattern consistency for Liyue characters
# Expected: LanYan should follow the same CamelCase pattern as other Liyue characters

# Search for all Liyue character entries to compare naming patterns
echo "Checking naming patterns for Liyue characters:"
rg "\"(Xiangling|Beidou|Xingqiu|Xiao|Ningguang|Zhongli|Yanfei|YunJin|Yaoyao|Xianyun|LanYan)\":" libs/gi/dm-localization/assets/locales/en/charNames_gen.json

Length of output: 517

libs/gi/dm-localization/assets/locales/cht/char_LanYan_gen.json (3)

10-11: LGTM! Consistent element formatting.

The HTML-like tags for formatting (<strong>) and element types (<anemo>) are used consistently across attack descriptions.

Also applies to: 14-15, 18-19


109-109: Verify the skill damage scaling values.

The passive talent mentions specific scaling values (309% and 774%) based on Elemental Mastery. These values should be cross-referenced with the skill parameters.

✅ Verification successful

Values in localization match skill parameters

The EM scaling values of 309% and 774% in the localization file exactly match the values defined in the skill parameters (3.09 and 7.74 when converted to percentages).

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify EM scaling values in passive2
echo "Checking EM scaling values in skillParam.json..."
rg -A 5 '"passive2".*\[' libs/gi/stats/Data/Characters/LanYan/skillParam.json

Length of output: 243


44-46: Verify the shield absorption values.

The skill description mentions a 250% absorption effect for Anemo damage and elemental damage after conversion. These values should be verified against the skill parameters.

libs/gi/stats/Data/Characters/LanYan/skillParam.json (3)

331-343: Confirmed: Passive talent scaling values match localization.

The passive2 array contains the correct EM scaling values:

  • Skill: 3.09 (309%)
  • Burst: 7.74 (774%)

These values match the description in the localization files.


4-19: Verify auto attack scaling progression.

The auto attack scaling values show a consistent progression pattern. However, the fifth array (lines 72-87) is identical to the fourth array (lines 55-70), which seems unusual.

Also applies to: 21-36, 38-53, 55-70, 72-87

✅ Verification successful

Identical scaling arrays are valid for this character's attack sequence.

The identical scaling values between the fourth and fifth arrays appear to be intentional. This pattern is also seen in other character files where specific attack sequences share the same scaling values. The progression within these arrays follows a consistent and balanced pattern similar to other characters.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Compare the fourth and fifth auto attack scaling arrays
echo "Checking for identical scaling arrays..."
sed -n '55,70p' libs/gi/stats/Data/Characters/LanYan/skillParam.json > fourth_array.txt
sed -n '72,87p' libs/gi/stats/Data/Characters/LanYan/skillParam.json > fifth_array.txt
diff fourth_array.txt fifth_array.txt

Length of output: 268


Script:

#!/bin/bash
# Get all auto attack arrays for comparison
echo "=== LanYan's Auto Attack Arrays ==="
echo "First array (21-36):"
sed -n '21,36p' libs/gi/stats/Data/Characters/LanYan/skillParam.json
echo -e "\nSecond array (38-53):"
sed -n '38,53p' libs/gi/stats/Data/Characters/LanYan/skillParam.json
echo -e "\nThird array (55-70):"
sed -n '55,70p' libs/gi/stats/Data/Characters/LanYan/skillParam.json
echo -e "\nFourth array (72-87):"
sed -n '72,87p' libs/gi/stats/Data/Characters/LanYan/skillParam.json

echo -e "\n=== Checking other character files for comparison ==="
fd "skillParam.json" libs/gi/stats/Data/Characters/ --exec head -n 100 {} | grep -A 15 "\["

Length of output: 67303


346-349: Review constellation parameter completeness.

Some constellation arrays are empty while others contain values. Verify that all constellation parameters are properly defined based on their descriptions in the localization files.

Also applies to: 351-354, 356-360

✅ Verification successful

Constellation parameters are correctly defined

The empty arrays and numeric parameters in the constellation definitions align perfectly with their descriptions in the localization file:

  • Empty arrays are used for passive effects (C1, C3, C5)
  • Numeric parameters match the exact values mentioned in descriptions (C2: 40% and 2s, C4: 60 EM and 12s)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check constellation descriptions against parameters
echo "Checking constellation descriptions..."
rg -A 2 '"constellation[1-6]".*name' libs/gi/dm-localization/assets/locales/*/char_LanYan_gen.json

Length of output: 1072


Script:

#!/bin/bash
# Check the complete constellation parameter structure
echo "=== Checking constellation parameters structure ==="
cat libs/gi/stats/Data/Characters/LanYan/skillParam.json | jq '.constellation1, .constellation2, .constellation3, .constellation4, .constellation5, .constellation6'

echo -e "\n=== Checking constellation localizations ==="
# Using a more flexible pattern for constellation entries
rg -g "char_LanYan_gen.json" "constellation[1-6]" libs/gi/dm-localization/assets/locales/en/

Length of output: 1050


Script:

#!/bin/bash
echo "=== Checking constellation descriptions ==="
cat libs/gi/dm-localization/assets/locales/en/char_LanYan_gen.json | jq '.constellation1, .constellation2, .constellation3, .constellation4, .constellation5, .constellation6'

Length of output: 1897

libs/gi/dm-localization/assets/locales/chs/char_LanYan_gen.json (1)

1-158: LGTM! Proper character conversion between Traditional and Simplified Chinese.

The content is consistently translated between Traditional and Simplified Chinese, maintaining the same meaning and structure while using the appropriate character variants.

libs/gi/dm-localization/assets/locales/ja/char_LanYan_gen.json (1)

7-7: Consistent naming convention for auto attack.

The Japanese version correctly prefixes the auto attack name with "通常攻撃・" (Normal Attack:), following the game's established pattern for ability names.

libs/gi/dm-localization/assets/locales/ko/char_LanYan_gen.json (1)

1-158: LGTM! Consistent localization across languages.

The Korean localization maintains consistent meaning and structure with other language versions while using appropriate Korean terminology and formatting.

libs/gi/dm-localization/assets/locales/en/char_LanYan_gen.json (2)

1-5: Basic character information looks good!

The character's name, title, and description are well-structured and provide clear, concise information about the character.


1-158: Verify localization consistency across all language files.

Let's ensure all translations maintain the same structure and HTML formatting.

✅ Verification successful

All localization files maintain consistent structure and formatting

The verification confirms that all language versions (en, vi, th, tr, ru) of char_LanYan_gen.json maintain:

  • Identical JSON structure
  • Perfect preservation of HTML formatting tags
  • Complete translations with no missing entries
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for structural consistency and HTML tag preservation across all language files

# Function to extract and compare structure
echo "Comparing JSON structure across all language files..."
for lang in en vi th tr ru; do
  jq -r 'path(..|select(type=="string")) | join(".")' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | sort > "structure_${lang}.txt"
done

# Compare structures
for lang in vi th tr ru; do
  diff "structure_en.txt" "structure_${lang}.txt"
done

# Check HTML tag consistency
echo "Checking HTML tag consistency..."
for lang in en vi th tr ru; do
  echo "=== ${lang} ==="
  grep -o '<[^>]*>' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | sort | uniq -c
done

Length of output: 3219

libs/gi/dm-localization/assets/locales/vi/char_LanYan_gen.json (1)

1-158: Vietnamese localization looks good!

The Vietnamese translation maintains proper structure and formatting while preserving cultural context.

libs/gi/dm-localization/assets/locales/tr/char_LanYan_gen.json (1)

1-158: Turkish localization looks good!

The Turkish translation maintains proper structure and formatting while preserving cultural context.

libs/gi/dm-localization/assets/locales/es/char_LanYan_gen.json (4)

1-5: Basic character information is well structured and properly localized!

The character's name, title, and description are correctly formatted and use proper Spanish localization.


6-40: Auto attack section follows the standard ability format!

The implementation includes all required attack types with proper HTML formatting and damage type tags.


41-98: Skill and burst sections are properly implemented!

Both abilities follow the game's standard format with:

  • Clear descriptions of mechanics
  • Proper HTML formatting
  • Well-structured parameters
  • Appropriate flavor text

99-157: Passives and constellations are complete and well-structured!

All talents and constellations are properly defined with:

  • Clear mechanical descriptions
  • Correct prerequisite references
  • Consistent numerical values
libs/gi/dm-localization/assets/locales/id/char_LanYan_gen.json (1)

1-158: Indonesian localization matches the base structure perfectly!

The file maintains consistent formatting and values while providing proper Indonesian translations.

libs/gi/dm-localization/assets/locales/pt/char_LanYan_gen.json (1)

1-158: Portuguese localization maintains structural integrity!

The file follows the same format as other languages while providing accurate Portuguese translations.

libs/gi/dm-localization/assets/locales/de/char_LanYan_gen.json (1)

1-158: German localization is properly structured!

The file maintains consistency with other language files while providing appropriate German translations.

libs/gi/dm-localization/assets/locales/fr/char_LanYan_gen.json (1)

1-158: French localization follows the established pattern!

The file maintains structural consistency while providing proper French translations.

libs/gi/dm-localization/assets/locales/it/char_LanYan_gen.json (1)

1-5: LGTM! Basic character information is well-defined.

The character's name, title, description, and constellation name are properly localized in Italian.

libs/gi/assets-data/src/AssetsData_gen.json (1)

2762-2778: LGTM! Asset mappings are complete and follow the standard pattern.

The LanYan entry includes all required assets and follows the same structure as other characters. The passive3 icon reuse from Sayu's talent is intentional as mentioned in the AI summary.

Comment on lines +44691 to +44706
1155.5629,
1271.1353,
1396.3386,
1531.173,
1675.6384,
1829.7349,
1993.4624,
2166.8208,
2349.8105,
2542.4312,
2744.6826,
2956.5652,
3178.0789,
3409.2236,
3649.9995
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Verify unusually high skill damage values.

The third array in the skill stats contains values starting from 1155.5629 and going up to 3649.9995, which are significantly higher than typical skill damage multipliers. Please verify if these values are correct or if they need adjustment.

Comment on lines +1 to +158
"4": "",
"5": "",
"6": "",
"7": "",
"8": "",
"9": "",
"10": "",
"11": "",
"12": "",
"13": "",
"14": "",
"15": ""
}
},
"burst": {
"name": "Lever lustré de lune",
"description": {
"0": "Lan Yan déploie une formation d'hirondelle lunaire, attirant les objets et ennemis proches et infligeant plusieurs fois des <anemo>DGT Anémo de zone</anemo>.",
"1": "<br/>",
"2": "<i>Strictement parlant, Lan Yan ne peut pas à elle seule constituer une « formation ». Cependant, elle peut en quelque sorte remplir cette exigence si ses figurines en rotin sont prises en compte... Probablement.</i>"
},
"skillParams": {
"0": "DGT compétence",
"1": "Temps de recharge",
"2": "Énergie élémentaire",
"3": "",
"4": "",
"5": "",
"6": "",
"7": "",
"8": "",
"9": "",
"10": "",
"11": "",
"12": "",
"13": "",
"14": "",
"15": ""
}
},
"passive1": {
"name": "Quatre charmes divinatoires scellés",
"description": {
"0": "Lorsque la ruée de la compétence élémentaire <strong>Danse plumée d'hirondelle-follet</strong> touche une cible, tout contact avec l'élément <hydro>Hydro</hydro>, <pyro>Pyro</pyro>, <cryo>Cryo</cryo> ou <electro>Électro</electro> permet au bouclier d'hirondelle-follet de bénéficier d'une absorption élémentaire, le bouclier pouvant ensuite absorber les DGT de l'élément correspondant 250 % plus efficacement.",
"1": "De plus, lorsque la compétence élémentaire <strong>Danse plumée d'hirondelle-follet</strong> permet au bouclier d'hirondelle-follet de bénéficier d'une absorption élémentaire, les anneaux lunaires que Lan Yan envoie sur les ennemis bénéficient d'un bonus de DGT de l'élément correspondant d'une valeur équivalant à 50 % des DGT d'origine, ces DGT étant considérés comme des DGT de compétence élémentaire."
}
},
"passive2": {
"name": "Charme panaché d'exorcisme",
"description": {
"0": "Les DGT infligés par la compétence élémentaire <strong>Danse plumée d'hirondelle-follet</strong> et le déchaînement élémentaire <strong>Lever lustré de lune</strong> augmentent respectivement d'une valeur équivalant à 309 % et 774 % de la maîtrise élémentaire de Lan Yan."
}
},
"passive3": {
"name": "Pensée comme un parfum soyeux",
"description": {
"0": "Quand Lan Yan fait partie de l'équipe, vos personnages dans l'équipe n'effraient pas certains animaux, tels que le papillon cristallin.",
"1": "Pour savoir quels sont les animaux concernés, consultez l'onglet « Divers » dans la section « Faune » du Bestiaire des Archives."
}
},
"constellation1": {
"name": "« Comme on peut marcher entre les nuages »",
"description": {
"0": "Après avoir déclenché l'absorption élémentaire de l'aptitude passive « Quatre charmes divinatoires scellés », une utilisation de la compétence élémentaire <strong>Danse plumée d'hirondelle-follet</strong> permet à Lan Yan de lancer un anneau lunaire supplémentaire sur les ennemis le moment venu.",
"1": "L'aptitude passive « Quatre charmes divinatoires scellés » doit être déjà débloquée."
}
},
"constellation2": {
"name": "« Tenue de danse ondulant comme du jade arc-en-ciel »",
"description": {
"0": "Pendant la durée du <strong>bouclier d'hirondelle-follet</strong>, le bouclier récupère 40 % d'absorption de DGT lorsque l'attaque normale de votre personnage déployé inflige des DGT. L'absorption de DGT peut être restaurée jusqu'à sa valeur d'origine, cet effet pouvant être déclenché une fois toutes les 2 s."
}
},
"constellation3": {
"name": "« Perçant nuage et brouillard sur des ailes blanches »",
"description": {
"0": "Niveau d'aptitude <strong>Danse plumée d'hirondelle-follet</strong> +3.",
"1": "Niveau max : 15"
}
},
"constellation4": {
"name": "« Des perles de sang de faucon-dragon en ornement »",
"description": {
"0": "La maîtrise élémentaire de tous les personnages de l'équipe à proximité augmente de 60 pts pendant 12 s après l'utilisation du déchaînement élémentaire <strong>Lever lustré de lune</strong>."
}
},
"constellation5": {
"name": "« De vous rencontrer mon cœur est joie »",
"description": {
"0": "Niveau d'aptitude <strong>Lever lustré de lune</strong> +3.",
"1": "Niveau max : 15"
}
},
"constellation6": {
"name": "« Sur l'aile slyphique sonnent les ornements argentés »",
"description": {
"0": "Confère une utilisation supplémentaire de <strong>Danse plumée d'hirondelle-follet</strong>."
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistencies found in localization files

  • German localization file appears to be missing or malformed
  • Time and point values are inconsistent:
    • Only French has complete "60 pts" and "12 s" formatting
    • Portuguese only shows "12s"
    • Spanish and Indonesian are missing these values
  • Core percentage values (250%, 50%, 309%, 774%, 40%) are consistent where present
🔗 Analysis chain

Verify consistency across all language files.

Let's ensure all localization files maintain consistent structure and values.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency across localization files for Lan Yan

# Check if all files exist with the same structure
for lang in es id pt de fr; do
  if [ ! -f "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" ]; then
    echo "Missing localization file for ${lang}"
    exit 1
  fi
done

# Verify consistent numerical values and HTML tags across all files
for lang in es id pt de fr; do
  echo "Checking ${lang} localization:"
  
  # Check numerical values
  echo "Numerical values:"
  jq -r '.passive2.description[0]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+%'
  
  # Count HTML tags
  echo "HTML tag count:"
  jq -r 'tostring' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '<[^>]*>' | sort | uniq -c
  
  echo "---"
done

Length of output: 4449


Script:

#!/bin/bash
# Description: Verify numerical values in Lan Yan localization files

for lang in es id pt de fr; do
  echo "Checking ${lang} localization:"
  
  # Extract and check all percentage values
  echo "Percentage values:"
  jq -r '.passive1.description[0], .passive1.description[1], .passive2.description[0]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+%'
  
  echo "---"
done

Length of output: 2189


Script:

#!/bin/bash
# First inspect the structure of one file
echo "File structure for fr locale:"
jq '.' libs/gi/dm-localization/assets/locales/fr/char_LanYan_gen.json

echo "---"
echo "Attempting to extract passive descriptions:"
for lang in fr; do
  echo "Checking ${lang}:"
  jq -r '.passive2.description[]' libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json
  echo "---"
done

Length of output: 8116


Script:

#!/bin/bash
# Extract specific numerical values from all locales
for lang in es id pt de fr; do
  echo "=== Checking ${lang} localization ==="
  
  echo "Passive 1 (should see 250% and 50%):"
  jq -r '.passive1.description[]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+\s*%'
  
  echo "Passive 2 (should see 309% and 774%):"
  jq -r '.passive2.description[]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+\s*%'
  
  echo "Constellation 2 (should see 40%):"
  jq -r '.constellation2.description[]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+\s*%'
  
  echo "Constellation 4 (should see 60 pts and 12 s):"
  jq -r '.constellation4.description[]' "libs/gi/dm-localization/assets/locales/${lang}/char_LanYan_gen.json" | grep -o '[0-9]\+\s*\(pts\|s\)'
  
  echo "---"
done

Length of output: 4962

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
libs/gi/sheets/src/Characters/LanYan/index.tsx (3)

29-31: Consider using explicit indices instead of incrementing variables.

The use of incrementing variables (a, s, b) for array indexing makes the code harder to maintain and more prone to errors if the skill parameter order changes.

Consider using explicit indices or enum values:

-let a = 0, s = 0, b = 0
+const AutoIndex = {
+  NORMAL_1: 0,
+  NORMAL_2_1: 1,
+  NORMAL_2_2: 2,
+  // ... other indices
+} as const

106-143: Add documentation for damage calculation formulas.

While the implementation is correct, adding comments explaining the damage calculation logic would improve maintainability, especially for complex cases like the A1 passive's elemental damage addition.


162-177: Improve type safety and documentation for auto attack utilities.

The utility functions would benefit from:

  1. TypeScript type annotations
  2. JSDoc comments explaining the logic
  3. Constants for magic numbers
+/** Maps internal array index to display index for multi-hit attacks */
 function autoIndex(index: number) {
+  const FIRST_MULTI_HIT = 3
   if (index > 3) {
     return index - 2
   } else if (index > 1) {
     return index - 1
   }
   return index
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68082dc and b9f6681.

📒 Files selected for processing (1)
  • libs/gi/sheets/src/Characters/LanYan/index.tsx (1 hunks)
🔇 Additional comments (4)
libs/gi/sheets/src/Characters/LanYan/index.tsx (4)

1-28: LGTM! Imports and initial setup are properly structured.

The necessary imports and character setup follow the established pattern.


145-160: LGTM! Constellation bonuses are properly implemented.

The constellation bonuses and their corresponding modifiers are correctly defined.


196-202: Verify charged attack hit count.

The charged attack damage has a multiplier of 3 (multi: 3). Please verify this is the correct number of hits for LanYan's charged attack animation.


81-86: Verify shield scaling attribute.

The shield scales with ATK, which is unusual as most shields in Genshin Impact scale with HP or DEF. Please verify this is intended.

✅ Verification successful

Shield scaling with ATK is valid.

While most shields scale with HP or DEF, there are precedents for alternative scaling attributes, such as Citlali's shield scaling with Elemental Mastery. LanYan's ATK scaling shield represents a similar design variation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other shield implementations to compare scaling attributes
ast-grep --pattern 'shieldNodeTalent($attr, $_, $_, $_)' | grep -B 2 -A 2 'shieldNodeTalent'

Length of output: 92


Script:

#!/bin/bash
# Search for shield-related implementations and calculations
rg -g '*.{ts,tsx}' "shield.*Talent|shieldNode" -B 2 -A 2

# Find shield utility function definitions
rg -g '*.{ts,tsx}' "export.*shield.*=.*" -B 2 -A 2

# Find character files that might have shields
fd -e tsx -e ts . libs/gi/sheets/src/Characters | xargs rg "shield" -l

Length of output: 18411

Copy link
Contributor

github-actions bot commented Jan 25, 2025

[frontend] [Sat Jan 25 18:45:21 UTC 2025] - Deployed ed29d53 to https://genshin-optimizer-prs.github.io/pr/2617/frontend (Takes 3-5 minutes after this completes to be available)

[Sat Jan 25 19:03:30 UTC 2025] - Deleted deployment

Copy link
Owner

@frzyc frzyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :clueless:

@nguyentvan7 nguyentvan7 merged commit a030cc0 into master Jan 25, 2025
9 checks passed
@nguyentvan7 nguyentvan7 deleted the van/go/lanyan branch January 25, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants