Skip to content

Commit 964e143

Browse files
authored
Merge pull request #138 from burinc/main
fix(galaga): fix collision detection with batched updates
2 parents a1b461d + b56117c commit 964e143

File tree

2 files changed

+174
-48
lines changed

2 files changed

+174
-48
lines changed

src/scittle/games/galaga.clj

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,18 +284,116 @@
284284
;; (/ (+ (:width a) (:width b)) 2))
285285
;; (< (Math/abs (- (:y a) (:y b)))
286286
;; (/ (+ (:height a) (:height b)) 2))))
287-
;;
288-
;; ;; Check bullet-enemy collisions
287+
;; ```
288+
289+
;; ### Critical Bug: Collision Detection Race Conditions
290+
291+
;; During development, we discovered a critical bug that made enemies invulnerable! The problem was subtle but devastating:
292+
293+
;; **The Bug:**
294+
;; ```clojure
295+
;; ;; ❌ BROKEN - Enemies won't die!
289296
;; (doseq [bullet bullets
290297
;; enemy enemies]
291298
;; (when (collides? :a bullet :b enemy)
292-
;; ;; Damage or destroy enemy
293-
;; (let [new-hits (dec (:hits enemy))]
294-
;; (if (<= new-hits 0)
295-
;; (destroy-enemy! :enemy enemy)
296-
;; (damage-enemy! :enemy enemy :hits new-hits)))))
299+
;; ;; Each collision triggers separate swap!
300+
;; (swap! game-state update :bullets remove-bullet)
301+
;; (swap! game-state update :enemies remove-enemy)
302+
;; ...))
297303
;; ```
298304

305+
;; **Two problems here:**
306+
307+
;; 1. **Stale Data**: `bullets` and `enemies` were captured at the start of `update-game!`, but we check collisions AFTER updating bullet positions. So we're checking collisions with OLD positions!
308+
309+
;; 2. **Race Conditions**: Each collision in `doseq` triggers a separate `swap!`, but all iterations use the ORIGINAL collections. If bullet A hits enemies 1 and 2, both iterations try to remove bullet A, causing state corruption.
310+
311+
;; **The Solution: Collision Batching**
312+
313+
;; We adopted the same pattern used in Asteroids (per [Erik Assum](https://github.com/slipset)'s excellent feedback):
314+
315+
;; ```clojure
316+
;; ;; ✅ FIXED - Batch collision detection
317+
;; ;; Get FRESH bullets and enemies after updates
318+
;; (let [current-bullets (:bullets @game-state)
319+
;; current-enemies (:enemies @game-state)
320+
;; hit-bullets (atom #{})
321+
;; hit-enemies (atom #{})
322+
;; damaged-enemies (atom {})
323+
;; score-added (atom 0)
324+
;; new-particles (atom [])]
325+
;;
326+
;; ;; Collect all collisions (but don't apply yet)
327+
;; (doseq [bullet current-bullets
328+
;; :when (not (contains? @hit-bullets bullet))
329+
;; enemy current-enemies
330+
;; :when (and (not (contains? @hit-enemies enemy))
331+
;; (not= (:state enemy) :destroyed))]
332+
;; (when (collides? :a bullet :b enemy)
333+
;; ;; Mark bullet as hit
334+
;; (swap! hit-bullets conj bullet)
335+
;;
336+
;; (let [new-hits (dec (:hits enemy))
337+
;; destroyed? (<= new-hits 0)]
338+
;; (if destroyed?
339+
;; ;; Enemy destroyed - mark for removal
340+
;; (do
341+
;; (swap! hit-enemies conj enemy)
342+
;; (swap! score-added + (:points enemy))
343+
;; (swap! new-particles concat
344+
;; (create-particles :x (:x enemy)
345+
;; :y (:y enemy)
346+
;; :count 10
347+
;; :color (:color enemy))))
348+
;; ;; Enemy damaged - mark for hit count update
349+
;; (swap! damaged-enemies assoc enemy new-hits)))))
350+
;;
351+
;; ;; Apply all collision effects at once (single atomic swap!)
352+
;; (when (or (seq @hit-bullets) (seq @hit-enemies) (seq @damaged-enemies))
353+
;; (when (seq @hit-enemies)
354+
;; (play-explosion-sound))
355+
;;
356+
;; (swap! game-state
357+
;; (fn [state]
358+
;; (-> state
359+
;; ;; Remove hit bullets
360+
;; (update :bullets
361+
;; (fn [bullets]
362+
;; (vec (remove #(contains? @hit-bullets %) bullets))))
363+
;; ;; Remove destroyed enemies and update damaged ones
364+
;; (update :enemies
365+
;; (fn [enemies]
366+
;; (vec (keep (fn [e]
367+
;; (cond
368+
;; ;; Enemy destroyed - remove it
369+
;; (contains? @hit-enemies e)
370+
;; nil
371+
;;
372+
;; ;; Enemy damaged - update hits
373+
;; (contains? @damaged-enemies e)
374+
;; (assoc e :hits (get @damaged-enemies e))
375+
;;
376+
;; ;; Enemy not hit - keep as is
377+
;; :else e))
378+
;; enemies))))
379+
;; ;; Add score for destroyed enemies
380+
;; (update :score + @score-added)
381+
;; ;; Add particles for destroyed enemies
382+
;; (update :particles
383+
;; (fn [particles]
384+
;; (vec (concat particles @new-particles)))))))))
385+
;; ```
386+
387+
;; **Benefits of the fix:**
388+
389+
;; 1. **Fresh Data**: Capture bullets/enemies AFTER position updates
390+
;; 2. **No Race Conditions**: Sets track which objects are already hit
391+
;; 3. **Single Atomic Update**: One `swap!` applies all effects at once
392+
;; 4. **Proper Boss Damage**: Correctly tracks multi-hit enemies
393+
;; 5. **Predictable Behavior**: State updates are deterministic
394+
395+
;; This same issue affected our Asteroids game, and the batched approach solved both!
396+
299397
;; ## Visual Effects
300398

301399
;; ### Particle System

src/scittle/games/galaga.cljs

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -382,47 +382,75 @@
382382
(update :y + (:vy p))
383383
(update :life dec)))
384384
%))))
385-
;; Check bullet-enemy collisions
386-
(doseq [bullet bullets
387-
enemy enemies]
388-
(when (and (collides? :a bullet :b enemy)
389-
(not= (:state enemy) :destroyed))
390-
(let [new-hits (dec (:hits enemy))
391-
destroyed? (<= new-hits 0)]
392-
(when destroyed?
393-
(play-explosion-sound)) ; Play explosion sound
394-
;; Single atomic update for all collision effects
395-
(swap! game-state
396-
(fn [state]
397-
(-> state
398-
;; Remove bullet
399-
(update :bullets #(vec (remove (fn [b] (= b bullet)) %)))
400-
;; Handle enemy destruction or damage
401-
(update :enemies
402-
(fn [enemies]
403-
(if destroyed?
404-
;; Destroy enemy
405-
(vec (remove (fn [e] (= e enemy)) enemies))
406-
;; Damage enemy
407-
(mapv (fn [e]
408-
(if (= e enemy)
409-
(assoc e :hits new-hits)
410-
e))
411-
enemies))))
412-
;; Add score if destroyed
413-
(#(if destroyed?
414-
(update % :score + (:points enemy))
415-
%))
416-
;; Add particles if destroyed
417-
(#(if destroyed?
418-
(update % :particles
419-
(fn [particles]
420-
(vec (concat particles
421-
(create-particles :x (:x enemy)
422-
:y (:y enemy)
423-
:count 10
424-
:color (:color enemy))))))
425-
%))))))))
385+
;; FIXED: Batch bullet-enemy collision detection
386+
;; Get FRESH bullets and enemies after updates
387+
(let [current-bullets (:bullets @game-state)
388+
current-enemies (:enemies @game-state)
389+
hit-bullets (atom #{})
390+
hit-enemies (atom #{})
391+
damaged-enemies (atom {})
392+
score-added (atom 0)
393+
new-particles (atom [])]
394+
395+
;; Collect all collisions (but don't apply yet)
396+
(doseq [bullet current-bullets
397+
:when (not (contains? @hit-bullets bullet))
398+
enemy current-enemies
399+
:when (and (not (contains? @hit-enemies enemy))
400+
(not= (:state enemy) :destroyed))]
401+
(when (collides? :a bullet :b enemy)
402+
;; Mark bullet as hit
403+
(swap! hit-bullets conj bullet)
404+
405+
(let [new-hits (dec (:hits enemy))
406+
destroyed? (<= new-hits 0)]
407+
(if destroyed?
408+
;; Enemy destroyed - mark for removal
409+
(do
410+
(swap! hit-enemies conj enemy)
411+
(swap! score-added + (:points enemy))
412+
(swap! new-particles concat
413+
(create-particles :x (:x enemy)
414+
:y (:y enemy)
415+
:count 10
416+
:color (:color enemy))))
417+
;; Enemy damaged - mark for hit count update
418+
(swap! damaged-enemies assoc enemy new-hits)))))
419+
420+
;; Apply all collision effects at once (single swap!)
421+
(when (or (seq @hit-bullets) (seq @hit-enemies) (seq @damaged-enemies))
422+
(when (seq @hit-enemies)
423+
(play-explosion-sound))
424+
425+
(swap! game-state
426+
(fn [state]
427+
(-> state
428+
;; Remove hit bullets
429+
(update :bullets
430+
(fn [bullets]
431+
(vec (remove #(contains? @hit-bullets %) bullets))))
432+
;; Remove destroyed enemies and update damaged ones
433+
(update :enemies
434+
(fn [enemies]
435+
(vec (keep (fn [e]
436+
(cond
437+
;; Enemy destroyed - remove it
438+
(contains? @hit-enemies e)
439+
nil
440+
441+
;; Enemy damaged - update hits
442+
(contains? @damaged-enemies e)
443+
(assoc e :hits (get @damaged-enemies e))
444+
445+
;; Enemy not hit - keep as is
446+
:else e))
447+
enemies))))
448+
;; Add score for destroyed enemies
449+
(update :score + @score-added)
450+
;; Add particles for destroyed enemies
451+
(update :particles
452+
(fn [particles]
453+
(vec (concat particles @new-particles)))))))))
426454
;; Check enemy bullet-player collisions
427455
(doseq [bullet enemy-bullets]
428456
(when (collides? :a bullet :b player)

0 commit comments

Comments
 (0)