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

initial SRO light cone sheet with targeted buff #2531

Merged
merged 4 commits into from
Nov 15, 2024
Merged

initial SRO light cone sheet with targeted buff #2531

merged 4 commits into from
Nov 15, 2024

Conversation

frzyc
Copy link
Owner

@frzyc frzyc commented Nov 6, 2024

Describe your changes

Pretty much #2520, but for lightcones.

Depends on the changes in #2533.

Issue or discord link

Testing/validation

image

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 Features

    • Introduced a new React component for displaying light cone information in a themed card format.
    • Added functionality for toggling visibility of light cone conditionals in the Combo Editor.
    • New light cone sheets and conditionals have been added for various characters.
    • Added a new optional property in the Header type for more flexible usage in components.
  • Improvements

    • Enhanced layout and presentation of relic and light cone displays using Material-UI components.
    • Improved logic for applying buffs based on equipped relic counts.
  • Bug Fixes

    • Removed deprecated properties in the light cone data structure for improved data handling.

@frzyc frzyc requested a review from nguyentvan7 November 6, 2024 14:22
Copy link
Contributor

coderabbitai bot commented Nov 6, 2024

Walkthrough

The changes in this pull request involve multiple modifications across various files within the project. Key updates include making the additional property optional in the Header type, adding a new lightCone export, and introducing new components for displaying light cone information. Additionally, several existing components were modified for better structure and presentation, while some files saw the removal of outdated logic and the introduction of new conditional checks for buffs.

Changes

File Change Summary
libs/pando/ui-sheet/src/types/header.ts Updated Header type: changed additional: ReactNode to additional?: ReactNode.
libs/sr/formula-ui/src/index.ts Added export statement for lightCone: export * from './lightCone'.
libs/sr/formula-ui/src/lightCone/index.ts Introduced lightConeUiSheets: Partial<Record<LightConeKey, UISheetElement>> with entry for PastSelfInMirror.
libs/sr/formula-ui/src/lightCone/sheets/PastSelfInMirror.tsx Created new React component for "Past Self In Mirror" light cone, exporting sheet object.
libs/sr/formula-ui/src/lightCone/util.tsx Added SuperImposeWrapper React component for handling superimpose calculations.
libs/sr/formula-ui/src/relic/cavern/WatchmakerMasterOfDreamMachinations.tsx Minor comment change; no functional alterations.
libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts Removed old conditionals; introduced useUltimate for buff logic.
libs/sr/formula/src/data/relic/sheets/WatchmakerMasterOfDreamMachinations.ts Added conditional check for team buff based on relic count.
libs/sr/formula/src/meta.ts Added new entries for conditionals and formulas for multiple characters.
libs/sr/page-team/src/ComboEditor.tsx Updated import for Tag type; added LightConeConditionals function.
libs/sr/page-team/src/LightConeSheetDisplay.tsx Created new component for displaying light cone information.
libs/sr/page-team/src/LightConeSheetsDisplay.tsx Introduced LightConeSheetsDisplay component for structured light cone display.
libs/sr/page-team/src/RelicSheetDisplay.tsx Modified layout and styling of relic display components; no functional changes.
libs/sr/stats/src/mappedStats/lightCone/maps/PastSelfInMirror.ts Updated dm object structure, replacing old properties with new ones.

Assessment against linked issues

