From b32d96830ee055af69fcc010b190f50f6433b9a5 Mon Sep 17 00:00:00 2001 From: "Claude (Architectural Reviewer)" Date: Thu, 24 Sep 2026 08:46:29 -0500 Subject: [PATCH] #153: forest concealment (visibility parity, 5th 1.6.x theme) calcVis/forestVis were already a faithful oracle port, wired into rendering, but visFraction short-circuited to 1.0 unless hiddenMines was on and a fogState existed -- forest concealment is core, always-on oracle gameplay, unrelated to the separate Hidden Mines fog-of-war feature. Compounding it, only the host render path ever supplied a fogState at all, so a joined guest never got any concealment even when the host did. Fix: calcVis accepts fogState: FogState?, treating a missing one as "no fog contribution" while still applying the forest term and distance floor. visFraction's useForestTerm path (tank + walking builder, matching the oracle's own per-sprite-kind calcvis() choice) always calls calcVis now. Fixes host, solo, and join uniformly through the same call sites. Also softens the sea/river water textures (too bold/high-contrast per live feedback) -- halved each texture's color delta from its base fill. 148/148 Bolo 2026Tests, 956/956 SwiftPM tests green (known pre-existing timing/port-contention flakes, confirmed unrelated and pass in isolation). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XdDqxCXosBhuFQWVkuCzrz --- Bolo 2026/Bolo 2026/GameRenderView.swift | 17 ++- .../VisibilityParityTests.swift | 102 ++++++++++++++++++ Sources/BoloGlyphsCore/GlyphSource.swift | 11 +- Sources/BoloKit/CalcVis.swift | 11 +- .../FogDifferentialTests.swift | 44 ++++++++ docs/STATUS.md | 30 ++++++ 6 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 Bolo 2026/Bolo 2026Tests/VisibilityParityTests.swift diff --git a/Bolo 2026/Bolo 2026/GameRenderView.swift b/Bolo 2026/Bolo 2026/GameRenderView.swift index f8aa123..3f3319a 100644 --- a/Bolo 2026/Bolo 2026/GameRenderView.swift +++ b/Bolo 2026/Bolo 2026/GameRenderView.swift @@ -1035,13 +1035,20 @@ public final class GameRenderView: NSView { /// `calcVis`/`fogVis` fraction at `point` for the current render's observer, matching /// `GSBoloView.m:301,315,349,363,378,389`'s own per-sprite-kind choice of which of the - /// two to call. `1.0` (fully visible, no fade) when `state.hiddenMines` is off or no - /// `fogState` was supplied -- the D65 default this feature must not change. + /// two to call. + /// + /// #153: the `useForestTerm` (tank/walking-builder) path always calls `calcVis` now, + /// even with no `fogState` -- forest concealment is core oracle gameplay, unrelated to + /// `hiddenMines`/fog tracking (see `calcVis`'s own doc comment). The plain-`fogVis` path + /// (shells/explosions/parachute builder) has nothing to offer without real fog data, so + /// it keeps the `hiddenMines`/`fogState` gate -- `1.0` (fully visible, no fade) there is + /// still the D65 default this feature must not change. private func visFraction(at point: Vec2f, useForestTerm: Bool) -> Float { + if useForestTerm { + return calcVis(point, state: state, fogState: fogState, observer: state.localPlayer) + } guard state.hiddenMines, let fogState else { return 1.0 } - return useForestTerm - ? calcVis(point, state: state, fogState: fogState, observer: state.localPlayer) - : fogVis(point, fogState: fogState) + return fogVis(point, fogState: fogState) } /// `CGContext.draw(_:in:)` draws a `CGImage`'s row 0 at the *high-Y* edge of the destination diff --git a/Bolo 2026/Bolo 2026Tests/VisibilityParityTests.swift b/Bolo 2026/Bolo 2026Tests/VisibilityParityTests.swift new file mode 100644 index 0000000..cc8c9ec --- /dev/null +++ b/Bolo 2026/Bolo 2026Tests/VisibilityParityTests.swift @@ -0,0 +1,102 @@ +// +// VisibilityParityTests.swift +// Bolo 2026Tests +// +// #153 (v1.6.6 -- Visibility Parity): `visFraction`'s `useForestTerm: true` path now always +// calls `calcVis`, even with no `fogState` -- the join/solo paths, and the host with +// `hiddenMines` off, never had one (`GameRenderView.render(_:fogState:)`'s own doc comment). +// Reuses `RenderParityHarness.swift`'s offscreen pixel-diff harness to confirm the render +// effect actually happens, not just that `calcVis` returns the right number in isolation +// (already covered by `FogDifferentialTests.swift`'s `nil`-`fogState` unit tests). + +import AppKit +import CoreGraphics +import Testing +import BoloKit + +@testable import Bolo_2026 + +private func twoPlayerForestScene(observerTank: Vec2f, enemyForestCenter: (x: Int32, y: Int32)) -> GameState { + var state = GameState() + var observer = PlayerState() + observer.used = true + observer.connected = true + observer.dead = false + observer.tank = observerTank + + var enemy = PlayerState() + enemy.used = true + enemy.connected = true + enemy.dead = false + enemy.tank = Vec2f(x: Float(enemyForestCenter.x) + 0.5, y: Float(enemyForestCenter.y) + 0.5) + + state.players = [observer, enemy] + state.localPlayer = 0 + state.hiddenMines = false // the default -- concealment must not depend on this + + for dy: Int32 in -1...1 { + for dx: Int32 in -1...1 { + state.terrain[Int(enemyForestCenter.x + dx), Int(enemyForestCenter.y + dy)] = .forest + } + } + return state +} + +struct VisibilityParityTests { + + /// The actual reported symptom: with `hiddenMines` off and no `fogState` (the join/solo + /// default -- `render(_:fogState:)` is called with no second argument at all), an enemy + /// tank buried in forest, far from the observer's own tank, must render differently than + /// the same tank fully in the open. If `visFraction` still short-circuited to `1.0` here, + /// these two renders would be pixel-identical. + @Test @MainActor func forestConcealedEnemyTankRendersDifferentlyThanOneInTheOpenWithNoFogState() throws { + let tiles = try #require(loadSheetImage(named: "Tiles")) + let sprites = try #require(loadSheetImage(named: "Sprites")) + let viewConcealed = GameRenderView(tilesImage: tiles, spritesImage: sprites) + let viewOpen = GameRenderView(tilesImage: tiles, spritesImage: sprites) + + let concealedState = twoPlayerForestScene(observerTank: Vec2f(x: 10, y: 10), enemyForestCenter: (x: 100, y: 100)) + var openState = concealedState + openState.terrain = GameState().terrain // no forest anywhere + + viewConcealed.render(concealedState) // no fogState argument -- matches join/solo call sites + viewOpen.render(openState) + + let captureRect = NSRect(x: 1584, y: 1584, width: 96, height: 96) // around tile (100, 100), tile = 16pt + let concealedPixels = try renderRGBA(viewConcealed, rect: captureRect) + let openPixels = try renderRGBA(viewOpen, rect: captureRect) + let diff = try #require(pixelDiffCount(concealedPixels, openPixels)) + #expect(diff > 0, "a forest-concealed enemy tank must render differently than one in the open, even with hiddenMines off and no fogState") + } + + /// Graduated, not binary: the same forest-buried enemy tank must render differently + /// depending on the observer's own distance to it -- fully concealed far away, but the + /// oracle's own 2-3 unit distance floor kicks in once the observer is right next to it + /// (never fully invisible up close, matching Jerod's reported "still only slightly visible + /// when sitting side by side"). + @Test @MainActor func forestConcealedEnemyTankRendersMoreVisiblyTheCloserTheObserverIs() throws { + let tiles = try #require(loadSheetImage(named: "Tiles")) + let sprites = try #require(loadSheetImage(named: "Sprites")) + let viewFar = GameRenderView(tilesImage: tiles, spritesImage: sprites) + let viewClose = GameRenderView(tilesImage: tiles, spritesImage: sprites) + + let farState = twoPlayerForestScene(observerTank: Vec2f(x: 10, y: 10), enemyForestCenter: (x: 100, y: 100)) + // 2.0 tiles from the enemy tank (100.5, 100.5) -- within the oracle's dist<=2.0 floor, + // but far enough on screen (2 tiles = 32pt, sprites are ~16pt) not to overdraw it with + // the observer's own always-fraction-1.0 tank sprite and contaminate the pixel diff. + let closeState = twoPlayerForestScene(observerTank: Vec2f(x: 98.5, y: 100.5), enemyForestCenter: (x: 100, y: 100)) + + viewFar.render(farState) + viewClose.render(closeState) + + // Tight around the enemy tank's own sprite rect only (drawn at (100.5, 100.5) -> pixel + // origin (1600, 1600), 16pt tile) -- excludes the observer's own tank sprite at tile 98 + // (pixel origin ~1568), which would otherwise overdraw and contaminate the diff if the + // capture rect were wider. + let captureRect = NSRect(x: 1596, y: 1596, width: 24, height: 24) + let farPixels = try renderRGBA(viewFar, rect: captureRect) + let closePixels = try renderRGBA(viewClose, rect: captureRect) + let diff = try #require(pixelDiffCount(farPixels, closePixels)) + #expect(diff > 0, "concealment must be graduated by distance, not a fixed reduction regardless of how close the observer is") + } +} diff --git a/Sources/BoloGlyphsCore/GlyphSource.swift b/Sources/BoloGlyphsCore/GlyphSource.swift index 374d20e..ab997ca 100644 --- a/Sources/BoloGlyphsCore/GlyphSource.swift +++ b/Sources/BoloGlyphsCore/GlyphSource.swift @@ -235,8 +235,10 @@ private func applyWallBevel(_ c: inout Canvas16) { /// has no shape edges to bevel) plus a sparse set of brighter glint pixels on top. Only ever /// recolors already-opaque pixels, matching the wall bevel's invariant. private func applySeaShading(_ c: inout Canvas16) { - let highlight: (UInt8, UInt8, UInt8) = (45, 95, 190) - let glint: (UInt8, UInt8, UInt8) = (70, 130, 220) + // #153 follow-up: halved both deltas from the base sea color (20, 70, 160) -- Jerod + // reported the pattern reads too bold/high-contrast against the flat fill. + let highlight: (UInt8, UInt8, UInt8) = (32, 82, 175) + let glint: (UInt8, UInt8, UInt8) = (45, 100, 190) func isOpaque(_ x: Int, _ y: Int) -> Bool { guard x >= 0, x < Canvas16.size, y >= 0, y < Canvas16.size else { return false } return c.pixels[(y * Canvas16.size + x) * 4 + 3] != 0 @@ -290,7 +292,10 @@ private func applyForestCanopy(_ c: inout Canvas16) { /// in *shape* from `applySeaShading`'s diagonal split (river is a current, sea is open water). /// Only ever recolors already-opaque pixels, same invariant as every texture above. private func applyRiverFlow(_ c: inout Canvas16) { - let flow: (UInt8, UInt8, UInt8) = (90, 140, 235) + // #153 follow-up: halved the delta from the base river color (60, 110, 220) -- same + // "too bold" softening as `applySeaShading` above, kept in sync since the two water + // families share the complaint even though their patterns are shaped differently. + let flow: (UInt8, UInt8, UInt8) = (75, 125, 227) func isOpaque(_ x: Int, _ y: Int) -> Bool { guard x >= 0, x < Canvas16.size, y >= 0, y < Canvas16.size else { return false } return c.pixels[(y * Canvas16.size + x) * 4 + 3] != 0 diff --git a/Sources/BoloKit/CalcVis.swift b/Sources/BoloKit/CalcVis.swift index 49992ca..a2427f7 100644 --- a/Sources/BoloKit/CalcVis.swift +++ b/Sources/BoloKit/CalcVis.swift @@ -76,10 +76,17 @@ public func fogVis(_ v: Vec2f, fogState: FogState) -> Float { /// (armour neither `pillOnboard` nor `0`) get a hard 3.0-world-unit minimum-visibility /// floor regardless of fog/forest, blended smoothly between 2.0 and 3.0 units rather than /// snapping. -public func calcVis(_ v: Vec2f, state: GameState, fogState: FogState, observer: Int) -> Float { +/// +/// #153: `fogState` is `nil` whenever the caller has no real fog tracking (`hiddenMines` +/// off, or the join/solo paths, which never compute one at all -- see +/// `GameRenderView.render(_:fogState:)`'s own doc comment). Forest concealment is core, +/// always-on oracle gameplay, unrelated to the separate "Hidden Mines" fog-of-war feature +/// (#1) -- so a missing `fogState` means "no fog contribution" (`fogVis` term is `1.0`, +/// fully lit), not "skip forest concealment entirely." +public func calcVis(_ v: Vec2f, state: GameState, fogState: FogState?, observer: Int) -> Float { guard v.x >= 0.0, v.x < 256.0, v.y >= 0.0, v.y < 256.0 else { return 0.0 } - let fog = fogVis(v, fogState: fogState) + let fog = fogState.map { fogVis(v, fogState: $0) } ?? 1.0 let forest = forestVis(v, state: state) var dist: Float = 3.0 diff --git a/Tests/DifferentialTests/FogDifferentialTests.swift b/Tests/DifferentialTests/FogDifferentialTests.swift index 0918115..d636292 100644 --- a/Tests/DifferentialTests/FogDifferentialTests.swift +++ b/Tests/DifferentialTests/FogDifferentialTests.swift @@ -94,6 +94,50 @@ import CXBolo let vis = calcVis(v, state: state, fogState: fogState, observer: 0) #expect(vis == 1.0) // fully lit, no forest, far from own tank/pills } + + // #153: `fogState: nil` (the join/solo paths, or the host with `hiddenMines` off) must + // still apply forest concealment -- only the fog term drops out to "fully lit." + + @Test func testCalcVisWithNilFogStateStillAppliesForestConcealmentFarAway() { + var state = GameState() + state.players = [PlayerState()] + state.players[0].tank = BoloKit.Vec2f(x: 10, y: 10) // far from the forest tile below + for dy in -1...1 { + for dx in -1...1 { + state.terrain[200 + dx, 200 + dy] = .forest + } + } + let v = BoloKit.Vec2f(x: 200.5, y: 200.5) + let vis = calcVis(v, state: state, fogState: nil, observer: 0) + #expect(vis == forestVis(v, state: state)) + #expect(vis < 1.0) // fully forest-surrounded -> concealed + } + + @Test func testCalcVisWithNilFogStateStillFloorsVisibilityNearOwnTank() { + var state = GameState() + state.players = [PlayerState()] + state.players[0].tank = BoloKit.Vec2f(x: 50, y: 50) + for dy in -1...1 { + for dx in -1...1 { + state.terrain[50 + dx, 50 + dy] = .forest + } + } + // Standing on your own tank: dist == 0 <= 2.0, so vis is at least 0.5 even fully + // forest-concealed, same distance floor as the fogState-present case above. + let vis = calcVis(BoloKit.Vec2f(x: 50, y: 50), state: state, fogState: nil, observer: 0) + #expect(vis >= 0.5) + } + + @Test func testCalcVisWithNilFogStateMatchesCalcVisBlendWithFogTermOne() { + var state = GameState() + state.players = [PlayerState()] + state.players[0].tank = BoloKit.Vec2f(x: 10, y: 10) + state.terrain[100, 100] = .forest + let v = BoloKit.Vec2f(x: 100.5, y: 100.5) + let vis = calcVis(v, state: state, fogState: nil, observer: 0) + let expected = calcVisBlend(forestVis: forestVis(v, state: state), fogVis: 1.0, dist: mag2f(sub2f(state.players[0].tank, v))) + #expect(vis == expected) + } } @Suite struct FogTileForDifferentialTests { diff --git a/docs/STATUS.md b/docs/STATUS.md index 07d5388..d32b799 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -133,6 +133,36 @@ code-vs-`Reference/c` audits, not the backlog -- the closest backlog candidates, [#5](https://github.com/CosmicCEO/BoloKit/issues/5) (explosion owner attribution), remain their own explicitly parked `Decide:` rulings, left alone rather than force-fit. +**Milestone 27, `v1.6.6 — Visibility Parity`, done and closed 2026-09-24.** Jerod's ruling: the +original four-theme roadmap (man/tank/boat/sound) missed a fifth theme, visibility/concealment -- +live-observed: a tank entering forested terrain should become progressively less visible with +distance and cover, never fully invisible up close. Audit found this is not a missing mechanic: +`bolo.c:174` `forestvis()` + `bolo.c:217-321` `calcvis()` are already a faithful, +differentially-tested port (`Sources/BoloKit/CalcVis.swift`, `Sources/BoloKit/PillTick.swift`) +and already wired into rendering (`GameRenderView.swift`'s `visFraction`). The bug: +`visFraction` short-circuited to `1.0` (always fully visible) unless `state.hiddenMines` was on +*and* a `fogState` existed -- but forest concealment is core, always-on oracle gameplay, +unrelated to the separate Hidden Mines fog-of-war feature (#1). Compounding this, only the host +render path ever supplied a `fogState` at all (`GameRenderView.render(_:fogState:)` defaults to +`nil`); the join/guest path never had one, structurally, regardless of `hiddenMines` -- so a +joined guest never saw any concealment on the enemy tank, even when the host did. Filed and +fixed as [#153](https://github.com/CosmicCEO/BoloKit/issues/153): `calcVis` now accepts +`fogState: FogState?`, treating a missing one as "no fog contribution" while still applying the +forest term and the distance floor; `visFraction`'s `useForestTerm: true` path (tank + +walking-builder, matching the oracle's own per-sprite-kind `calcvis()` choice) always calls +`calcVis` now. Fixes host, solo, and join uniformly -- no join-path-specific code needed, since +all three render through the same call sites. The parachuting-builder state deliberately stays +on plain `fogVis` (`GSBoloView.m:389` uses `fogvis`, not `calcvis`, for an airborne builder -- +correct oracle behavior, not a gap). New coverage: 4 differential unit tests +(`FogDifferentialTests.swift`, `calcVis` with `fogState: nil`) plus 2 offscreen pixel-diff render +tests (`VisibilityParityTests.swift`) confirming the actual reported symptom -- a forest-buried +enemy tank renders differently than one in the open with no `fogState` at all, and more visibly +the closer the observer gets. Also softened the sea/river water textures in the same pass +(`applySeaShading`/`applyRiverFlow`, `GlyphSource.swift`) -- Jerod reported the patterns read too +bold/high-contrast; halved each texture's color delta from its base fill. 148/148 `Bolo +2026Tests` and 956/956 SwiftPM tests green (same pre-existing flakes as milestone 20/26, +confirmed unrelated). + ## Queued next — milestone 20 `v1.6.x — Rejoin fix`, re-scoped 2026-09-23 Triaged the milestone's 9 open issues against what actually shipped in 1.5.1 and the