Update terror zone configuration to support new expansion JSON format#63
Conversation
Support the new "Reign of the Warlock" expansion JSON format where zone IDs changed from numeric (1-36) to string identifiers (e.g., "Act1-BurialGrounds"). Key changes: - Changed TerrorZone.id and Settings.terrorZoneConfig from number to string types - Updated terrorZoneNames mapping to use string zone IDs - Added NUMERIC_TO_STRING_ZONE_ID mapping for backward compatibility - Updated terrorZoneService to handle new JSON structure with type and name fields - Added automatic migration in settings.ts to convert old numeric configs to string IDs - Fixed Base UI module imports to resolve test environment issues - Fixed non-null assertion warnings in test files Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Add per-setting-key warning deduplication to prevent repeated "[object Object]" warnings during application startup. Also add automatic cleanup of corrupted settings from the database. Changes: - Add warnedKeys Set to track which settings have been warned about - Modify parseJSON to accept optional settingKey parameter - Update all parseJSONSetting calls to pass setting key name - Add cleanupCorruptedSettings function to delete corrupted settings - Call cleanup during database schema initialization - Import eq from drizzle-orm for proper where clause syntax This ensures users only see one warning per corrupted setting instead of repeated spam, and corrupted settings are automatically cleaned up on app start. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add conversion from legacy numeric zone IDs (1-36) to string zone IDs (e.g., "Act1-BurialGrounds") to properly display zone names in the UI. Previously, numeric IDs would fall back to "Zone X" display. Changes: - Update readZonesFromFile to detect numeric vs string zone IDs - Convert numeric IDs using NUMERIC_TO_STRING_ZONE_ID mapping - Assign proper human-readable names from TERROR_ZONE_NAMES - Add tests for numeric ID conversion and unknown ID handling - Update createZonesJson test helper to accept numeric IDs This ensures users see proper zone names like "Burial Grounds, Crypt, and Mausoleum" instead of "Zone 1" in the terror zone configuration UI. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update NUMERIC_TO_STRING_ZONE_ID to use 0-based indexing that matches the actual zone indices in desecratedzones.json. Create exhaustive mapping from 0-36 with no gaps. Key fixes: - Zone 18: 'Act2-AncientTunnels' → 'Act2-TalRashas' - Zone 28: 'Act4-CityOfTheDamned' → 'Act4-ChaosSanctuary' - Add missing zones 10, 12, 13 for backward compatibility: - Zone 10: 'Act1-StonyField' - Zone 12: 'Act1-BlackMarsh' - Zone 13: 'Act1-Pit' - Changed from 1-based (1-36) to 0-based (0-36) indexing - Updated terrorZoneService test to use correct zone IDs This fixes the issue where zones 18, 28, and others were showing as "Zone X" instead of their actual names in the terror zone configuration. All zone indices now have proper mappings to display correct names. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
0b20ecc to
e13b96a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e13b96af4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 5: 'Act1-BloodMoor', | ||
| 6: 'Act1-Jail', | ||
| 7: 'Act1-MooMooFarm', | ||
| 8: 'Act1-Tristram', |
There was a problem hiding this comment.
Map legacy numeric zone IDs by ID, not by array order
migrateTerrorZoneConfig in electron/database/settings.ts depends on NUMERIC_TO_STRING_ZONE_ID, but this table reassigns legacy ID 8 to Act1-Tristram even though prior numeric IDs in this codebase used 8 for Stony Field (and 12 for Tristram). That means upgrades with existing numeric terrorZoneConfig will silently disable/enable the wrong zones after migration. The conversion table needs to preserve the old numeric ID semantics, not the new array position.
Useful? React with 👍 / 👎.
| 32: 'Act5-AncientsWay', | ||
| 33: 'Act5-FrozenTundra', | ||
| 34: 'Act5-WorldstoneKeep', | ||
| }; |
There was a problem hiding this comment.
Add mappings for legacy terror zone IDs 35 and 36
The migration map ends at 34, but previous persisted configs could contain IDs 35 and 36. During migrateTerrorZoneConfig, those keys are left as '35'/'36', which do not match any current string zone IDs, so user preferences for those zones are dropped and they revert to defaults after upgrade. Include explicit conversions (or a deterministic fallback) for all legacy IDs to avoid silent data loss.
Useful? React with 👍 / 👎.
Summary
Updates the terror zone configuration feature to support the new "Reign of the Warlock" expansion JSON format where zone IDs changed from numeric (1-36) to string identifiers (e.g.,
Act1-BurialGrounds).Key Changes
Terror Zone Configuration
TerrorZone.idfromnumbertostringSettings.terrorZoneConfigfromRecord<number, boolean>toRecord<string, boolean>NUMERIC_TO_STRING_ZONE_IDmapping for backward compatibility migrationterrorZoneServiceto handle new JSON structure withtypeandnamewrapper fieldsAutomatic Migration
migrateTerrorZoneConfig()function inelectron/database/settings.tsCode Quality Improvements
element!with proper null checks:expect(element).toBeTruthy(); element as TypeItemCard.test.tsx,ItemDetailsDialog.test.tsx,RunList.test.tsxZone ID Mapping
All 36 terror zones now use string identifiers:
Act1-BurialGrounds,Act1-Catacombs,Act1-ColdPlains, etc.Act2-Sewers,Act2-RockyWaste,Act2-DryHills, etc.Act3-SpiderForest,Act3-GreatMarsh,Act3-FlayerJungle, etc.Act4_OuterSteppes,Act4-RiverOfFlame,Act4-ChaosSanctuaryAct5-BloodyFoothils,Act5-ArreatPlateau,Act5-CrystallinePassage, etc.Test plan
bun run typecheck)bun run format)bun run lint)bun run check)bun run test:run)🤖 Generated with Claude Code