Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,16 @@ dotnet build BlocksBeyondTheStars.sln # build everything (Linux)
`run-tests.ps1` defaults to the fast .NET suites (`Dotnet` + `ClientCore`); the Unity Editor suites
(`UnityEdit` EditMode, `UnityPlay` PlayMode-vs-real-server-exe) are opt-in via `-Suites`. How the client is
tested against the real server is documented in
[docs/developer/CLIENT_TESTING.md](docs/developer/CLIENT_TESTING.md).
[docs/developer/CLIENT_TESTING.md](docs/developer/CLIENT_TESTING.md); how to write tests for the
server/shared suite (fixtures, exemplars, conventions) is in
[docs/developer/SERVER_TESTING.md](docs/developer/SERVER_TESTING.md).

**Internal spec citations (`anf_*.md`):** some doc comments cite files like
`anf_admin_einstellungen.md` §… as "technical requirements". These are the project's internal
German design specs from before open-sourcing and are **not in the public repository**. The
English summary in the doc comment is the authoritative public statement of the behaviour; a
comment that leans on such a citation without summarising the rule is a documentation bug —
fix or report it rather than hunting for the file.

To confirm a client rebuild actually happened, check the `BlocksBeyondTheStars.Client.dll` timestamp in the
build output (the `.exe` timestamp is not reliable). The full build guide — pipeline details,
Expand Down
4 changes: 3 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ If you are a developer, we welcome pull requests.
dotnet test # run all xUnit tests (keep them green)
```
The playable Windows client is built with `scripts/build-client.ps1` (requires the Unity
Editor). See [docs/developer/DEVELOPER.md](docs/developer/DEVELOPER.md).
Editor). See [docs/developer/DEVELOPER.md](docs/developer/DEVELOPER.md). If you want to
*write* tests (a great first contribution), start with
[docs/developer/SERVER_TESTING.md](docs/developer/SERVER_TESTING.md).
3. **Open a pull request** against `main` with a short description of the change and why.
Small, focused PRs are easier to review and merge.

Expand Down
19 changes: 19 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -7099,6 +7099,25 @@ is **pre-approved** (keys in `tools/ai-assets/.env`, run via `uv`).

---

## ✅ Done (2026-08-04): server test-writing guide + de-ghosted spec citations (#571 follow-up)

A contributor stepped down from #571 ("not enough business-logic context to write meaningful
tests"); root-cause analysis found the gaps and this closes them:

- **`docs/developer/SERVER_TESTING.md`** (new) — writing tests for the server/shared suite:
three exemplar tests to copy from (pure → content-loading → full server), the invisible
fixtures (`TestPaths.DataDir()` + `ContentLoader`, `TestLocales`), an "invariants, not
mirrored constants" table per target kind (the tautology trap), determinism rules (incl. the
Win/Linux libm trig gotcha), and the CI analyzer traps. Linked from CONTRIBUTING.md,
AGENTS.md and the docs index.
- **Internal spec citations de-ghosted** — 26 source files cite `anf_*.md` German design specs
that are gitignored (`/media/`), so fork contributors chased references into the void.
AGENTS.md now states the convention (doc-comment summary = public authority; a citation
without a summary is a doc bug); the three #571 target files leaning hardest on citations got
self-contained behaviour summaries (`MissionValidator` rule list, `ServerPresets` lookup
contract, `FrequencyExtensions` consumers + invariants incl. the `OreFactor` Off≠0 exception).
- Comment/docs-only change — no behaviour difference; issue #571 body refreshed to match.

## ✅ Done (2026-08-04): NPCs grounded, brighter and individual (#711)

Three related NPC problems fixed in one branch (issue #711):
Expand Down
3 changes: 3 additions & 0 deletions docs/developer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ belongs in TODO.md). Each doc states its own status near the top. Last reorganis
the CI job pair, the artifact execute-bit fix, and running an unsigned/un-notarized `.app`.
- [CLIENT_TESTING.md](CLIENT_TESTING.md) — how the Unity client is tested against the **real** game server
(the `Client.Core` split, the three test tiers, the selectable `run-tests.ps1` runner).
- [SERVER_TESTING.md](SERVER_TESTING.md) — **writing tests** for the server/shared .NET suite: exemplar
tests to copy from, the content/locale fixtures, what "meaningful" assertions look like, and the CI
analyzer traps. Start here for a first test contribution (e.g. issue #571).
- [SELF_HOSTING.md](SELF_HOSTING.md) — run and host a dedicated server, config keys, the web portal & updates.

## World & worldgen
Expand Down
102 changes: 102 additions & 0 deletions docs/developer/SERVER_TESTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# Server testing — writing your first test for the .NET suite

> **Status: contributor guide.** Written 2026-08-04 after contributor feedback on issue #571
> ("I don't have enough context to write meaningful tests"). [CLIENT_TESTING.md](CLIENT_TESTING.md)
> covers how the Unity client is tested; **this doc covers the server/shared suite**
> (`tests/BlocksBeyondTheStars.Tests`) — where things are, which existing tests to copy from,
> and what "meaningful" means for the kinds of code you'll be testing.
## The one-minute version

```powershell
dotnet test -c Release --filter Category!=Slow # fast daily loop (~3 min)
dotnet test # everything, incl. ~31 Slow soak tests
./scripts/test-coverage.ps1 -Open # full coverage report (slow, ~30 min)
```

Add a file to `tests/BlocksBeyondTheStars.Tests/`, name it `<Thing>Tests.cs`, use plain xUnit
`[Fact]`/`[Theory]`. No registration needed anywhere — CI shards test classes across runners
automatically ([`scripts/partition-tests.py`](../../scripts/partition-tests.py)), so a new class
just gets picked up.

## Three exemplars to copy from

Most of the 220+ test files are integration-style tests around a full `GameServer`**don't
start there**. These three show the ladder, simplest first:

1. **Pure logic, no fixtures**[`NameGeneratorTests.cs`](../../tests/BlocksBeyondTheStars.Tests/NameGeneratorTests.cs).
Asserts determinism (same seed → same output) and *shape* properties ("two capitalised
words") over a seed range. This is the pattern for `Vector3i`, `ChunkCoord`, noise,
`Localizer`, `FrequencyExtensions`, `ServerPresets`, `MissionValidator`.
2. **Needs game content**[`BlockShapeTests.cs`](../../tests/BlocksBeyondTheStars.Tests/BlockShapeTests.cs).
Loads the real data-driven content once in the constructor and uses a Guid-named temp dir
with `IDisposable` cleanup.
3. **Full server**[`GameServerIntegrationTests.cs`](../../tests/BlocksBeyondTheStars.Tests/GameServerIntegrationTests.cs).
Constructs a real `GameServer` with a loopback transport and a SQLite repo. Only needed
when the behaviour under test *is* the server loop (ticks, intents, persistence).

## Fixtures & helpers you would otherwise not find

- **`TestPaths.DataDir()`** ([TestPaths.cs](../../tests/BlocksBeyondTheStars.Tests/TestPaths.cs))
walks up from the test output directory to the repo's `data/` folder — so tests run against
the *real* shipped content from any build output location. The standard opener is:

```csharp
private static GameContent Load() => ContentLoader.LoadFromDirectory(TestPaths.DataDir());
```

`GameContent` is the loaded form of `data/*.json` (blocks, items, recipes, planets …); most
validators and generators take it as a parameter, and loading the real thing is both easier
and more honest than mocking it.
- **`TestLocales.Load("en" | "de")`** ([TestLocales.cs](../../tests/BlocksBeyondTheStars.Tests/TestLocales.cs))
reads the real locale tables — use it to assert a feature's keys exist in **both** languages
(a missing key renders as literal `[some.key]` in game instead of failing loudly).
- **Temp state**: `Path.Combine(Path.GetTempPath(), "bbts_<topic>_" + Guid.NewGuid().ToString("N"))`
plus `IDisposable` cleanup — never share paths between tests; the suite runs 4-way parallel.

## What "meaningful" means here (the tautology trap)

A lot of the untested surface is small pure functions whose *constants are the implementation*
e.g. `FrequencyExtensions.Probability(Rare) == 0.15`. A test that just restates the switch arm
mirrors the code and verifies nothing. **Assert the invariants instead** — the properties that
must survive any future retuning of the numbers:

| Target kind | Meaningful assertions |
|---|---|
| Factor tables (`FrequencyExtensions`) | `Off` is exactly `0` where "off must mean off"; values are **monotone** in enum order; the enum's default level maps to `1.0` for factors documented as "existing worlds unchanged" |
| Primitive types (`Vector3i`, `ChunkCoord`) | equality ↔ hash-code consistency, roundtrips (pack/unpack, parse/format), arithmetic identities |
| Lookup tables (`ServerPresets`, locale keys) | case-insensitivity, unknown name → `null` (not throw), every advertised `Names` entry resolves |
| Validators (`MissionValidator`) | each documented error case produces a problem; a known-good definition produces **zero** problems |
| `Localizer` | active-locale hit, English fallback, unknown key → `[key]` wrap, empty key → empty string |
| Noise / RNG / generators | same seed → same output; output range; seam continuity at the world wrap (see [WORLD_WRAP.md](WORLD_WRAP.md)); **never compare trig-derived floats or golden-hash them**`sin`/`cos` differ between Windows and Linux libm and the hash will pass locally and fail in CI. Hash integers, or assert structural properties |

If you can't tell which invariants are intended: the XML doc comment on the type is the public
authority on behaviour (see the note on internal specs below), and asking on the issue is
always fine — pointing at an undocumented spot is itself a useful contribution.

## Where behaviour is specified

- The **XML doc comments** on the types themselves — these are kept as the public source of
truth for intended behaviour.
- The [docs/developer/ index](README.md) has a deep-dive per area (worldgen →
[WORLD_GENERATION.md](WORLD_GENERATION.md), topology → [WORLD_WRAP.md](WORLD_WRAP.md), …).
- Some comments cite `anf_*.md` files ("technical requirements"). Those are the project's
**internal German design specs from before open-sourcing; they are not in the public repo.**
Treat the English summary in the doc comment as the authoritative statement of intent — and
if a comment leans on such a citation without summarising the rule, that's a doc bug worth
reporting (or fixing in your PR).

## CI rules that will bite you

CI treats **warnings as errors** (Roslyn + Meziantou + VS.Threading analyzers). The recurring
first-PR traps:

- Async test methods need an `Async` suffix (**VSTHRD200**).
- Don't `await` a `TaskCompletionSource.Task` in tests (**VSTHRD003**) — use a `SemaphoreSlim`.
- Any test not marked `[Trait("Category", "Slow")]` must finish **well under 120 s** — the PR
gate ([`scripts/check-test-durations.py`](../../scripts/check-test-durations.py)) fails on it.
Use the `Slow` trait for soak/heavy-worldgen tests; they still run on every push to `main`.
- Don't rely on `dotnet test -v minimal` output to check for warnings — it hides them. Do a
clean `dotnet build -warnaserror` (see AGENTS.md "Local verification after changes").
- The suite pins `maxParallelThreads: 4` ([xunit.runner.json](../../tests/BlocksBeyondTheStars.Tests/xunit.runner.json))
— heavy worldgen tests funnel through shared caches; don't "fix" a slow local run by raising it.
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
namespace BlocksBeyondTheStars.Shared.Configuration;

/// <summary>
/// Predefined rule profiles (technical requirements / `anf_admin_einstellungen.md` §4 and
/// `anf_space_flight.md` §13.2) so admins don't have to set every rule individually.
/// Predefined rule profiles so admins don't have to set every rule individually. Contract:
/// <see cref="Names"/> lists every available preset; <see cref="Get"/> resolves trimmed and
/// case-insensitively and returns null for unknown (or null) names — callers fall back to
/// default rules. (Originally the internal design specs `anf_admin_einstellungen.md` §4 /
/// `anf_space_flight.md` §13.2 — this summary is the public authority.)
/// </summary>
public static class ServerPresets
{
Expand Down
10 changes: 7 additions & 3 deletions src/BlocksBeyondTheStars.Shared/Missions/MissionValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@
namespace BlocksBeyondTheStars.Shared.Missions;

/// <summary>
/// Validates a mission definition against the loaded content (technical requirements /
/// `anf_admin_blueprinf.md` §10). Shared by the in-game player editor and the admin
/// extension editor so the same rules apply everywhere.
/// Validates a mission definition against the loaded content. Shared by the in-game player
/// editor and the admin extension editor so the same rules apply everywhere. The rules: a
/// mission needs a non-empty id and at least one objective; every objective needs a supported
/// type (Collect/Mine/Deliver), a positive required count and a target that exists in the
/// loaded content (a block for Mine, an item otherwise); every reward must reference a known
/// item with a positive count. (Originally the internal design spec `anf_admin_blueprinf.md`
/// §10 — this summary is the public authority.)
/// </summary>
public static class MissionValidator
{
Expand Down
19 changes: 16 additions & 3 deletions src/BlocksBeyondTheStars.Shared/World/WorldDescription.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,18 @@ public enum Frequency
Frequent,
}

/// <summary>
/// Maps a <see cref="Frequency"/> level to the numeric knobs the generators consume. The exact
/// constants are tuning values and may be retuned; the intended invariants are: every mapping is
/// monotone non-decreasing in enum order, <see cref="Frequency.Off"/> is exactly 0 wherever
/// "off must mean off" (<see cref="OreFactor"/> is the documented exception), and factor-style
/// mappings return 1.0 at the owning property's default level so existing worlds are unchanged.
/// </summary>
public static class FrequencyExtensions
{
/// <summary>Selection weight; 0 means the feature never spawns.</summary>
/// <summary>Relative weight for weighted selection — consumed by the universe generator's
/// planet-type roll (<see cref="WorldDescription.PlanetTypeFrequencies"/>); 0 means the
/// feature never spawns.</summary>
public static int Weight(this Frequency f) => f switch
{
Frequency.Off => 0,
Expand All @@ -26,7 +35,9 @@ public static class FrequencyExtensions
_ => 0,
};

/// <summary>Probability in [0,1] used for per-body chance rolls.</summary>
/// <summary>Probability in [0,1] for independent chance rolls — e.g. the universe
/// generator's space-station and wreck gates (<see cref="WorldDescription.SpaceStations"/>,
/// <see cref="WorldDescription.Wrecks"/>) and the settlement/station template-use rolls.</summary>
public static double Probability(this Frequency f) => f switch
{
Frequency.Off => 0.0,
Expand All @@ -37,7 +48,9 @@ public static class FrequencyExtensions
_ => 0.0,
};

/// <summary>Flora/tree density factor (world options; Normal = unchanged, Off = barren).</summary>
/// <summary>Flora/tree density multiplier applied to per-planet flora generation
/// (world options / <see cref="WorldDescription.FloraDensity"/>; Normal = 1.0 = unchanged,
/// Off = barren).</summary>
public static double FloraFactor(this Frequency f) => f switch
{
Frequency.Off => 0.0,
Expand Down