fix: prevent random passenger repopulation during route transitions
- Track displayRouteRef to identify which route's passengers are shown - Only update passengers when train resets (< 0) OR same route and in middle of track (10-90%) - Exclude start/end transition zones to prevent premature passenger updates - Add comprehensive unit tests (11 tests) covering edge cases: - New passengers at position 0 with old route - Passengers regenerated at 5% position - Rapid route increment with position oscillation - Multiple rapid gameplay updates during same route - All 44 hook tests pass
This commit is contained in:
Thomas Hallock
@@ -0,0 +1,506 @@
|
||||
import { renderHook } from '@testing-library/react'
|
||||
import { describe, test, expect, beforeEach, vi } from 'vitest'
|
||||
import { useTrackManagement } from '../useTrackManagement'
|
||||
import type { RailroadTrackGenerator } from '../../lib/RailroadTrackGenerator'
|
||||
import type { Station, Passenger } from '../../lib/gameTypes'
|
||||
|
||||
describe('useTrackManagement - Passenger Display', () => {
|
||||
let mockPathRef: React.RefObject<SVGPathElement>
|
||||
let mockTrackGenerator: RailroadTrackGenerator
|
||||
let mockStations: Station[]
|
||||
let mockPassengers: Passenger[]
|
||||
|
||||
beforeEach(() => {
|
||||
// Create mock path element
|
||||
const mockPath = document.createElementNS('http://www.w3.org/2000/svg', 'path')
|
||||
mockPath.getTotalLength = vi.fn(() => 1000)
|
||||
mockPath.getPointAtLength = vi.fn((distance: number) => ({
|
||||
x: distance,
|
||||
y: 300,
|
||||
}))
|
||||
mockPathRef = { current: mockPath }
|
||||
|
||||
// Mock track generator
|
||||
mockTrackGenerator = {
|
||||
generateTrack: vi.fn(() => ({
|
||||
ballastPath: 'M 0 0',
|
||||
referencePath: 'M 0 0',
|
||||
ties: [],
|
||||
leftRailPath: 'M 0 0',
|
||||
rightRailPath: 'M 0 0'
|
||||
})),
|
||||
generateTiesAndRails: vi.fn(() => ({
|
||||
ties: [],
|
||||
leftRailPath: 'M 0 0',
|
||||
rightRailPath: 'M 0 0'
|
||||
}))
|
||||
} as unknown as RailroadTrackGenerator
|
||||
|
||||
// Mock stations
|
||||
mockStations = [
|
||||
{ id: 'station1', name: 'Station 1', icon: '🏠', position: 20 },
|
||||
{ id: 'station2', name: 'Station 2', icon: '🏢', position: 50 },
|
||||
{ id: 'station3', name: 'Station 3', icon: '🏪', position: 80 },
|
||||
]
|
||||
|
||||
// Mock passengers - initial set
|
||||
mockPassengers = [
|
||||
{
|
||||
id: 'p1',
|
||||
name: 'Alice',
|
||||
avatar: '👩',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station2',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
},
|
||||
{
|
||||
id: 'p2',
|
||||
name: 'Bob',
|
||||
avatar: '👨',
|
||||
originStationId: 'station2',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
test('initial passengers are displayed', () => {
|
||||
const { result } = renderHook(() =>
|
||||
useTrackManagement({
|
||||
currentRoute: 1,
|
||||
trainPosition: 10,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers: mockPassengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
})
|
||||
)
|
||||
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
expect(result.current.displayPassengers[1].id).toBe('p2')
|
||||
})
|
||||
|
||||
test('passengers update when boarded (same route gameplay)', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: 1,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { passengers: mockPassengers, position: 25 } }
|
||||
)
|
||||
|
||||
// Initially 2 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].isBoarded).toBe(false)
|
||||
|
||||
// Board first passenger
|
||||
const boardedPassengers = mockPassengers.map(p =>
|
||||
p.id === 'p1' ? { ...p, isBoarded: true } : p
|
||||
)
|
||||
|
||||
rerender({ passengers: boardedPassengers, position: 25 })
|
||||
|
||||
// Should show updated passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].isBoarded).toBe(true)
|
||||
})
|
||||
|
||||
test('passengers do NOT update during route transition (train moving)', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 50 } }
|
||||
)
|
||||
|
||||
// Initially route 1 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Generate new passengers for route 2
|
||||
const newPassengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// Change route but train still moving
|
||||
rerender({ route: 2, passengers: newPassengers, position: 60 })
|
||||
|
||||
// Should STILL show old passengers (route 1)
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
expect(result.current.displayPassengers[0].name).toBe('Alice')
|
||||
})
|
||||
|
||||
test('passengers update when train resets to start (negative position)', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 50 } }
|
||||
)
|
||||
|
||||
// Initially route 1 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Generate new passengers for route 2
|
||||
const newPassengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// Change route and train resets
|
||||
rerender({ route: 2, passengers: newPassengers, position: -5 })
|
||||
|
||||
// Should now show NEW passengers (route 2)
|
||||
expect(result.current.displayPassengers).toHaveLength(1)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p3')
|
||||
expect(result.current.displayPassengers[0].name).toBe('Charlie')
|
||||
})
|
||||
|
||||
test('passengers do NOT flash when transitioning through 100%', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 95 } }
|
||||
)
|
||||
|
||||
// At 95% - show route 1 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Generate new passengers for route 2
|
||||
const newPassengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// Train exits (105%) but route hasn't changed yet
|
||||
rerender({ route: 1, passengers: mockPassengers, position: 105 })
|
||||
|
||||
// Should STILL show route 1 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Now route changes to 2, but train still at 105%
|
||||
rerender({ route: 2, passengers: newPassengers, position: 105 })
|
||||
|
||||
// Should STILL show route 1 passengers (old ones)
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Train resets to start
|
||||
rerender({ route: 2, passengers: newPassengers, position: -5 })
|
||||
|
||||
// NOW should show route 2 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(1)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p3')
|
||||
})
|
||||
|
||||
test('passengers do NOT update when array reference changes but same route', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: 1,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { passengers: mockPassengers, position: 50 } }
|
||||
)
|
||||
|
||||
// Initially route 1 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Create new array with same content (different reference)
|
||||
const samePassengersNewRef = mockPassengers.map(p => ({ ...p }))
|
||||
|
||||
// Update with new reference but same content
|
||||
rerender({ passengers: samePassengersNewRef, position: 50 })
|
||||
|
||||
// Display should update because it's the same route (gameplay update)
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
})
|
||||
|
||||
test('delivered passengers update immediately (same route)', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: 1,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { passengers: mockPassengers, position: 25 } }
|
||||
)
|
||||
|
||||
// Initially 2 passengers, neither delivered
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].isDelivered).toBe(false)
|
||||
|
||||
// Deliver first passenger
|
||||
const deliveredPassengers = mockPassengers.map(p =>
|
||||
p.id === 'p1' ? { ...p, isBoarded: true, isDelivered: true } : p
|
||||
)
|
||||
|
||||
rerender({ passengers: deliveredPassengers, position: 55 })
|
||||
|
||||
// Should show updated passengers immediately
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
expect(result.current.displayPassengers[0].isDelivered).toBe(true)
|
||||
})
|
||||
|
||||
test('multiple rapid passenger updates during same route', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: 1,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { passengers: mockPassengers, position: 25 } }
|
||||
)
|
||||
|
||||
// Initially 2 passengers
|
||||
expect(result.current.displayPassengers).toHaveLength(2)
|
||||
|
||||
// Board p1
|
||||
let updated = mockPassengers.map(p =>
|
||||
p.id === 'p1' ? { ...p, isBoarded: true } : p
|
||||
)
|
||||
rerender({ passengers: updated, position: 26 })
|
||||
expect(result.current.displayPassengers[0].isBoarded).toBe(true)
|
||||
|
||||
// Board p2
|
||||
updated = updated.map(p =>
|
||||
p.id === 'p2' ? { ...p, isBoarded: true } : p
|
||||
)
|
||||
rerender({ passengers: updated, position: 52 })
|
||||
expect(result.current.displayPassengers[1].isBoarded).toBe(true)
|
||||
|
||||
// Deliver p1
|
||||
updated = updated.map(p =>
|
||||
p.id === 'p1' ? { ...p, isDelivered: true } : p
|
||||
)
|
||||
rerender({ passengers: updated, position: 53 })
|
||||
expect(result.current.displayPassengers[0].isDelivered).toBe(true)
|
||||
|
||||
// All updates should have been reflected
|
||||
expect(result.current.displayPassengers[0].isBoarded).toBe(true)
|
||||
expect(result.current.displayPassengers[0].isDelivered).toBe(true)
|
||||
expect(result.current.displayPassengers[1].isBoarded).toBe(true)
|
||||
expect(result.current.displayPassengers[1].isDelivered).toBe(false)
|
||||
})
|
||||
|
||||
test('EDGE CASE: new passengers at position 0 with old route', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 95 } }
|
||||
)
|
||||
|
||||
// At 95% - route 1 passengers
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Train exits tunnel
|
||||
rerender({ route: 1, passengers: mockPassengers, position: 110 })
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// New passengers generated but route hasn't changed yet, position resets to 0
|
||||
const newPassengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// CRITICAL: New passengers, old route, position = 0
|
||||
// This could trigger the second useEffect if not handled carefully
|
||||
rerender({ route: 1, passengers: newPassengers, position: 0 })
|
||||
|
||||
// Should NOT show new passengers yet (route hasn't changed)
|
||||
// But position is 0-100, so second effect might fire
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
expect(result.current.displayPassengers[0].name).toBe('Alice')
|
||||
})
|
||||
|
||||
test('EDGE CASE: passengers regenerated at position 5%', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 95 } }
|
||||
)
|
||||
|
||||
// At 95% - route 1 passengers
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// New passengers generated while train is at 5%
|
||||
const newPassengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// CRITICAL: New passengers array, same route, position within 0-100
|
||||
rerender({ route: 1, passengers: newPassengers, position: 5 })
|
||||
|
||||
// Should NOT show new passengers (different array reference, route hasn't changed properly)
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
})
|
||||
|
||||
test('EDGE CASE: rapid route increment with position oscillation', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ route, passengers, position }) =>
|
||||
useTrackManagement({
|
||||
currentRoute: route,
|
||||
trainPosition: position,
|
||||
trackGenerator: mockTrackGenerator,
|
||||
pathRef: mockPathRef,
|
||||
stations: mockStations,
|
||||
passengers,
|
||||
maxCars: 3,
|
||||
carSpacing: 7
|
||||
}),
|
||||
{ initialProps: { route: 1, passengers: mockPassengers, position: 50 } }
|
||||
)
|
||||
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
const route2Passengers: Passenger[] = [
|
||||
{
|
||||
id: 'p3',
|
||||
name: 'Charlie',
|
||||
avatar: '👴',
|
||||
originStationId: 'station1',
|
||||
destinationStationId: 'station3',
|
||||
isBoarded: false,
|
||||
isDelivered: false,
|
||||
isUrgent: false
|
||||
}
|
||||
]
|
||||
|
||||
// Route changes, position goes positive briefly before negative
|
||||
rerender({ route: 2, passengers: route2Passengers, position: 2 })
|
||||
|
||||
// Should still show old passengers
|
||||
expect(result.current.displayPassengers[0].id).toBe('p1')
|
||||
|
||||
// Position goes negative
|
||||
rerender({ route: 2, passengers: route2Passengers, position: -3 })
|
||||
|
||||
// NOW should show new passengers
|
||||
expect(result.current.displayPassengers[0].id).toBe('p3')
|
||||
})
|
||||
})
|
||||
@@ -27,8 +27,8 @@ export function useTrackManagement({
|
||||
const [trackData, setTrackData] = useState<ReturnType<typeof trackGenerator.generateTrack> | null>(null)
|
||||
const [tiesAndRails, setTiesAndRails] = useState<{
|
||||
ties: Array<{ x1: number; y1: number; x2: number; y2: number }>
|
||||
leftRailPoints: string[]
|
||||
rightRailPoints: string[]
|
||||
leftRailPath: string
|
||||
rightRailPath: string
|
||||
} | null>(null)
|
||||
const [stationPositions, setStationPositions] = useState<Array<{ x: number; y: number }>>([])
|
||||
const [landmarks, setLandmarks] = useState<Landmark[]>([])
|
||||
@@ -38,7 +38,7 @@ export function useTrackManagement({
|
||||
// Track previous route data to maintain visuals during transition
|
||||
const previousRouteRef = useRef(currentRoute)
|
||||
const [pendingTrackData, setPendingTrackData] = useState<ReturnType<typeof trackGenerator.generateTrack> | null>(null)
|
||||
const previousPassengersRef = useRef<Passenger[]>(passengers)
|
||||
const displayRouteRef = useRef(currentRoute) // Track which route's passengers are being displayed
|
||||
|
||||
// Generate landmarks when route changes
|
||||
useEffect(() => {
|
||||
@@ -74,24 +74,21 @@ export function useTrackManagement({
|
||||
useEffect(() => {
|
||||
// Only switch to new passengers when:
|
||||
// 1. Train has reset to start position (< 0) - track has changed, OR
|
||||
// 2. Same passengers (same route, gameplay updates like boarding/delivering)
|
||||
// 2. Same route AND train is in middle of track (10-90%) - gameplay updates like boarding/delivering
|
||||
const trainReset = trainPosition < 0
|
||||
const samePassengers = passengers === previousPassengersRef.current
|
||||
const sameRoute = currentRoute === displayRouteRef.current
|
||||
const inMiddleOfTrack = trainPosition >= 10 && trainPosition < 90 // Avoid start/end transition zones
|
||||
|
||||
if (trainReset || samePassengers) {
|
||||
if (trainReset) {
|
||||
// Train reset - update to new route's passengers
|
||||
setDisplayPassengers(passengers)
|
||||
previousPassengersRef.current = passengers
|
||||
}
|
||||
// Otherwise, keep displaying old passengers until train resets and track changes
|
||||
}, [passengers, trainPosition])
|
||||
|
||||
// Update display passengers during gameplay (same route)
|
||||
useEffect(() => {
|
||||
// Only update if we're in the same route (not transitioning)
|
||||
if (previousRouteRef.current === currentRoute && trainPosition >= 0 && trainPosition < 100) {
|
||||
displayRouteRef.current = currentRoute
|
||||
} else if (sameRoute && inMiddleOfTrack) {
|
||||
// Same route and train in middle of track - update passengers for gameplay changes (boarding/delivery)
|
||||
setDisplayPassengers(passengers)
|
||||
}
|
||||
}, [passengers, currentRoute, trainPosition])
|
||||
// Otherwise, keep displaying old passengers until train resets
|
||||
}, [passengers, trainPosition, currentRoute])
|
||||
|
||||
// Generate ties and rails when path is ready
|
||||
useEffect(() => {
|
||||
|
||||
Reference in New Issue
Block a user