Skip to content

unmobius game#13

Merged
Sajjon merged 2 commits into
mainfrom
unmobius_game
Apr 25, 2026
Merged

unmobius game#13
Sajjon merged 2 commits into
mainfrom
unmobius_game

Conversation

@Sajjon
Copy link
Copy Markdown
Owner

@Sajjon Sajjon commented Apr 24, 2026

  • revert usage of Mobius in GameVC
  • introduce SwiftIntro/Views/SingleCellTypeCollectionView; cleanup GameView and GameVC
  • simplify GameVC

Copilot AI review requested due to automatic review settings April 24, 2026 06:21
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 98.64407% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.62%. Comparing base (269c505) to head (dfaf5c2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...wiftIntro/Views/SingleCellTypeCollectionView.swift 89.47% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
- Coverage   98.74%   98.62%   -0.13%     
==========================================
  Files          68       39      -29     
  Lines        3593     1090    -2503     
==========================================
- Hits         3548     1075    -2473     
+ Misses         45       15      -30     
Files with missing lines Coverage Δ
SwiftIntro/App/RootVC.swift 100.00% <100.00%> (ø)
SwiftIntro/Extensions/NSObject_Extension.swift 100.00% <100.00%> (ø)
SwiftIntro/Features/Game/GameVC.swift 100.00% <100.00%> (ø)
SwiftIntro/Features/Game/GameViewModel.swift 100.00% <100.00%> (ø)
...iftIntro/Features/Game/Model/State/GameModel.swift 100.00% <100.00%> (ø)
...ro/Features/Game/Model/Values/CardDuplicates.swift 100.00% <100.00%> (ø)
...Intro/Features/Game/Model/Values/CardSingles.swift 100.00% <ø> (ø)
SwiftIntro/Features/Game/View/CardCVCell.swift 100.00% <ø> (ø)
SwiftIntro/Features/Game/View/CardGridLayout.swift 100.00% <100.00%> (ø)
SwiftIntro/Features/Game/View/GameView.swift 100.00% <100.00%> (ø)
... and 10 more

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the Game feature away from Mobius to a simpler MVVM flow by introducing a GameViewModel, encapsulating the collection view inside GameView, and removing Mobius-related logic, tests, and package dependencies.

Changes:

  • Replace Mobius loop/effects/events with GameViewModel callbacks (onModelChanged, onFlipCard, onNavigateToGameOver).
  • Encapsulate the card grid via SingleCellTypeCollectionView and inject data source/delegate into GameView.
  • Update/restructure tests and remove Mobius test suites + Mobius package references.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
SwiftIntro/Features/Game/GameVC.swift Migrates VC to MVVM wiring and lifecycle start/stop of the view model.
SwiftIntro/Features/Game/GameViewModel.swift New state/logic owner for gameplay, timers, and navigation signaling.
SwiftIntro/Features/Game/View/GameView.swift Encapsulates collection view and adds flip animation helper.
SwiftIntro/Features/Game/View/MemoryDataSourceAndDelegate.swift Moves to init-injected closures and aligns docs with MVVM.
SwiftIntro/Views/SingleCellTypeCollectionView.swift New typed collection view wrapper that registers a single cell type.
SwiftIntroTests/Features/Game/GameVCTests.swift Updates VC tests for MVVM lifecycle and closure wiring.
SwiftIntroTests/Features/Game/GameViewModelTests.swift New unit tests covering view model behavior (tap flow, timers, navigation).
SwiftIntroTests/Features/Game/GameModelTests.swift New tests for GameModel/CardModel defaults and derived properties.
SwiftIntroTests/Features/Game/View/MemoryDataSourceAndDelegateTests.swift Updates helper construction for new initializer signature.
SwiftIntroTests/Dependencies/ImmediateClock.swift Extracts reusable test clock for timer-dependent tests.
SwiftIntroTests/Features/Game/Logic/GameLoopTests.swift Removes Mobius loop tests (Mobius removed).
SwiftIntroTests/Features/Game/Logic/GameLogicTests.swift Removes Mobius update-function tests (logic moved to VM).
SwiftIntroTests/Features/Game/Logic/GameEffectTests.swift Removes Mobius effect description tests.
SwiftIntroTests/Features/Game/Logic/GameEffectHandlerTests.swift Removes Mobius effect handler tests (effects removed).
SwiftIntro/Features/Game/Logic/GameLoop.swift Removes Mobius loop orchestration.
SwiftIntro/Features/Game/Logic/GameLogic.swift Removes Mobius update function.
SwiftIntro/Features/Game/Logic/GameEvent.swift Removes Mobius event type.
SwiftIntro/Features/Game/Logic/GameEffect.swift Removes Mobius effect type.
SwiftIntro/Features/Game/Logic/GameEffectHandler.swift Removes Mobius effect executor.
SwiftIntro.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved Drops Mobius (and related) pins from SwiftPM resolution.
SwiftIntro.xcodeproj/project.pbxproj Removes Mobius package references and adds new sources/tests to the project.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cell.animateFlip(faceUp: isFaceUp)
}