Objective Addressed Explanation
Add light cone UI sheets framework for targeted buffs (#2530)

Possibly related PRs

  • initial SRO relic ui-sheet #2520: The changes in this PR involve the creation of a UI sheet for the "Watchmaker Master of Dream Machinations" relic, which may relate to the modifications in the Header type that allow for more flexible usage in components, potentially including relic UI sheets.
  • minor updates to relic/lightcone/character stat display #2521: This PR focuses on enhancing the display of statistics for relics, which could be relevant to the changes in the Header type as it may affect how data is structured and displayed in the UI.
  • Add light cone optimization #2533: This PR introduces optimizations related to light cones, which may connect to the changes in the Header type if light cones are displayed or managed in a similar manner as relics, indicating a broader impact on UI components that utilize the Header type.

Suggested reviewers

  • nguyentvan7
  • tiffanynwyeung

🐰 In the meadow where we play,
Changes bloom, bright as day.
Headers shift, and UI gleams,
Light cones dance in our dreams.
With every line, our code takes flight,
Hopping forward, pure delight! 🌼✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 58dc487 and 24c7565.

📒 Files selected for processing (2)
  • libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts (1 hunks)
  • libs/sr/page-team/src/LightConeSheetDisplay.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/sr/page-team/src/LightConeSheetDisplay.tsx
🔇 Additional comments (2)
libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts (2)

19-19: LGTM: Clean implementation of the conditional logic.

The use of allBoolConditionals for tracking ultimate usage state is a good approach, following the established patterns in the codebase.


30-30: Verify the count-based conditional buff implementation.

The implementation correctly uses cmpGE to check the light cone count before applying the buff, similar to artifact set counters. However, we should verify if this is the intended behavior as per the past review comment.

Let's check if other light cones use similar count-based conditionals:

✅ Verification successful

Based on the search results, I can now generate the final response since I have enough information to verify the count-based conditional implementation:

The count-based conditional implementation using cmpGE is consistent across all light cone sheets

The search results show that all light cone sheets in the codebase follow the same pattern of using cmpGE(lcCount, 1, ...) to implement count-based conditional buffs. This is a standardized approach used throughout the codebase, confirming that the implementation in PastSelfInMirror.ts follows the established pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for similar light cone count-based conditional patterns
# Expected: Find other instances of lcCount usage in light cone sheets

# Search for lcCount usage in light cone sheets
rg "lcCount" "libs/sr/formula/src/data/lightCone/sheets/" -A 5

# Search for cmpGE with count checks
ast-grep --pattern 'cmpGE($count, $_, $buff)'

Length of output: 192383


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 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.

@frzyc frzyc added the SRO For Star Rail Optimizer label Nov 6, 2024
@frzyc frzyc self-assigned this Nov 6, 2024
Copy link
Contributor

github-actions bot commented Nov 6, 2024

[sr-frontend] [Wed Nov 6 14:27:04 UTC 2024] - Deployed 107e3c4 to https://genshin-optimizer-prs.github.io/pr/2531/sr-frontend (Takes 3-5 minutes after this completes to be available)

[sr-frontend] [Wed Nov 6 14:30:03 UTC 2024] - Deployed fc9d22b to https://genshin-optimizer-prs.github.io/pr/2531/sr-frontend (Takes 3-5 minutes after this completes to be available)

[sr-frontend] [Wed Nov 6 14:55:55 UTC 2024] - Deployed 52aa27b to https://genshin-optimizer-prs.github.io/pr/2531/sr-frontend (Takes 3-5 minutes after this completes to be available)

[sr-frontend] [Sun Nov 10 17:48:24 UTC 2024] - Deployed 904967c to https://genshin-optimizer-prs.github.io/pr/2531/sr-frontend (Takes 3-5 minutes after this completes to be available)

[Fri Nov 15 19:00:39 UTC 2024] - Deleted deployment

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: 5

🧹 Outside diff range and nitpick comments (18)
libs/pando/ui-sheet/src/types/header.ts (1)

3-7: Consider adding JSDoc comments to document the type.

Adding JSDoc comments would help other developers understand the purpose of each field and when additional should/shouldn't be provided.

+/**
+ * Represents a header component with icon and text content.
+ */
 export type Header = {
+  /** The icon to display in the header */
   icon: ReactNode
+  /** The main text content of the header */
   text: ReactNode
+  /** Optional additional content to display in the header */
   additional?: ReactNode
 }
libs/sr/formula-ui/src/lightCone/index.ts (2)

5-8: Add documentation for the exported mapping.

While the implementation is correct, consider adding JSDoc documentation to explain:

  • The purpose of this mapping
  • How it's used in the light cone sheet display
  • Why it's a partial record
+/**
+ * Mapping of light cone keys to their corresponding UI sheet elements.
+ * This is used by LightConeSheetDisplay to render the appropriate UI for each light cone.
+ * Partial record as not all light cones have specialized UI sheets.
+ */
 export const lightConeUiSheets: Partial<Record<LightConeKey, UISheetElement>> =

5-8: Consider adding a type guard helper.

To prevent runtime errors when accessing non-existent light cone sheets, consider adding a type guard helper function.

/**
 * Type guard to check if a light cone has a UI sheet implementation
 */
export function hasLightConeSheet(key: LightConeKey): key is keyof typeof lightConeUiSheets {
  return key in lightConeUiSheets
}
libs/sr/formula-ui/src/lightCone/util.tsx (2)

5-13: Add JSDoc documentation for better maintainability.

The implementation looks good, but would benefit from documentation explaining its purpose and usage.

Add JSDoc comments above the component:

+/**
+ * A wrapper component that provides the light cone's superimposition level to its children.
+ * @param children - A render function that receives the superimposition level
+ * @returns The rendered children with the computed superimposition level
+ * @example
+ * <SuperImposeWrapper>
+ *   {(superimpose) => <div>Superimposition: {superimpose}</div>}
+ * </SuperImposeWrapper>
+ */
export function SuperImposeWrapper({

10-11: Consider adding error boundaries for calc context.

While the nullish coalescing operator handles undefined calc gracefully, consider wrapping the component usage with an error boundary to handle potential computation errors.

Example error boundary implementation:

class LightConeErrorBoundary extends React.Component<{ children: ReactNode }, { hasError: boolean }> {
  constructor(props: { children: ReactNode }) {
    super(props);
    this.state = { hasError: false };
  }

  static getDerivedStateFromError() {
    return { hasError: true };
  }

  render() {
    if (this.state.hasError) {
      return <div>Error loading light cone data.</div>;
    }
    return this.props.children;
  }
}
libs/sr/stats/src/mappedStats/lightCone/maps/PastSelfInMirror.ts (1)

Line range hint 1-13: Improve code documentation and maintainability.

The code lacks documentation explaining what each stat represents and their expected values. This makes maintenance difficult, especially when working with game-specific terminology.

Consider adding:

  1. JSDoc comments explaining each stat's purpose
  2. Type definitions for the expected shape of the data
  3. Constants for the expected array indices

Example:

/** Stats for the "Past Self in Mirror" Light Cone */
interface PastSelfInMirrorStats {
  /** Break effect percentage */
  _brEffect_: number;
  /** Damage boost percentage */
  dmg_: number;
  /** Effect duration in seconds */
  duration: number;
  /** Break effect threshold */
  brEffect_thresh_: number;
  /** Energy regeneration rate */
  enerRegn: number;
}

const STAT_INDICES = {
  BR_EFFECT: 0,
  DMG: 1,
  DURATION: 2,
  BR_EFFECT_THRESH: 3,
  ENER_REGN: 4,
} as const;

// ... rest of the code
libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts (1)

29-30: LGTM! Consider adding documentation.

The buff registration is well-implemented, using team-wide damage modification that scales with superimpose level when the ultimate is active. This is a cleaner approach compared to the previous implementation.

Consider adding a comment to document the buff's behavior:

  registerBuff(
    'cond_dmg_',
+   // Increases team damage when the wearer uses their ultimate
    teamBuff.premod.dmg_.add(useUltimate.ifOn(subscript(superimpose, dm.dmg_)))
  )
libs/sr/page-team/src/LightConeSheetsDisplay.tsx (2)

6-7: TODO comment needs tracking and implementation plan.

The hardcoded array limits the component's reusability. Consider:

  1. Making the light cone list configurable through props
  2. Implementing a data-driven approach to dynamically load available light cones

Would you like me to:

  1. Create a GitHub issue to track this TODO?
  2. Propose an implementation for making the light cone list configurable?

8-22: Consider enhancing responsive layout.

While the current grid implementation works, it could benefit from responsive design improvements:

-            <Grid item xs={1} key={lcKey}>
+            <Grid item xs={3} sm={2} md={1} key={lcKey}>

This change would:

  • Show 1 column on mobile (xs)
  • Show 1.5 columns on tablets (sm)
  • Show 3 columns on desktop (md and up)
libs/sr/page-team/src/LightConeSheetDisplay.tsx (2)

24-32: Simplify NextImage implementation.

The current NextImage fallback pattern could be simplified. Consider creating a reusable image component that handles this logic internally.

-          <Box
-            component={NextImage ? NextImage : 'img'}
-            alt="Relic Piece Image"
-            src={lightConeAsset(lcKey, 'cover')}
-            sx={{
-              maxHeight: '5em',
-              width: 'auto',
-            }}
-          />
+          <AdaptiveImage
+            component={NextImage}
+            fallbackComponent="img"
+            alt={`Light Cone ${lcKey} Cover`}
+            src={lightConeAsset(lcKey, 'cover')}
+            sx={{ maxHeight: '5em', width: 'auto' }}
+          />

42-61: Consider memoizing the LightConeUiSheetElement component.

Since this component receives props that might not change frequently, memoizing it could improve performance by preventing unnecessary re-renders.

-function LightConeUiSheetElement({
+const LightConeUiSheetElement = React.memo(function LightConeUiSheetElement({
   uiSheetElement,
 }: {
   uiSheetElement: UISheetElement
 }) {
   const { documents, title } = uiSheetElement
   return (
     // ... component implementation ...
   )
-}
+})
libs/sr/page-team/src/RelicSheetDisplay.tsx (1)

35-37: LGTM! Consider documenting heading hierarchy.

The Typography wrapper with h6 variant improves semantic structure and maintains consistent styling.

Consider adding a comment documenting the heading level choice to help maintain accessibility standards across the application.

libs/sr/formula-ui/src/lightCone/sheets/PastSelfInMirror.tsx (3)

22-25: Implement text interpolation for light cone description

The TODO comment indicates that text interpolation is missing. This could affect the display of dynamic values in the description.

Would you like me to help implement the getInterpolateObject functionality for light cones?


61-68: Add translation for energy regeneration title

The title "Energy regen at start of each wave" should use the translation system for consistency.

- title: 'Energy regen at start of each wave',
+ title: trans('dm.energyRegenWaveStart'),

18-72: Consider enhancing type safety for sheet configuration

While the current implementation is functional, consider these improvements for better type safety and validation:

  1. Create a type for the document structure to ensure consistent formatting across light cone sheets
  2. Add runtime validation for the dm values to catch potential data inconsistencies early

Would you like me to provide an example implementation of these improvements?

libs/sr/page-team/src/ComboEditor.tsx (2)

135-136: Address the translation TODO comment.

The button text "Light Cone Conditionals" needs to be added to the translation system.

Would you like me to help set up the translation key and update this component accordingly?


131-142: Consider refactoring to reduce code duplication.

The LightConeConditionals component is nearly identical to RelicConditionals. Consider creating a reusable modal component to reduce duplication.

Here's a suggested refactor:

+interface ConditionalModalProps {
+  buttonText: string;
+  children: React.ReactNode;
+}
+
+function ConditionalModal({ buttonText, children }: ConditionalModalProps) {
+  const [show, onShow, onHide] = useBoolState()
+  return (
+    <>
+      <Button onClick={onShow}>{buttonText}</Button>
+      <ModalWrapper open={show} onClose={onHide}>
+        {children}
+      </ModalWrapper>
+    </>
+  )
+}

 function RelicConditionals() {
-  const [show, onShow, onHide] = useBoolState()
   return (
-    <>
-      <Button onClick={onShow}>Relic Conditionals</Button>
-      <ModalWrapper open={show} onClose={onHide}>
+    <ConditionalModal buttonText="Relic Conditionals">
         <RelicSheetsDisplay />
-      </ModalWrapper>
-    </>
+    </ConditionalModal>
   )
 }

 function LightConeConditionals() {
-  const [show, onShow, onHide] = useBoolState()
   return (
-    <>
-      <Button onClick={onShow}>Light Cone Conditionals</Button>
-      <ModalWrapper open={show} onClose={onHide}>
+    <ConditionalModal buttonText="Light Cone Conditionals">
         <LightConeSheetsDisplay />
-      </ModalWrapper>
-    </>
+    </ConditionalModal>
   )
 }
libs/sr/formula/src/meta.ts (1)

2416-2418: LGTM! Consider adding documentation.

The new useUltimate conditional for PastSelfInMirror follows the established pattern and naming conventions.

Consider adding a comment to document when this conditional is triggered and its gameplay implications for better maintainability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 12298ec and 58dc487.

📒 Files selected for processing (14)
  • libs/pando/ui-sheet/src/types/header.ts (1 hunks)
  • libs/sr/formula-ui/src/index.ts (1 hunks)
  • libs/sr/formula-ui/src/lightCone/index.ts (1 hunks)
  • libs/sr/formula-ui/src/lightCone/sheets/PastSelfInMirror.tsx (1 hunks)
  • libs/sr/formula-ui/src/lightCone/util.tsx (1 hunks)
  • libs/sr/formula-ui/src/relic/cavern/WatchmakerMasterOfDreamMachinations.tsx (1 hunks)
  • libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts (1 hunks)
  • libs/sr/formula/src/data/relic/sheets/WatchmakerMasterOfDreamMachinations.ts (0 hunks)
  • libs/sr/formula/src/meta.ts (1 hunks)
  • libs/sr/page-team/src/ComboEditor.tsx (4 hunks)
  • libs/sr/page-team/src/LightConeSheetDisplay.tsx (1 hunks)
  • libs/sr/page-team/src/LightConeSheetsDisplay.tsx (1 hunks)
  • libs/sr/page-team/src/RelicSheetDisplay.tsx (2 hunks)
  • libs/sr/stats/src/mappedStats/lightCone/maps/PastSelfInMirror.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • libs/sr/formula/src/data/relic/sheets/WatchmakerMasterOfDreamMachinations.ts
✅ Files skipped from review due to trivial changes (1)
  • libs/sr/formula-ui/src/relic/cavern/WatchmakerMasterOfDreamMachinations.tsx
🔇 Additional comments (14)
libs/sr/formula-ui/src/index.ts (1)

2-2: LGTM! Verify the exported module.

The new export follows the established pattern and aligns with the PR objectives for adding light cone support.

Let's verify that the exported module exists:

✅ Verification successful

Export verified and exists as expected

The lightCone module exists at the expected location with its index file, confirming that the export statement is valid and won't cause build issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the lightCone module exists
# Expected: The lightCone directory and index file should exist

# Check if the lightCone directory exists
fd --type d "^lightCone$" libs/sr/formula-ui/src

# Check if the index file exists
fd --type f "^index\.(ts|tsx)$" libs/sr/formula-ui/src/lightCone

Length of output: 193

libs/pando/ui-sheet/src/types/header.ts (1)

6-6: LGTM! Let's verify the impact of this change.

The change to make additional optional is syntactically correct, but we should verify that all components using this type handle the optional case appropriately.

✅ Verification successful

Based on the search results, I can now provide a final response:

Making additional optional is safe and correctly handled

The codebase shows that the additional property in the Header type is safely handled:

  1. The only usage is in HeaderDisplay.tsx where it's destructured with an alias as action:
const { icon, text: title, additional: action } = header
  1. The property is used in a safe way through the CardHeaderCustom component's action prop, which naturally handles undefined/optional values.

  2. No non-null assertions (!), mandatory checks, or unsafe assumptions about the property are found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all usages of the Header type to ensure they handle the optional case
# Look for components that might assume additional is always present

# Search for Header type usage
rg -l "Header" --type ts --type tsx | xargs rg -A 5 "additional"

# Search for potential type assertions or non-null checks that might need updating
rg -l "Header" --type ts --type tsx | xargs rg "additional!"

Length of output: 169202


Script:

#!/bin/bash
# Search for Header type usage and any non-null assertions on additional property
ast-grep --pattern $'interface Header {
  $$$
  additional$_: $_
  $$$
}'

# Search for direct usage of header.additional with non-null assertion
ast-grep --pattern 'header.additional!'

# Search for components using Header type and accessing additional property
rg -l "Header" --type typescript | xargs rg "\.additional\b" -A 2 -B 2

Length of output: 222


Script:

#!/bin/bash
# Search for files containing Header type and check usage of additional property
rg -t ts -t tsx "Header.*{[^}]*additional" -B 2 -A 2

# Search for components destructuring or accessing additional property
rg -t ts -t tsx "additional.*=.*header" -B 2 -A 2

# Search for any non-null assertions on additional property
rg -t ts -t tsx "additional!" -B 2 -A 2

Length of output: 320


Script:

#!/bin/bash
# Search for Header interface definition and usage
rg "interface Header" -A 5 --type ts

# Search for components using the additional prop
rg "additional.*header" --type ts

# Look for files importing and using Header type
rg "import.*Header" --type ts -A 5

Length of output: 41318

libs/sr/formula-ui/src/lightCone/index.ts (2)

1-3: LGTM! Clean and well-organized imports.

The imports follow TypeScript best practices with explicit type imports and clear organization.


1-8: Verify integration with LightConeSheetDisplay.

Let's verify that this new export is properly integrated with the LightConeSheetDisplay component mentioned in the summary.

✅ Verification successful

This will help us understand:

  1. How lightConeUiSheets is used in the component
  2. If the structure matches similar patterns in the codebase

Integration verified and matches expected patterns

The implementation is correctly integrated:

  • lightConeUiSheets is properly imported and used in LightConeSheetDisplay
  • The component handles undefined sheets gracefully with a null check
  • The pattern of using Record<LightConeKey, ...> is consistent with other light cone related mappings in the codebase
  • The structure follows the UI sheet pattern with proper typing (UISheetElement)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of lightConeUiSheets in LightConeSheetDisplay

# Test 1: Find the LightConeSheetDisplay component
echo "Looking for LightConeSheetDisplay component..."
rg -l "LightConeSheetDisplay"

# Test 2: Verify the import and usage of lightConeUiSheets
echo "Checking usage of lightConeUiSheets..."
rg "import.*lightConeUiSheets.*from.*formula-ui"
rg "lightConeUiSheets\[.*\]"

Length of output: 606


Script:

#!/bin/bash
# Let's verify the usage context in LightConeSheetDisplay

# Check the implementation of LightConeSheetDisplay
echo "Content of LightConeSheetDisplay.tsx:"
cat libs/sr/page-team/src/LightConeSheetDisplay.tsx

# Check if there are similar patterns in other light cone related files
echo -e "\nChecking similar patterns in other files:"
rg "Record<.*LightConeKey.*>" -A 2

Length of output: 3201

libs/sr/formula/src/data/lightCone/sheets/PastSelfInMirror.ts (1)

19-19: LGTM! Clean conditional declaration.

The new useUltimate conditional is well-named and follows the established pattern for conditional declarations in the codebase.

libs/sr/page-team/src/LightConeSheetsDisplay.tsx (2)

1-4: LGTM! Clean and well-organized imports.

The imports are properly structured with clear separation between external dependencies and local components.


10-10: Verify CardThemed component requirements.

Let's verify if the CardThemed component has any required props that need to be provided.

libs/sr/page-team/src/LightConeSheetDisplay.tsx (1)

1-15: LGTM! Well-organized imports.

The imports are properly structured with clear separation between types and components.

libs/sr/page-team/src/RelicSheetDisplay.tsx (2)

61-61: Great use of Material-UI patterns!

The changes demonstrate excellent practices:

  • Using Stack for consistent spacing between elements
  • Maintaining typography hierarchy with h6 variant
  • Following the same pattern as LightConeSheetDisplay for consistency

Also applies to: 64-68


35-37: Verify consistency with light cone implementation.

Let's ensure these UI patterns are consistently applied in the light cone implementation referenced in PR #2520.

Also applies to: 61-68

✅ Verification successful

Let me try a different approach to verify the consistency by searching for specific patterns in the codebase.


Let me try one more time with corrected file type flags:


Based on the search results, I can see that both relic and light cone implementations follow consistent UI patterns using Typography and Stack components. Specifically:

  1. The light cone implementation in LightConeSheetDisplay.tsx uses:
<Typography variant="h6">
  <LightConeName lcKey={lcKey} />
</Typography>
  1. The relic implementation in RelicSheetDisplay.tsx uses:
<Typography variant="h6">
  <RelicSetName setKey={setKey} />
</Typography>

Both implementations also use similar Stack spacing and CardHeader patterns:

<Stack spacing={1}>
  {documents.map((doc, i) => (
    <DocumentDisplay key={i} document={doc} />
  ))}
</Stack>

The UI patterns are consistently applied between relic and light cone implementations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent UI patterns between relic and light cone implementations

# Test 1: Check for similar Typography and Stack usage in light cone components
echo "Checking for consistent UI patterns in light cone components..."
rg -l "Typography.*variant.*h6|Stack.*spacing.*1" --type tsx

# Test 2: Check for similar CardHeader usage
echo "Checking for consistent CardHeader usage..."
ast-grep --pattern 'CardHeader title={$_} titleTypographyProps={{ variant: "h6" }}'

Length of output: 410


Script:

#!/bin/bash
# Search for Typography and Stack components in the codebase
echo "Searching for Typography and Stack patterns..."
rg -A 2 'Typography.*variant.*h6|Stack.*spacing' --type ts --type tsx

# Search for light cone related components
echo "Searching for light cone related files and components..."
rg -l "LightCone|lightcone" --type ts --type tsx

# Search for specific component structure in light cone files
echo "Checking component structure in light cone files..."
rg -A 5 '<Typography|<Stack' --type ts --type tsx

Length of output: 597


Script:

#!/bin/bash
# Search for Typography and Stack components in the codebase
echo "Searching for Typography and Stack patterns..."
rg -A 2 'Typography.*variant.*h6|Stack.*spacing'

# Search for light cone related components
echo "Searching for light cone related files and components..."
fd -e tsx -e ts | xargs rg -l "LightCone|lightcone"

# Search for component structure in light cone related files
echo "Searching component structure in PR #2520..."
gh pr view 2520 --json files -q '.files[].path' | grep -i 'lightcone' | xargs rg -A 5 '<Typography|<Stack' 2>/dev/null

Length of output: 351230

libs/sr/formula-ui/src/lightCone/sheets/PastSelfInMirror.tsx (1)

1-17: LGTM: Clean imports and constant declarations

The imports are well-organized and the constants are properly typed and initialized.

libs/sr/page-team/src/ComboEditor.tsx (2)

8-8: LGTM! Import statements are correctly structured.

The imports are properly organized, with the new LightConeSheetsDisplay import supporting the new light cone conditionals feature.

Also applies to: 21-21


112-112: LGTM! Component integration follows existing patterns.

The LightConeConditionals component is properly integrated into the Stack, maintaining consistency with the existing UI structure.

libs/sr/formula/src/meta.ts (1)

2416-2418: Verify UI integration for the new conditional.

Let's ensure the UI components properly handle this new conditional.

✅ Verification successful

UI integration for the new conditional is properly implemented

The PastSelfInMirror component correctly implements the useUltimate conditional with:

  • Proper metadata binding to cond.useUltimate
  • Appropriate field displays for damage bonus and duration
  • Correct integration with the light cone index
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if UI components handle the new useUltimate conditional

# Test 1: Find UI components that reference PastSelfInMirror
echo "Searching for UI components that use PastSelfInMirror..."
rg -l "PastSelfInMirror" --type tsx

# Test 2: Check if the useUltimate conditional is properly referenced
echo "Checking for useUltimate conditional usage..."
rg "useUltimate.*PastSelfInMirror|PastSelfInMirror.*useUltimate" --type tsx

Length of output: 393


Script:

#!/bin/bash
# Let's try searching without file type restrictions since tsx might not be recognized
# and also include other possible UI file extensions

# Search for PastSelfInMirror references in potential UI files
echo "Searching for UI components that use PastSelfInMirror..."
rg -l "PastSelfInMirror" --type js --type jsx --type ts

# Search for useUltimate and PastSelfInMirror usage
echo "Checking for useUltimate conditional usage..."
rg -A 2 -B 2 "useUltimate.*PastSelfInMirror|PastSelfInMirror.*useUltimate" --type js --type jsx --type ts

# Additional search for any UI-related files that might reference these
echo "Searching in UI-related directories..."
fd -e js -e jsx -e ts -e tsx -p ".*ui.*|.*components.*|.*views.*" | xargs rg -l "PastSelfInMirror|useUltimate"

Length of output: 822


Script:

#!/bin/bash
# Let's examine the content of the files we found to verify the integration

echo "Checking PastSelfInMirror component implementation..."
cat "libs/sr/formula-ui/src/lightCone/sheets/PastSelfInMirror.tsx"

echo -e "\nChecking lightCone index file..."
cat "libs/sr/formula-ui/src/lightCone/index.ts"

echo -e "\nChecking WatchmakerMasterOfDreamMachinations component..."
cat "libs/sr/formula-ui/src/relic/cavern/WatchmakerMasterOfDreamMachinations.tsx"

Length of output: 5108

@frzyc frzyc marked this pull request as draft November 9, 2024 05:19
@frzyc frzyc marked this pull request as ready for review November 10, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SRO For Star Rail Optimizer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add light cone ui sheets framework for targeted buffs
2 participants