Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 57 minutes and 42 seconds. ⌛ 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 ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR refactors the image processing pipeline in the build system. It simplifies image naming by removing hash-based collision resolution in the parser and replaces filename deduplication tracking with a hash-keyed processed file map in the build process. The JSON generation control flow is restructured to separate merging category data from writing output files, and the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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. Comment |
82ac6b3 to
d785728
Compare
845d0ee to
1db7259
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build/build.ts (1)
184-188:⚠️ Potential issue | 🔴 CriticalKeep the image-name reconciliation pass even when the manifest hash is unchanged.
saveImage()now mutatesitem.imageNamefor duplicate hashes and filename conflicts, but Line 188 returns before any of that runs when only non-manifest data changed.saveJson()then re-emits the parser's raw names, so duplicate items can point at files that were never written. Let the loop run and rely on the per-itemcached?.hash !== hashcheck to skip actual downloads.Proposed fix
async saveImages(items: Item[], manifest: ImageManifest): Promise<void> { - // No need to go through every item if the manifest didn't change. I'm - // guessing the `fileTime` key in each element works more or less like a - // hash, so any change to that changes the hash of the full thing. - if (!hashManager.hasChanged('Manifest')) return; const bar = new Progress('Fetching Images', items.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build/build.ts` around lines 184 - 188, The early return in saveImages prevents the image-name reconciliation that saveImage performs, so remove or alter the "if (!hashManager.hasChanged('Manifest')) return;" logic in saveImages to always iterate items; instead, only skip the actual download per-item by relying on the existing per-item check (cached?.hash !== hash) inside saveImage. Ensure saveImages still calls saveImage for every Item to allow mutation of item.imageName (resolving duplicate hashes/filename conflicts) while letting saveImage decide whether to download based on cached?.hash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/build.ts`:
- Around line 257-268: The collision check currently treats any existing file as
a collision (using existsSync(filePath)), which can rename the current item's
own previous output and break later hash checks; modify the condition so we only
treat it as a collision when the on-disk file is owned by a different item—e.g.
check ownership via cached?.uniqueName or the processed[hash] mapping instead of
existsSync alone: only enter the rename branch when existsSync(filePath) &&
cached?.uniqueName !== item.uniqueName (or processed[hash] !== item.imageName),
leaving the file untouched if the existing file belongs to the same uniqueName,
and ensure the subsequent cached/hash logic still prevents unnecessary writes
when the hash matches.
In `@build/parser.ts`:
- Around line 575-588: In addImageName(), normalize backslashes on
image.textureLocation (same as saveImage() uses) before splitting: const
imageStub = image.textureLocation.replace(/\\/g, '/'); then derive uniqueName =
imageStub.split('!')[0] and treat an empty string as invalid (if (!uniqueName) {
warnings.missingImage.push(item.name); return; }). Next get imageName =
uniqueName.split('/').reverse()[0] and reject empty imageName similarly. After
calling sanitize(imageName), validate the sanitized result is non-empty before
assigning item.imageName = sanitized; otherwise push to warnings.missingImage
and return. Reference: addImageName(), image.textureLocation, saveImage(),
sanitize(), warnings.missingImage, item.imageName.
---
Outside diff comments:
In `@build/build.ts`:
- Around line 184-188: The early return in saveImages prevents the image-name
reconciliation that saveImage performs, so remove or alter the "if
(!hashManager.hasChanged('Manifest')) return;" logic in saveImages to always
iterate items; instead, only skip the actual download per-item by relying on the
existing per-item check (cached?.hash !== hash) inside saveImage. Ensure
saveImages still calls saveImage for every Item to allow mutation of
item.imageName (resolving duplicate hashes/filename conflicts) while letting
saveImage decide whether to download based on cached?.hash.
🪄 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: b8d4d278-bf7b-4e7c-9653-5f4058e22dd8
⛔ Files ignored due to path filters (296)
data/img/10YearAnniversaryLoginSongItemStoreIcon.pngis excluded by!**/*.pngdata/img/13angTV.pngis excluded by!**/*.pngdata/img/1999Aoi.pngis excluded by!**/*.pngdata/img/1999BoybandPoster.pngis excluded by!**/*.pngdata/img/1999ComicCoverPoster.pngis excluded by!**/*.pngdata/img/1999CommunityARGEmblem.pngis excluded by!**/*.pngdata/img/1999Drippy.pngis excluded by!**/*.pngdata/img/1999EntHybridPistol.pngis excluded by!**/*.pngdata/img/1999FlareBandPoster.pngis excluded by!**/*.pngdata/img/1999InfShotgunWeapon.pngis excluded by!**/*.pngdata/img/1999InfSporePistolWeapon.pngis excluded by!**/*.pngdata/img/1999OnlyneWallPoster.pngis excluded by!**/*.pngdata/img/1999Pizza.pngis excluded by!**/*.pngdata/img/1999PrologueQuestKeyChain.pngis excluded by!**/*.pngdata/img/1999QuestKeyChain.pngis excluded by!**/*.pngdata/img/1999ResourceCommonA.pngis excluded by!**/*.pngdata/img/1999ResourceCommonAContainer.pngis excluded by!**/*.pngdata/img/1999ResourceCommonB.pngis excluded by!**/*.pngdata/img/1999ResourceCommonBContainer.pngis excluded by!**/*.pngdata/img/1999ResourceDefense.pngis excluded by!**/*.pngdata/img/1999ResourceRareA.pngis excluded by!**/*.pngdata/img/1999ResourceRareAContainer.pngis excluded by!**/*.pngdata/img/1999ResourceUncommonA.pngis excluded by!**/*.pngdata/img/1999ResourceUncommonAContainer.pngis excluded by!**/*.pngdata/img/1999ResourceUncommonB.pngis excluded by!**/*.pngdata/img/1999ResourceUncommonBContainer.pngis excluded by!**/*.pngdata/img/1999TankHoodOrnament.pngis excluded by!**/*.pngdata/img/1999TrophyBronze.pngis excluded by!**/*.pngdata/img/1999Y2KEarB.pngis excluded by!**/*.pngdata/img/1999Y2KEarC.pngis excluded by!**/*.pngdata/img/1999Y2KEyeA.pngis excluded by!**/*.pngdata/img/1999Y2KEyeB.pngis excluded by!**/*.pngdata/img/1999Y2KEyeC.pngis excluded by!**/*.pngdata/img/2020ZerOGlyph.pngis excluded by!**/*.pngdata/img/2025GCX.pngis excluded by!**/*.pngdata/img/4MORI4N.pngis excluded by!**/*.pngdata/img/7thAnniversaryPoster.pngis excluded by!**/*.pngdata/img/8thAnniversaryPoster.pngis excluded by!**/*.pngdata/img/A.pngis excluded by!**/*.pngdata/img/AGGP.pngis excluded by!**/*.pngdata/img/AHCommonPickup.pngis excluded by!**/*.pngdata/img/AHR.pngis excluded by!**/*.pngdata/img/AHRarePickup.pngis excluded by!**/*.pngdata/img/AHUncommonPickup.pngis excluded by!**/*.pngdata/img/AK47Weapon.pngis excluded by!**/*.pngdata/img/AVReceiver.pngis excluded by!**/*.pngdata/img/AbacusSmall.pngis excluded by!**/*.pngdata/img/AbberaPrime.pngis excluded by!**/*.pngdata/img/AbilityBlock.jpgis excluded by!**/*.jpgdata/img/AbilityDurationMod.jpgis excluded by!**/*.jpgdata/img/AbilityEfficiencyMod.jpgis excluded by!**/*.jpgdata/img/AbilityRangeMod.jpgis excluded by!**/*.jpgdata/img/AbilityStrengthMod.jpgis excluded by!**/*.jpgdata/img/Abrasys.pngis excluded by!**/*.pngdata/img/AbyssOfDagathLoginSongItemStoreIcon.pngis excluded by!**/*.pngdata/img/AcceltraDeluxeIISkin.pngis excluded by!**/*.pngdata/img/AcceltraDeluxeSkin.pngis excluded by!**/*.pngdata/img/AcceltraPrime.pngis excluded by!**/*.pngdata/img/AccessibleGamer.pngis excluded by!**/*.pngdata/img/AccuracyWhileAiming.jpgis excluded by!**/*.jpgdata/img/AckAndBrunt.pngis excluded by!**/*.pngdata/img/AckAndBruntConclave.pngis excluded by!**/*.pngdata/img/AckAndBruntDayOfTheDead.pngis excluded by!**/*.pngdata/img/AckAndBruntNigtwave.pngis excluded by!**/*.pngdata/img/AckBruntIncarnonAdapter.pngis excluded by!**/*.pngdata/img/AckBruntNightwatchMod.jpgis excluded by!**/*.jpgdata/img/AcolyteSynpai.pngis excluded by!**/*.pngdata/img/Acrid.jpgis excluded by!**/*.jpgdata/img/Acrid.pngis excluded by!**/*.pngdata/img/ActivateDeimosRequiemTotem.pngis excluded by!**/*.pngdata/img/Actuator.pngis excluded by!**/*.pngdata/img/AdelfosSelene.pngis excluded by!**/*.pngdata/img/AdikDarkCero.pngis excluded by!**/*.pngdata/img/AdmiralBahroo.pngis excluded by!**/*.pngdata/img/AdrenalStim.pngis excluded by!**/*.pngdata/img/AdultHeadARemastered.pngis excluded by!**/*.pngdata/img/AdultHeadBRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadCRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadDRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadERemastered.pngis excluded by!**/*.pngdata/img/AdultHeadFRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleA.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleB.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleC.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleD.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleE.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleF.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleG.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleH.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleI.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleJ.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleK.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleL.pngis excluded by!**/*.pngdata/img/AdultHeadFemaleM.pngis excluded by!**/*.pngdata/img/AdultHeadGRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadHRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadIRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadJRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadKRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadLRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadMRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadMaleA.pngis excluded by!**/*.pngdata/img/AdultHeadMaleB.pngis excluded by!**/*.pngdata/img/AdultHeadMaleC.pngis excluded by!**/*.pngdata/img/AdultHeadMaleD.pngis excluded by!**/*.pngdata/img/AdultHeadMaleE.pngis excluded by!**/*.pngdata/img/AdultHeadMaleF.pngis excluded by!**/*.pngdata/img/AdultHeadMaleG.pngis excluded by!**/*.pngdata/img/AdultHeadMaleH.pngis excluded by!**/*.pngdata/img/AdultHeadMaleI.pngis excluded by!**/*.pngdata/img/AdultHeadMaleJ.pngis excluded by!**/*.pngdata/img/AdultHeadMaleK.pngis excluded by!**/*.pngdata/img/AdultHeadMaleL.pngis excluded by!**/*.pngdata/img/AdultHeadMaleM.pngis excluded by!**/*.pngdata/img/AdultHeadNRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadORemastered.pngis excluded by!**/*.pngdata/img/AdultHeadPRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadQRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadRRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadSRemastered.pngis excluded by!**/*.pngdata/img/AdultHeadTRemastered.pngis excluded by!**/*.pngdata/img/AdultOperatorAgile.pngis excluded by!**/*.pngdata/img/AdultOperatorNoble.pngis excluded by!**/*.pngdata/img/AeonKnight.pngis excluded by!**/*.pngdata/img/AesopYOLIANGlyph.pngis excluded by!**/*.pngdata/img/Afuris.pngis excluded by!**/*.pngdata/img/AfurisForestCamoSkin.pngis excluded by!**/*.pngdata/img/AfurisPrime.pngis excluded by!**/*.pngdata/img/Agkuza.pngis excluded by!**/*.pngdata/img/AimGlide.pngis excluded by!**/*.pngdata/img/AirHockey.pngis excluded by!**/*.pngdata/img/AirSlideBoost.jpgis excluded by!**/*.jpgdata/img/AirSupportCarpetBomb.pngis excluded by!**/*.pngdata/img/AirSupportGrineer.pngis excluded by!**/*.pngdata/img/AirSupportLiset.pngis excluded by!**/*.pngdata/img/AirSupportMantis.pngis excluded by!**/*.pngdata/img/AirSupportNightwave.pngis excluded by!**/*.pngdata/img/AirSupportRareInstinct.pngis excluded by!**/*.pngdata/img/AirSupportSentryTurret.pngis excluded by!**/*.pngdata/img/Ajingom.pngis excluded by!**/*.pngdata/img/AkSura.pngis excluded by!**/*.pngdata/img/Akariayataka.pngis excluded by!**/*.pngdata/img/AkariusPrime.pngis excluded by!**/*.pngdata/img/Akbolto.pngis excluded by!**/*.pngdata/img/AkboltoOrmolu.pngis excluded by!**/*.pngdata/img/AkboltoPrime.pngis excluded by!**/*.pngdata/img/Akbronco.pngis excluded by!**/*.pngdata/img/AkbroncoPrime.pngis excluded by!**/*.pngdata/img/AkbroncoPrimeViralMod.jpgis excluded by!**/*.jpgdata/img/Akjagara.pngis excluded by!**/*.pngdata/img/AkjagaraIridosSkin.pngis excluded by!**/*.pngdata/img/AkjagaraPrime.pngis excluded by!**/*.pngdata/img/Aklato.pngis excluded by!**/*.pngdata/img/AklatoConclaveSkin.pngis excluded by!**/*.pngdata/img/AklatoDayOfTheDead.pngis excluded by!**/*.pngdata/img/AklatoKintsugi.pngis excluded by!**/*.pngdata/img/AklatoNocturne.pngis excluded by!**/*.pngdata/img/AklatoRixtyMOLSkin.pngis excluded by!**/*.pngdata/img/Aklex.pngis excluded by!**/*.pngdata/img/AklexConclaveSkin.pngis excluded by!**/*.pngdata/img/AklexPrime.pngis excluded by!**/*.pngdata/img/Akmagnus.pngis excluded by!**/*.pngdata/img/AkmagnusDakila.pngis excluded by!**/*.pngdata/img/AkmagnusHiveLight.pngis excluded by!**/*.pngdata/img/AkmagnusObsidian.pngis excluded by!**/*.pngdata/img/AkmagnusPrime.pngis excluded by!**/*.pngdata/img/Akrabu.pngis excluded by!**/*.pngdata/img/AkrabuPrime.pngis excluded by!**/*.pngdata/img/Aksomati.pngis excluded by!**/*.pngdata/img/AksomatiPrime.pngis excluded by!**/*.pngdata/img/Akstileto.pngis excluded by!**/*.pngdata/img/AkstiletoPrime.pngis excluded by!**/*.pngdata/img/AkstilettoConclaveSkin.pngis excluded by!**/*.pngdata/img/AkstilettoPrimeAmmoEfficiencyMod.jpgis excluded by!**/*.jpgdata/img/Akvasto.pngis excluded by!**/*.pngdata/img/AkvastoConclaveSkin.pngis excluded by!**/*.pngdata/img/AkvastoDayOfTheDeadSkin.pngis excluded by!**/*.pngdata/img/AkvastoForestCamoSkin.pngis excluded by!**/*.pngdata/img/AkvastoPrime.pngis excluded by!**/*.pngdata/img/AkvastoVoidSkin.pngis excluded by!**/*.pngdata/img/Akzani.pngis excluded by!**/*.pngdata/img/AladV.pngis excluded by!**/*.pngdata/img/AlainLove.pngis excluded by!**/*.pngdata/img/Albrecht2in1Display.pngis excluded by!**/*.pngdata/img/AlbrechtHatCommunityGlyph.pngis excluded by!**/*.pngdata/img/AlbrechtPortrait.pngis excluded by!**/*.pngdata/img/AlchemistAgile.pngis excluded by!**/*.pngdata/img/AlchemistDistill.pngis excluded by!**/*.pngdata/img/AlchemistNoble.pngis excluded by!**/*.pngdata/img/AlchemistRush.pngis excluded by!**/*.pngdata/img/AlchemistSerpent.pngis excluded by!**/*.pngdata/img/AlchemistTransmuteAugmentCard.jpgis excluded by!**/*.jpgdata/img/AlchemistTransmuter.pngis excluded by!**/*.pngdata/img/Alertium.pngis excluded by!**/*.pngdata/img/AlexanderDario.pngis excluded by!**/*.pngdata/img/AlexandraLive.pngis excluded by!**/*.pngdata/img/AlloyPlate.pngis excluded by!**/*.pngdata/img/AlternoxDeluxeSkin.pngis excluded by!**/*.pngdata/img/AlternoxPrime.pngis excluded by!**/*.pngdata/img/Althani.pngis excluded by!**/*.pngdata/img/Altra.pngis excluded by!**/*.pngdata/img/AltraPrime.pngis excluded by!**/*.pngdata/img/Alyekk.pngis excluded by!**/*.pngdata/img/AmalgamArgonak.jpgis excluded by!**/*.jpgdata/img/AmalgamDaikyu.jpgis excluded by!**/*.jpgdata/img/AmalgamEventBadge.pngis excluded by!**/*.pngdata/img/AmalgamFurax.jpgis excluded by!**/*.jpgdata/img/AmalgamJavlok.jpgis excluded by!**/*.jpgdata/img/AmalgamRipkas.jpgis excluded by!**/*.jpgdata/img/AmarExilusMod.jpgis excluded by!**/*.jpgdata/img/AmarHeader.pngis excluded by!**/*.pngdata/img/AmarMeleeMod.jpgis excluded by!**/*.jpgdata/img/AmarWarframeMod.jpgis excluded by!**/*.jpgdata/img/AmaruChromaAvatarBright.pngis excluded by!**/*.pngdata/img/AmaruChromaAvatarDark.pngis excluded by!**/*.pngdata/img/AmazingBuriGlyph.pngis excluded by!**/*.pngdata/img/AmazonOni.pngis excluded by!**/*.pngdata/img/Ambulas.pngis excluded by!**/*.pngdata/img/AmbulasDataFragment.pngis excluded by!**/*.pngdata/img/AmbulasEventBadge.pngis excluded by!**/*.pngdata/img/AmbulasEvent_e.pngis excluded by!**/*.pngdata/img/Amesha.pngis excluded by!**/*.pngdata/img/AmethystAntitoxin.pngis excluded by!**/*.pngdata/img/AmirAccoladeGlyph.pngis excluded by!**/*.pngdata/img/AmirPixelGlyph.pngis excluded by!**/*.pngdata/img/AmirValentineGlyph.pngis excluded by!**/*.pngdata/img/Amphis.pngis excluded by!**/*.pngdata/img/Amphor.pngis excluded by!**/*.pngdata/img/Amprex.pngis excluded by!**/*.pngdata/img/AmprexDayofTheDeadSkin.pngis excluded by!**/*.pngdata/img/AnJetCat.pngis excluded by!**/*.pngdata/img/AnaviIvy.pngis excluded by!**/*.pngdata/img/AngelsOfTheZarimanQuestKeychain.pngis excluded by!**/*.pngdata/img/AngelsOfTheZarimanSongItemStoreIcon.pngis excluded by!**/*.pngdata/img/AngryIceberg.pngis excluded by!**/*.pngdata/img/AngryUnicorn.pngis excluded by!**/*.pngdata/img/Angstrum.pngis excluded by!**/*.pngdata/img/AngstrumConclave.pngis excluded by!**/*.pngdata/img/AngstrumDayOfTheDead.pngis excluded by!**/*.pngdata/img/AngstrumIncarnonAdapter.pngis excluded by!**/*.pngdata/img/AnimaAlt2Helmet.pngis excluded by!**/*.pngdata/img/AnimaAspect.pngis excluded by!**/*.pngdata/img/AnimalInstincts.jpgis excluded by!**/*.jpgdata/img/AnimalLure.pngis excluded by!**/*.pngdata/img/AnimalTagBolarolaCommon.pngis excluded by!**/*.pngdata/img/AnimalTagBolarolaRare.pngis excluded by!**/*.pngdata/img/AnimalTagBolarolaUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagCondrocCommon.pngis excluded by!**/*.pngdata/img/AnimalTagCondrocRare.pngis excluded by!**/*.pngdata/img/AnimalTagCondrocUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagHorrasqueCommon.pngis excluded by!**/*.pngdata/img/AnimalTagHorrasqueRare.pngis excluded by!**/*.pngdata/img/AnimalTagHorrasqueUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedCritterCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedCritterRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedCritterUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedKDriveCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedKDriveRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedKDriveUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMaggotCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMaggotRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMaggotUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMergooCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMergooRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedMergooUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedPredatorCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedPredatorRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedPredatorUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedThwompCommon.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedThwompRare.pngis excluded by!**/*.pngdata/img/AnimalTagInfestedThwompUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagKuakaCommon.pngis excluded by!**/*.pngdata/img/AnimalTagKuakaRare.pngis excluded by!**/*.pngdata/img/AnimalTagKuakaUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagKubrodonCommon.pngis excluded by!**/*.pngdata/img/AnimalTagKubrodonRare.pngis excluded by!**/*.pngdata/img/AnimalTagKubrodonUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagMergooCommon.pngis excluded by!**/*.pngdata/img/AnimalTagMergooRare.pngis excluded by!**/*.pngdata/img/AnimalTagMergooUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagPobbersCommon.pngis excluded by!**/*.pngdata/img/AnimalTagPobbersRare.pngis excluded by!**/*.pngdata/img/AnimalTagPobbersUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagSawgawCommon.pngis excluded by!**/*.pngdata/img/AnimalTagSawgawRare.pngis excluded by!**/*.pngdata/img/AnimalTagSawgawUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagStoverCommon.pngis excluded by!**/*.pngdata/img/AnimalTagStoverRare.pngis excluded by!**/*.pngdata/img/AnimalTagStoverUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagVampireKavatCommon.pngis excluded by!**/*.pngdata/img/AnimalTagVampireKavatRare.pngis excluded by!**/*.pngdata/img/AnimalTagVampireKavatUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagVirminkCommon.pngis excluded by!**/*.pngdata/img/AnimalTagVirminkRare.pngis excluded by!**/*.pngdata/img/AnimalTagVirminkUncommon.pngis excluded by!**/*.pngdata/img/AnimalTagZongroCommon.pngis excluded by!**/*.png
📒 Files selected for processing (4)
build/build.tsbuild/parser.tsdata/cache/.export.jsondata/cache/.images.json
…shes Some images use the same hash with a different filename. Effectivally the images are the same so they can be reused between different items
720d881 to
f220eac
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
build/build.ts (1)
257-264:⚠️ Potential issue | 🟠 MajorCollision check still triggers for current item's own previous output.
The
existsSync(filePath)check at line 258 fires when any file with that name exists, including the current item's output from a previous run. On incremental builds where the hash hasn't changed:
processed[hash]is empty (fresh run state) → continueexistsSync("texture.png")istrue(from previous run)item.imageNamerenamed to"ItemName.png"cached?.hash === hash→ download skipped"ItemName.png"never created, but JSON points to itTo only rename when a different item owns the file, add an ownership check:
- if (existsSync(filePath)) { + if (existsSync(filePath) && cached?.uniqueName !== item.uniqueName) {This skips the rename when the existing file belongs to the same item, allowing the subsequent cache check to correctly prevent unnecessary re-downloads while keeping
item.imageNamevalid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build/build.ts` around lines 257 - 264, The collision check renames the file whenever it exists, even if the existing file is the same item's previous output; update the existsSync(filePath) branch to detect ownership first (use processed which maps hash->imageName): compute ownerHash = Object.entries(processed).find(([, name]) => name === item.imageName)?.[0] and only perform the rename when ownerHash is undefined or ownerHash !== hash (i.e., a different item owns that filename); otherwise skip renaming so cached?.hash === hash can correctly skip re-download and item.imageName stays valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/build.ts`:
- Around line 120-127: The sort method currently contains a leftover debug
console.log call that prints the Item when a.name is falsy; remove that
console.log(a) from the sort(a: Item, b: Item): number function so the
comparator only uses a.name.localeCompare and falls back to
a.uniqueName.localeCompare when names are equal (no other logic changes needed).
---
Duplicate comments:
In `@build/build.ts`:
- Around line 257-264: The collision check renames the file whenever it exists,
even if the existing file is the same item's previous output; update the
existsSync(filePath) branch to detect ownership first (use processed which maps
hash->imageName): compute ownerHash = Object.entries(processed).find(([, name])
=> name === item.imageName)?.[0] and only perform the rename when ownerHash is
undefined or ownerHash !== hash (i.e., a different item owns that filename);
otherwise skip renaming so cached?.hash === hash can correctly skip re-download
and item.imageName stays valid.
🪄 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: 866b2b98-7580-4ef3-876c-56c5afc1d8ea
📒 Files selected for processing (3)
build/build.tsbuild/parser.tstest/utilities/find.spec.mjs
✅ Files skipped from review due to trivial changes (1)
- test/utilities/find.spec.mjs
AyAyEm
left a comment
There was a problem hiding this comment.
Couldn't find the conflict name resolution for these items:
Octavia.png 2
Mesa.png 2
Ember.png 2
Frost.png 2
Gara.png 2
Mirage.png 2
Mag.png 2
Rhino.png 2
InarosPrime.png 2
EliteSanctuaryOnslaught.png 2
FireExtinguisher.png 2
GenericComponent.png 2
MerulinaPrime.png 2
GenericArchwingSystems.png 2
Amphis.png 2
Imperator.png 2
GenericGear.png 2
AccuracyWhileAiming.jpg 3
CritChanceWhileAiming.jpg 3
CritDamageWhileAiming.jpg 3
FireRateWhileAiming.jpg 3
StatusChanceWhileAiming.jpg 3
WeaponFireIterationsSPMod.jpg 3
WeaponStatusChanceSPMod.jpg 3
WeaponWeakpointCriticalChanceMod.jpg 2
EquinoxAgile.png 2
EquinoxNoble.png 2
AmalgamEventBadge.png 2
GarudaDeluxe.png 2
HydroidDeluxe.png 2
RhinoDeluxe.png 2
Rank10Seeker.png 2
| // Check if the previous image was for a component because they might | ||
| // have different naming schemes like lex-prime | ||
| if (!cached || cached.hash !== hash || cached.isComponent !== isComponent) { | ||
| if (cached?.hash !== hash) { |
There was a problem hiding this comment.
In which situation both values might be undefined here?
| sort(a: Item, b: Item): number { | ||
| if (!a.name) console.log(a); | ||
| const res = a.name.localeCompare(b.name); | ||
| if (res === 0) { | ||
| return a.uniqueName.localeCompare(b.uniqueName); | ||
| } | ||
| return res; | ||
| } | ||
|
|
||
| merge(categories: Record<string, Item[]>): Item[] { | ||
| let all: Item[] = []; | ||
|
|
||
| // Category names are provided by this.applyCustomCategories | ||
| for (const category of Object.keys(categories)) { | ||
| const categoryData = categories[category]; | ||
| if (!categoryData) continue; | ||
| all = all.concat(categoryData); | ||
| } | ||
|
|
||
| // All.json (all items in one file) | ||
| all.sort(this.sort.bind(this)); | ||
|
|
||
| return all; | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this refactor necessary for the images?
|
@SlayerOrnstein i'm totally down for this, but the tests are failing... |
|
Yeah just been picking at the code a bit. Was redoing it so that |
What did you fix?
Reduce duplication by leaning more on the image manifest and hashes.
The idea is to just use the internal name of the image resource as the image name to reduce complexity in
addImageName. This also allowssaveImageto be smarter about reusing image names when a duplicate hash is found in thetextureLocation(hashes being the same means the image is the same, even if the filenames are different). This also allowssaveImageto properly handle situations where the filename is the same but the resource is different as is the case withEmberand her monotone glyph also being calledEmber.pngin these cases theItemname is used instead.Any updates or last minute changes by
saveImagewill also update the correspondingItemand push writing the json till after the images are downloaded.Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
Bug Fixes
Refactor