/// Updates all model-driven UI — called by the Mobius loop `acceptClosure` on every model update.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment on render(_:) still references the Mobius loop acceptClosure, but the game screen has been migrated to MVVM (GameViewModel + callbacks). Update the comment to reflect the new call site (e.g., GameViewModel.onModelChanged / GameVC.viewModel.start).

Suggested change
/// Updates all model-driven UI — called by the Mobius loop `acceptClosure` on every model update.
/// Updates all model-driven UI — typically invoked from the MVVM update flow
/// (for example via `GameViewModel.onModelChanged` after `GameVC.viewModel.start`).

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +14
// - `_ = vc.view` triggers loadView + viewDidLoad, which wires the view model and
// fires its initial `onModelChanged` callback so the score label is rendered.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These “Notes on approach” are now inaccurate: GameVC no longer overrides viewDidLoad, and the initial onModelChanged is fired from viewWillAppear via viewModel.start(...), not from viewDidLoad. Please update the comment so the tests match the current lifecycle wiring.

Suggested change
// - `_ = vc.view` triggers loadView + viewDidLoad, which wires the view model and
// fires its initial `onModelChanged` callback so the score label is rendered.
// - `_ = vc.view` triggers view loading/setup. The initial `onModelChanged`
// callback is fired later from `viewWillAppear` via `viewModel.start(...)`,
// which is what renders the initial score label state.

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +158
model.clickCount += 1
model.cards[index].isFlipped = true
guard let pendingIndex = model.pendingCardIndex else {
// swiftformat:disable:next redundantSelf
logGame.debug("First card flipped @\(index) — waiting for 2nd 🃏 (click #\(self.model.clickCount))")
model.pendingCardIndex = index
onFlipCard?(index, true)
return
}
logGame
.debug("Second card of pair flipped at index \(index) — evaluating against pending index \(pendingIndex)")
model.pendingCardIndex = nil
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

model is a value type (GameModel is a struct) with a didSet that calls onModelChanged. In applyFlip you mutate several nested fields (clickCount, cards[index], pendingCardIndex), which will trigger didSet (and thus onModelChanged) multiple times per tap, potentially causing extra renders and intermediate UI states. Consider building a local copy (var newModel = model), applying all mutations, then assigning model = newModel once so observers see a single coherent snapshot per user action.

Suggested change
model.clickCount += 1
model.cards[index].isFlipped = true
guard let pendingIndex = model.pendingCardIndex else {
// swiftformat:disable:next redundantSelf
logGame.debug("First card flipped @\(index) — waiting for 2nd 🃏 (click #\(self.model.clickCount))")
model.pendingCardIndex = index
onFlipCard?(index, true)
return
}
logGame
.debug("Second card of pair flipped at index \(index) — evaluating against pending index \(pendingIndex)")
model.pendingCardIndex = nil
var newModel = model
newModel.clickCount += 1
newModel.cards[index].isFlipped = true
guard let pendingIndex = model.pendingCardIndex else {
newModel.pendingCardIndex = index
model = newModel
// swiftformat:disable:next redundantSelf
logGame.debug("First card flipped @\(index) — waiting for 2nd 🃏 (click #\(self.model.clickCount))")
onFlipCard?(index, true)
return
}
logGame
.debug("Second card of pair flipped at index \(index) — evaluating against pending index \(pendingIndex)")
newModel.pendingCardIndex = nil
model = newModel

