Add new theme support and improve localization handling#423
Open
dimon222 wants to merge 3 commits into
Open
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds per-locale ignored-item maps to LanguageProcessor, exposes clipboard/ignore helpers in Data, uses normalized OCR text for non‑English part matching (with fallbacks to IgnoredItemNames), updates OCR reward/clipboard behavior and themes, and bumps schema, deps, and assembly version. ChangesIgnored Item Names & Localization + OCR/Data Integration
Schema & Build Metadata
Sequence DiagramsequenceDiagram
participant OCR as OCR Engine
participant Data as Data API
participant LP as LanguageProcessor
participant Cache as IgnoredNamesCache
participant UI as UI/Clipboard
OCR->>Data: IsIgnoredItem(partText)
Data->>LP: IsIgnoredItem(partText)
LP->>Cache: Lookup cached HashSet (keys+values)
Cache-->>LP: true/false
LP-->>Data: result
alt ignored
Data-->>OCR: true
OCR->>OCR: hideRewardInfo = true
else not ignored
OCR->>Data: GetLocalizedNameForClipboard(englishName)
Data->>LP: lookup localized name in marketItems or processor
LP-->>Data: localizedName
Data->>Data: RemoveBlueprintTerms(localizedName)
Data-->>OCR: cleanedClipboardText
OCR->>UI: display / copy clipboard
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 14 minutes and 23 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
WFInfo/Data.cs (1)
1049-1113:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocalized ignored items still cannot resolve here.
This branch only searches
marketItems, butReloadItems()only adds entries whose names contain" Prime"on Line 181-Line 183. That excludes every new ignored-item mapping for localized non-prime rewards (Forma Blueprint,Kuva,Riven Sliver,Ayatan Amber Star, etc.), so this path still returns9999/nullandWFInfo/Ocr.csdrops the reward before the ignore handling runs. Add a fallback against the current processor’sIgnoredItemNames(ornameData) before treating the OCR result as unmatched.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WFInfo/Data.cs` around lines 1049 - 1113, The localized branch only searches marketItems (populated by ReloadItems()) which omits many localized ignored-item mappings, so add a fallback that checks the current language processor's ignored-name mappings (e.g., processor.IgnoredItemNames or processor.nameData) using the same normalization and Levenshtein logic before returning unmatched; specifically, after building marketItemsSnapshot but before concluding this branch, iterate the processor's IgnoredItemNames/nameData entries, compute CalculateLevenshteinDistance against normalizedName, apply the same distance filter (val < storedName.Length * 0.5) and update low, lowest, lowest_unfiltered and multipleLowest exactly as done for marketItems entries so localized ignored items (Forma Blueprint, Kuva, Riven Sliver, etc.) are resolved.
🧹 Nitpick comments (2)
WFInfo/LanguageProcessing/EnglishLanguageProcessor.cs (1)
22-35: ⚡ Quick win
IgnoredItemNamesallocates a newDictionaryon every property access across all five processorsEvery language processor defines
IgnoredItemNamesas an expression-bodied property (=>), which constructs and returns a freshDictionary<string, string>on each access. Even if the base class caches it into aHashSetafter the first read, any subsequent direct accesses (e.g., for debugging, reflection, or future base-class changes) re-allocate. Consider backing with astatic readonlyfield instead:♻️ Suggested pattern (apply to all five processors)
- public override Dictionary<string, string> IgnoredItemNames => new Dictionary<string, string> - { - ["Forma Blueprint"] = "Forma Blueprint", - ... - }; + private static readonly Dictionary<string, string> _ignoredItemNames = new Dictionary<string, string> + { + ["Forma Blueprint"] = "Forma Blueprint", + ... + }; + + public override Dictionary<string, string> IgnoredItemNames => _ignoredItemNames;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WFInfo/LanguageProcessing/EnglishLanguageProcessor.cs` around lines 22 - 35, IgnoredItemNames currently constructs a new Dictionary<string,string> on each property access in EnglishLanguageProcessor (and the other language processor classes); change it to use a shared static readonly backing field (e.g., private static readonly Dictionary<string,string> _ignoredItemNames) and have the IgnoredItemNames property return that field instead of creating a new Dictionary each time; update EnglishLanguageProcessor.IgnoredItemNames (and apply the same change to the other four processor classes) so the dictionary is allocated once and reused.WFInfo/LanguageProcessing/EuropeanLanguageProcessor.cs (1)
86-99: Consider cachingIgnoredItemNamesasstatic readonlyfields for cleaner initialization.The base class uses
GetOrAddcaching to compute a HashSet once per processor type (line 94–112 of LanguageProcessor.cs), so the dictionary allocation is not a performance bottleneck. However, storing the dictionaries as static readonly fields eliminates even that single allocation per processor type and simplifies the property getter to a single return statement. This is a low-effort, optional refactoring.Also worth considering: the abstract declaration at
LanguageProcessor.cs:86usesDictionary<string, string>as the return type. UsingIReadOnlyDictionary<string, string>would better communicate that callers should not mutate these collections and would provide additional safety.Applies to lines: 86–99, 118–131, 164–177, 222–235, 284–297 (all five processors in EuropeanLanguageProcessor.cs)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WFInfo/LanguageProcessing/EuropeanLanguageProcessor.cs` around lines 86 - 99, Replace the per-instance dictionary initializations for the IgnoredItemNames properties in EuropeanLanguageProcessor (all five processor classes) with shared cached static readonly fields and have the property return that field; specifically, create private static readonly IReadOnlyDictionary<string,string> fields (one per processor) initialized once with the current key/value pairs, then change each override of IgnoredItemNames to return its corresponding static field (e.g., change the override of IgnoredItemNames in EuropeanLanguageProcessor to return the newly created static field). Also update the abstract property declaration in LanguageProcessor (LanguageProcessor.IgnoredItemNames) from Dictionary<string,string> to IReadOnlyDictionary<string,string> so the overrides can use the readonly interface, and ensure the field types and property signatures match that IReadOnlyDictionary<string,string> type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@WFInfo/Data.cs`:
- Around line 1206-1237: RemoveBlueprintTerms fails to strip blueprint words
that are adjacent to punctuation (e.g., " - Plan", "Кресленник: Форма") and
doesn't remove English "Blueprint" in non-English locales; update
RemoveBlueprintTerms to iterate processor.BlueprintRemovals and remove
occurrences of each term when preceded or followed by common punctuation or
punctuation+whitespace (handle patterns like "[:\\-–—]\\s*TERM\\s*$",
"\\s*TERM\\s*[:\\-–—]", and boundary-only cases) using case-insensitive
Regex.Replace, also ensure you always strip the English fallback "Blueprint"
regardless of _settings.Locale (in addition to the existing locale-specific
removals and the Russian "^Черт[её]ж:" handling), then Trim() the result; use
the existing LanguageProcessorFactory.GetCurrentProcessor() and
processor.BlueprintRemovals to find terms and keep the final result.Trim()
behavior.
---
Outside diff comments:
In `@WFInfo/Data.cs`:
- Around line 1049-1113: The localized branch only searches marketItems
(populated by ReloadItems()) which omits many localized ignored-item mappings,
so add a fallback that checks the current language processor's ignored-name
mappings (e.g., processor.IgnoredItemNames or processor.nameData) using the same
normalization and Levenshtein logic before returning unmatched; specifically,
after building marketItemsSnapshot but before concluding this branch, iterate
the processor's IgnoredItemNames/nameData entries, compute
CalculateLevenshteinDistance against normalizedName, apply the same distance
filter (val < storedName.Length * 0.5) and update low, lowest, lowest_unfiltered
and multipleLowest exactly as done for marketItems entries so localized ignored
items (Forma Blueprint, Kuva, Riven Sliver, etc.) are resolved.
---
Nitpick comments:
In `@WFInfo/LanguageProcessing/EnglishLanguageProcessor.cs`:
- Around line 22-35: IgnoredItemNames currently constructs a new
Dictionary<string,string> on each property access in EnglishLanguageProcessor
(and the other language processor classes); change it to use a shared static
readonly backing field (e.g., private static readonly Dictionary<string,string>
_ignoredItemNames) and have the IgnoredItemNames property return that field
instead of creating a new Dictionary each time; update
EnglishLanguageProcessor.IgnoredItemNames (and apply the same change to the
other four processor classes) so the dictionary is allocated once and reused.
In `@WFInfo/LanguageProcessing/EuropeanLanguageProcessor.cs`:
- Around line 86-99: Replace the per-instance dictionary initializations for the
IgnoredItemNames properties in EuropeanLanguageProcessor (all five processor
classes) with shared cached static readonly fields and have the property return
that field; specifically, create private static readonly
IReadOnlyDictionary<string,string> fields (one per processor) initialized once
with the current key/value pairs, then change each override of IgnoredItemNames
to return its corresponding static field (e.g., change the override of
IgnoredItemNames in EuropeanLanguageProcessor to return the newly created static
field). Also update the abstract property declaration in LanguageProcessor
(LanguageProcessor.IgnoredItemNames) from Dictionary<string,string> to
IReadOnlyDictionary<string,string> so the overrides can use the readonly
interface, and ensure the field types and property signatures match that
IReadOnlyDictionary<string,string> type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eec3dfca-4bc0-4298-ba99-786445eae92c
📒 Files selected for processing (15)
WFInfo/Data.csWFInfo/FodyWeavers.xsdWFInfo/LanguageProcessing/ChineseLanguageProcessor.csWFInfo/LanguageProcessing/CyrillicLanguageProcessor.csWFInfo/LanguageProcessing/EnglishLanguageProcessor.csWFInfo/LanguageProcessing/EuropeanLanguageProcessor.csWFInfo/LanguageProcessing/JapaneseLanguageProcessor.csWFInfo/LanguageProcessing/KoreanLanguageProcessor.csWFInfo/LanguageProcessing/LanguageProcessor.csWFInfo/LanguageProcessing/PolishLanguageProcessor.csWFInfo/LanguageProcessing/ThaiLanguageProcessor.csWFInfo/LanguageProcessing/TurkishLanguageProcessor.csWFInfo/Ocr.csWFInfo/Properties/AssemblyInfo.csWFInfo/WFInfo.csproj
dimon222
changed the title
Member
Author
|
@coderabbitai help |
Contributor
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@WFInfo/LanguageProcessing/CyrillicLanguageProcessor.cs`:
- Around line 94-109: The three Ukrainian translations for the "Galariak"
entries in the _ignoredItemNames dictionary are incorrect; update the values for
the keys "Galariak Prime Blueprint", "Galariak Prime Blade", and "Galariak Prime
Handle" to use the correct Ukrainian spelling and hyphenation (e.g.
"Ґаларіак-Прайм", "Ґаларіак-Прайм лезо", "Ґаларіак-Прайм рукоять") within the
_ignoredItemNames initializer so IgnoredItemNames returns the corrected strings;
verify against the in-game Ukrainian display if hyphen placement differs and
adjust accordingly.
- Around line 22-37: The ignored dictionary values in _ignoredItemNames are not
normalized before fuzzy comparison, so CalculateLevenshteinDistance (which calls
LevenshteinDistanceWithPreprocessing) compares raw localizedName entries like
"Чертёж: Форма" against already-normalized OCR names and fails; fix by
normalizing dictionary values with the same NormalizeForPatternMatching logic
before comparison—either apply NormalizeForPatternMatching to each value when
building/returning IgnoredItemNames (or store a pre-normalized parallel
dictionary) or pass a non-null normalizer into
LevenshteinDistanceWithPreprocessing inside
CyrillicLanguageProcessor.CalculateLevenshteinDistance so the localizedName is
normalized the same way prior to computing distance.
In `@WFInfo/LanguageProcessing/EnglishLanguageProcessor.cs`:
- Line 28: In EnglishLanguageProcessor.cs update the ignored-words dictionary to
include the missing key "Ayatan Cyan Star" alongside the existing ["Ayatan Amber
Star"] entry so OCR outputs mapping to that exact string are ignored; locate the
ignored list in the EnglishLanguageProcessor class (the dictionary that contains
["Ayatan Amber Star"] = "Ayatan Amber Star") and add an analogous entry for
"Ayatan Cyan Star" to prevent Levenshtein fallback matches.
In `@WFInfo/LanguageProcessing/TurkishLanguageProcessor.cs`:
- Around line 22-37: The ignored-item matching fails when OCR loses Turkish
diacritics because IsIgnoredItem (called in Ocr.cs around the
IsIgnoredItem(part) check) compares raw part text against the Turkish-valued
_ignoredItemNames/IgnoredItemNames dictionary using accent-sensitive comparison;
fix by normalizing both sides: either 1) call
TurkishLanguageProcessor.NormalizeTurkishCharacters (or equivalent normalizer
used in GetPartName) on the incoming part string before calling IsIgnoredItem,
or 2) pre-normalize the dictionary values in TurkishLanguageProcessor (transform
_ignoredItemNames to ASCII-folded keys/values) so IgnoredItemNames returns
normalized entries; update IsIgnoredItem usage to consistently compare
normalized strings (use the same normalization routine as in GetPartName).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fb9e9fe-deb2-4b38-9aec-b67e4a8fd004
📒 Files selected for processing (11)
WFInfo/Data.csWFInfo/LanguageProcessing/ChineseLanguageProcessor.csWFInfo/LanguageProcessing/CyrillicLanguageProcessor.csWFInfo/LanguageProcessing/EnglishLanguageProcessor.csWFInfo/LanguageProcessing/EuropeanLanguageProcessor.csWFInfo/LanguageProcessing/JapaneseLanguageProcessor.csWFInfo/LanguageProcessing/KoreanLanguageProcessor.csWFInfo/LanguageProcessing/LanguageProcessor.csWFInfo/LanguageProcessing/PolishLanguageProcessor.csWFInfo/LanguageProcessing/ThaiLanguageProcessor.csWFInfo/LanguageProcessing/TurkishLanguageProcessor.cs
✅ Files skipped from review due to trivial changes (1)
- WFInfo/LanguageProcessing/ThaiLanguageProcessor.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- WFInfo/LanguageProcessing/LanguageProcessor.cs
- WFInfo/LanguageProcessing/KoreanLanguageProcessor.cs
- WFInfo/Data.cs
- WFInfo/LanguageProcessing/ChineseLanguageProcessor.cs
- WFInfo/LanguageProcessing/EuropeanLanguageProcessor.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores