fix: prevent unresponsive behavior on oversized cities#443
fix: prevent unresponsive behavior on oversized cities#443libdefi wants to merge 1 commit intoamilich:mainfrom
Conversation
|
@libdefi is attempting to deploy a commit to the andrew-4640's projects Team on Vercel. A member of the Team first needs to authorize it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }, [multiplayer?.connectionState, multiplayer?.roomCode, multiplayer?.initialState]); | ||
| const m = useMessages(); | ||
| const canExpandCity = gridSize + GRID_EXPANSION_STEP * 2 <= MAX_GRID_SIZE; | ||
| const canShrinkCity = gridSize > MIN_GRID_SIZE; |
There was a problem hiding this comment.
Shrink button enabled when operation will fail
Medium Severity · Logic Bug
canShrinkCity checks gridSize > MIN_GRID_SIZE (i.e. > 50), but shrinkGrid rejects when gridSize - GRID_EXPANSION_STEP * 2 < MIN_GRID_SIZE (i.e. gridSize < 80). For desktop's default grid size of 70, the shrink button appears enabled but the operation silently fails. The same mismatch exists in MobileToolbar and SettingsPanel. The condition needs to account for the full GRID_EXPANSION_STEP * 2 reduction.
Additional Locations (2)
| gridSize: result.newSize, | ||
| services: createEmptyServices(result.newSize), | ||
| speed: 0, // Start paused after forced resize to avoid immediate heavy simulation. | ||
| }; |
There was a problem hiding this comment.
Normalization doesn't update bounds after grid shrink
Medium Severity · Logic Bug
normalizeLoadedGridSize updates grid, gridSize, services, and speed after shrinking, but does not update bounds. Both expandCity and shrinkCity always set bounds to { minX: 0, minY: 0, maxX: newSize - 1, maxY: newSize - 1 } when changing grid size. After forced normalization of an oversized save, bounds will reference coordinates beyond the actual grid dimensions, which could cause out-of-bounds issues downstream.
xkonjin
left a comment
xkonjin
left a comment
There was a problem hiding this comment.
Quick review pass:
- Main risk area here is UI state transitions, empty/error states, and interaction regressions.
- I didn’t see targeted regression coverage in the diff; please add or point CI at a focused test for the changed path in Sidebar.tsx, SettingsPanel.tsx, vehicleSystems.ts (+5 more).
- Before merge, I’d smoke-test the behavior touched by Sidebar.tsx, SettingsPanel.tsx, vehicleSystems.ts (+5 more) with malformed input / retry / rollback cases, since that’s where this class of change usually breaks.
2 participants
Summary
This PR fixes browser unresponsiveness caused by oversized cities by adding hard limits and reducing expensive per-frame work.
Problem
When city size became too large, the app could hit heavy main-thread workloads (simulation + pathfinding + spawn scans), leading to "Page Unresponsive" in the browser.
Changes
MAX_GRID_SIZE = 250MIN_GRID_SIZE = 50GRID_EXPANSION_STEP = 15expandCity()now returnsbooleanBehavioral Notes
Validation
npx tsc --noEmitpasses.Note
Medium Risk
Touches core state loading and grid resize logic, which can affect save compatibility and city integrity if edge cases are missed; changes are scoped but impact critical gameplay state.
Overview
Adds shared grid constraints via
gameLimitsand enforces them end-to-end:expandCity()now returnsboolean, refuses to grow pastMAX_GRID_SIZE, and UI entry points (sidebar, settings, mobile toolbar) disable expand/shrink actions and surface limit messaging.Introduces load-time normalization that automatically shrinks oversized saved/imported states down to the supported maximum (and pauses simulation after forced resizing) to keep legacy/oversized saves playable.
Optimizes
vehicleSystemsspawn logic by caching expensive full-grid finder results pergridVersion, avoiding repeated O(n²) scans during batch spawns; also updatesexpandGrid/shrinkGriddefaults and minimum-size checks to use the shared constants.Written by Cursor Bugbot for commit c9eec72. This will update automatically on new commits. Configure here.