Copilot uses AI. Check for mistakes.
Comment on lines +228 to +230
) {
flipBackWorkItem = clock.schedule(after: 1.0) { [weak self] in
self?.flipBackCards(index1: index1, index2: index2)
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scheduleFlipBack overwrites flipBackWorkItem without cancelling any previously scheduled work item. If the user triggers multiple non-matches quickly, you can end up with multiple pending flip-backs that can’t all be cancelled by stop(). Consider cancelling any existing flipBackWorkItem before scheduling a new one (or tracking all pending items), and clearing flipBackWorkItem once it fires.

Suggested change
) {
flipBackWorkItem = clock.schedule(after: 1.0) { [weak self] in
self?.flipBackCards(index1: index1, index2: index2)
) {
flipBackWorkItem?.cancel()
flipBackWorkItem = clock.schedule(after: 1.0) { [weak self] in
self?.flipBackCards(index1: index1, index2: index2)
self?.flipBackWorkItem = nil

Copilot uses AI. Check for mistakes.
Comment on lines +217 to +220
clock.schedule(after: 1.0) { [weak self] in
logGame.debug("Firing onNavigateToGameOver callback")
self?.onNavigateToGameOver?(outcome)
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triggerGameOver() schedules delayed navigation via clock.schedule(...) but doesn’t keep the returned DispatchWorkItem. Even though stop() nils onNavigateToGameOver, the scheduled work item will still execute after stop(), doing unnecessary work and making the view model harder to reason about if additional side effects are added later. Consider storing the returned work item (separate from flipBackWorkItem) and cancelling it in stop().

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to +52
func maxHeight(
in availableHeight: CGFloat,
flowLayout: UICollectionViewFlowLayout
) -> CGFloat {
let rows = CGFloat(rows)
let spacing = flowLayout.sectionInset.top + flowLayout.sectionInset.bottom
+ flowLayout.minimumLineSpacing * (rows - 1)
// `trunc` avoids sub-pixel cell sizes that cause layout rounding artefacts.
return trunc((availableHeight - spacing) / rows)
}

/// Maximum card width that fits all columns within `availableWidth`.
func maxWidth(
in availableWidth: CGFloat,
flowLayout: UICollectionViewFlowLayout
) -> CGFloat {
let columns = CGFloat(columns)
let spacing = flowLayout.sectionInset.left + flowLayout.sectionInset.right
+ flowLayout.minimumInteritemSpacing * (columns - 1)
return trunc((availableWidth - spacing) / columns)
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In maxHeight/maxWidth, let rows = CGFloat(rows) / let columns = CGFloat(columns) shadows the stored properties, which is easy to misread and error-prone during refactors. Rename the local constants (e.g. rowCount, columnCount) to avoid shadowing.

Copilot uses AI. Check for mistakes.
Comment on lines +58 to +64
/// Creates and registers `Cell.self` as single cell kind
static func make() -> SingleCellTypeCollectionView {
SingleCellTypeCollectionView(
dataSource: nil,
delegate: nil
)
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make() has an invalid return type for a generic class. SingleCellTypeCollectionView requires the Cell generic parameter, so this signature won’t compile as written. Return Self or SingleCellTypeCollectionView<Cell> instead.

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +31
@available(*, unavailable)
required init?(coder _: NSCoder) {
fatalError()
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required init?(coder:) uses fatalError() with no message, which makes crashes harder to diagnose. Consider providing a descriptive message (or using @available(*, unavailable, message: ...)).

Suggested change
@available(*, unavailable)
required init?(coder _: NSCoder) {
fatalError()
@available(*, unavailable, message: "Use init(dataSource:delegate:) or make() to create SingleCellTypeCollectionView programmatically.")
required init?(coder _: NSCoder) {
fatalError("init(coder:) is unavailable for SingleCellTypeCollectionView. Use init(dataSource:delegate:) or make() to create it programmatically.")

Copilot uses AI. Check for mistakes.
Comment on lines +78 to 89
/// Wires the callbacks, pushes the initial phase out, and kicks off the data fetch.
func start(
onPhaseChange: @escaping OnPhaseChange,
onNavigateToGame: @escaping OnNavigateToGame
) {
self.onPhaseChange = onPhaseChange
self.onNavigateToGame = onNavigateToGame
// swiftformat:disable:next redundantSelf
logNet.info("LoadingViewModel starting — config: \(self.config)")
onPhaseChange(phase)
fetchData()
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start() calls onPhaseChange(phase) and then immediately calls fetchData(), which sets phase = .loading again. Because didSet runs even when assigning the same value, this results in duplicate phase renders/logs. Consider removing the explicit onPhaseChange(phase) call (or stop reassigning .loading inside fetchData()).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

SwiftIntro.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved:11

  • diffuser is still pinned in Package.resolved, but the codebase no longer imports it. If Diffuser has been fully removed, drop this pin (and the corresponding Xcode package product dependency) so dependency resolution stays minimal and deterministic.
    {
      "identity" : "diffuser",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/spotify/diffuser",
      "state" : {
        "revision" : "fea166a5c366209e68bfd957b18780e7abba522c"
      }
    },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread SwiftIntro.xcodeproj/project.pbxproj Outdated
@@ -48,37 +49,31 @@
BBFE0003BBFE0003BBFE0003 /* Kingfisher in Frameworks */ = {isa = PBXBuildFile; productRef = BBFE0002BBFE0002BBFE0002 /* Kingfisher */; };
CC020002CC020002CC020002 /* GameSetup.xcstrings in Resources */ = {isa = PBXBuildFile; fileRef = CC020001CC020001CC020001 /* GameSetup.xcstrings */; };
DFSR0003DFSR0003DFSR0003 /* Diffuser in Frameworks */ = {isa = PBXBuildFile; productRef = DFSR0002DFSR0002DFSR0002 /* Diffuser */; };
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diffuser is still linked as a Swift package product/framework, but there are no remaining import Diffuser usages in the app or tests. Consider removing the Diffuser package dependency from the Xcode project and Package.resolved to reduce build time and avoid carrying an unused dependency.

Suggested change
DFSR0003DFSR0003DFSR0003 /* Diffuser in Frameworks */ = {isa = PBXBuildFile; productRef = DFSR0002DFSR0002DFSR0002 /* Diffuser */; };

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +82
guard let view = vc.view as? GameOverView else {
XCTFail("Expected vc.view to be GameOverView, got \(type(of: vc.view))", file: file, line: line)
return GameOverView()
}
return view
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper returns a brand-new GameOverView() after an XCTFail, which can let the test continue and potentially pass while interacting with the wrong view instance. Prefer making this helper throws and using XCTUnwrap (or failing hard) so a view-type mismatch reliably fails the test and doesn't mask wiring regressions.

Copilot uses AI. Check for mistakes.
Comment on lines 69 to 74
guard let view = vc.view as? GameView else {
XCTFail("Expected vc.view to be GameView, got \(type(of: vc.view))", file: file, line: line)
return GameView()
}
var model = GameModel(cards: cards, level: level)
model.matches = matches
return model
return view
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper returns a brand-new GameView() after an XCTFail, which can let tests continue and potentially pass while interacting with a view instance that isn't attached to the VC under test. Prefer making the helper throws and using XCTUnwrap so a type mismatch reliably fails the test.

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +90
guard let cv = gameView(of: vc, file: file, line: line)
.subviews.compactMap({ $0 as? UICollectionView }).first
else {
XCTFail("No UICollectionView found in GameView subviews", file: file, line: line)
return UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout())
}
return cv
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to gameView(of:), this helper returns a new empty UICollectionView(...) after XCTFail, which can hide regressions by letting tests keep running. Consider using throws + XCTUnwrap (or otherwise terminating the test) so missing/changed view hierarchy is a hard failure.

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +47
guard
let cell = dequeueReusableCell(withReuseIdentifier: Self.reuseIdentifier, for: indexPath) as? Cell
else {
fatalError("Incorrect implementation - unexpectedly wrong cell type - this is a programmer error")
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fatalError in dequeueReusableCell(at:) has a fairly generic message. Including the expected Cell.self, the actual dequeued type, and the reuse identifier would make debugging mis-registrations much faster if this ever trips.

Suggested change
guard
let cell = dequeueReusableCell(withReuseIdentifier: Self.reuseIdentifier, for: indexPath) as? Cell
else {
fatalError("Incorrect implementation - unexpectedly wrong cell type - this is a programmer error")
let dequeuedCell = dequeueReusableCell(
withReuseIdentifier: Self.reuseIdentifier,
for: indexPath
)
guard let cell = dequeuedCell as? Cell else {
fatalError(
"Incorrect implementation - unexpectedly wrong cell type. " +
"Expected: \(Cell.self), actual: \(type(of: dequeuedCell)), " +
"reuseIdentifier: \(Self.reuseIdentifier). This is a programmer error."
)

Copilot uses AI. Check for mistakes.
Comment on lines 25 to +26
7B21A5BB1CFF6281002E5EE5 /* MemoryDataSourceAndDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B21A5BA1CFF6281002E5EE5 /* MemoryDataSourceAndDelegate.swift */; };
CGL000002CGL000002CGL00002 /* CardGridLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = CGL000001CGL000001CGL00001 /* CardGridLayout.swift */; };
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new PBX object IDs added for CardGridLayout/GameViewModel/ImmediateClock (e.g. CGL000002…, GVM00002…, IMC00002…) are not valid 24-hex PBX identifiers. This will likely make project.pbxproj unreadable by Xcode / xcodebuild. Regenerate these entries via Xcode (add the files normally) or replace them with proper PBX IDs and ensure all references are consistent.

Suggested change
7B21A5BB1CFF6281002E5EE5 /* MemoryDataSourceAndDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B21A5BA1CFF6281002E5EE5 /* MemoryDataSourceAndDelegate.swift */; };
CGL000002CGL000002CGL00002 /* CardGridLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = CGL000001CGL000001CGL00001 /* CardGridLayout.swift */; };
7B21A5BB1CFF6281002E5EE5 /* MemoryDataSourceAndDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 48F0A1B12F9B100100A1B2C3 /* CardGridLayout.swift */; };
48F0A1B22F9B100100A1B2C3 /* CardGridLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = 48F0A1B12F9B100100A1B2C3 /* CardGridLayout.swift */; };

Copilot uses AI. Check for mistakes.
Game feature
- Drop the Mobius update loop, effects, events, and effect handler.
- Introduce GameViewModel: single source of truth for game state, exposes
  onModelChanged / onFlipCard / onNavigateToGameOver callbacks; owns
  flip-back and game-over DispatchWorkItems so they can be cancelled
  on viewDidDisappear.
- GameVC becomes a thin wiring layer: loadView installs GameView,
  viewWillAppear wires viewModel.start(...), viewDidDisappear stops it.
- GameView encapsulates the SingleCellTypeCollectionView<CardCVCell>
  internally; data source/delegate are init-injected.
- MemoryDataSourceAndDelegate's closures are non-optional `let` so missing
  wiring is a compile error rather than silently ignored.
- Extract grid-sizing math into testable CardGridLayout struct.

Loading feature
- LoadingVC now owns the view and wires LoadingViewModel callbacks in
  viewDidLoad (matching GameVC's pattern); init no longer threads view
  references through the view model.
- LoadingViewModel exposes onPhaseChange / onNavigateToGame callbacks
  (no Diffuser dependency).
- Drop dead Phase.initial NOOP case; logging-only branch removed.
- Warn when prefetch completes after onNavigateToGame is cleared.

Game-over flow
- Thread the original GameConfiguration through GameOutcome so RootVC
  no longer reconstructs `GameConfiguration(level: outcome.level)`.
- GameOverVC takes only an outcome; restart uses outcome.config.

Game setup
- GameSetupView is stateless: `startGameTapped` reads control values
  fresh and emits a GameConfiguration, no internal `var config`.

Reusable cell support
- Introduce CellProtocol with a default `cellIdentifier` via
  `String(describing: Self.self)`. SingleCellTypeCollectionView now
  constrains `Cell: UICollectionViewCell & CellProtocol`.
- NSObject.className: replace force-unwrap with `String(describing: self)`.

Polish
- GameView.animateFlip warns when the target cell is off-screen.
- GameOverView circular-button diameter named (no magic 80).
- CardDuplicates exposes init(ordered:) so tests can build a deterministic
  deck without retry-shuffling 4096 times.
- SingleCellTypeCollectionView.init(coder:) gets a descriptive fatalError.

Tests
- New unit tests: GameViewModel, GameView, CardGridLayout, CardSingles
  (Equatable/Hashable), LoadingViewModel (Phase.description, stop, retry),
  SingleCellTypeCollectionView (make / dequeue / cellForItemAt).
- LoadingVCTests: viewDidDisappear → stop, retry button @objc target.
- Replace force-casts in VC tests with XCTUnwrap / XCTFail with file:line.
- Mobius-era tests removed.

Tooling
- codecov.yml: ignore SwiftIntroTests/** so test code does not count
  toward coverage measurements.
- SwiftIntroTests/.swiftlint.yml: relax identifier_name min_length to 1
  (test-only) so locals like `vc`, `cv`, `a`, `b` don't trip the linter.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 47 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +15 to +20
func apa() {
let string = "A cool string"
if string != "A cool string" {
print("Not A cool string")
}
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper method (apa) is unrelated to the tested behavior, uses inconsistent indentation, and contains a print. Please remove it (or convert it into an actual test_… method) to keep the test suite focused and deterministic.

Copilot uses AI. Check for mistakes.
Comment on lines +204 to +206
model.cards[index].isMatched = true
model.cards[pendingIndex].isMatched = true
model.matches += 1
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleMatch mutates model three times (isMatched twice + matches), which will trigger the model's didSet/onModelChanged multiple times and can cause redundant renders / intermediate UI states. Consider batching these changes into a local copy and assigning model once.

Suggested change
model.cards[index].isMatched = true
model.cards[pendingIndex].isMatched = true
model.matches += 1
var newModel = model
newModel.cards[index].isMatched = true
newModel.cards[pendingIndex].isMatched = true
newModel.matches += 1
model = newModel

Copilot uses AI. Check for mistakes.
Comment on lines +263 to +264
model.cards[index1].isFlipped = false
model.cards[index2].isFlipped = false
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flipBackCards flips two cards by mutating model twice, which will call onModelChanged twice and can briefly leave the model in a half-updated state. Consider updating both flags on a local newModel and assigning model = newModel once before firing onFlipCard callbacks.

Suggested change
model.cards[index1].isFlipped = false
model.cards[index2].isFlipped = false
var newModel = model
newModel.cards[index1].isFlipped = false
newModel.cards[index2].isFlipped = false
model = newModel

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +146
guard let onNavigateToGame else {
logNav.warning("Image prefetch completed but onNavigateToGame is nil — navigation skipped")
return
}
logNet.info("All images in memory cache — navigating to game")
onNavigateToGame(PreparedGame(config: config, cards: cards))
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleFetchSuccess's prefetchImages completion is an escaping closure; accessing onNavigateToGame and config here without self. will not compile due to the explicit-self requirement. Bind from self.onNavigateToGame (or use self.onNavigateToGame?) and pass self.config when building PreparedGame.

Suggested change
guard let onNavigateToGame else {
logNav.warning("Image prefetch completed but onNavigateToGame is nil — navigation skipped")
return
}
logNet.info("All images in memory cache — navigating to game")
onNavigateToGame(PreparedGame(config: config, cards: cards))
guard let onNavigateToGame = self.onNavigateToGame else {
logNav.warning("Image prefetch completed but onNavigateToGame is nil — navigation skipped")
return
}
logNet.info("All images in memory cache — navigating to game")
onNavigateToGame(PreparedGame(config: self.config, cards: cards))

Copilot uses AI. Check for mistakes.
- GameViewModel.handleMatch: build a local copy and assign `model` once instead of mutating it three times (two isMatched flags + matches counter), so observers receive one coherent snapshot per matching pair instead of three intermediate updates
- GameViewModel.flipBackCards: same treatment for the two isFlipped resets so the flip-back timer never briefly shows one card face-up and one face-down to observers
- SingleCellTypeCollectionViewTests: remove the stray `apa()` helper (debug leftover with print and tab indentation that didn't belong in the test class)
- Skipped Copilot's LoadingViewModel comment about implicit self in handleFetchSuccess: with `guard let self else { return }` Swift 5.7+ rebinds self for the rest of the closure, so accessing `onNavigateToGame` and `config` without explicit `self.` is well-formed; CI confirms the file compiles
@Sajjon Sajjon merged commit ad8d2f4 into main Apr 25, 2026
2 checks passed
@Sajjon Sajjon deleted the unmobius_game branch April 25, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants