lennart/nightclub-arcadia
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor the skill system to adhere to the lore handbook

Browse Source
This commit is contained in:
lennart
2026-08-15 11:13:15 +02:00
parent 063e94f6eb
commit 9e5c3277bf
70 changed files with 2271 additions and 445 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/skill-system-refactor-plan.md
+620
View File
@@ -0,0 +1,620 @@
# Skill System Refactor Plan — placeholder roster → Skill Bible roster
**Audience:** an implementing agent with no prior context on this repo.
**Repo root:** `/Users/lennart/Dev/nightlcub-arcadia`
**Unity project:** `NightclubArcadia/` — Unity `6000.5.8f1`, Yarn Spinner 3 (`dev.yarnspinner.unity`), TextMeshPro.
**Unity editor binary:** `/Applications/Unity/Hub/Editor/6000.5.8f1/Unity.app/Contents/MacOS/Unity`
Read this whole document before touching a file. The section that matters most is
[§2 The reconciliation principle](#2-the-reconciliation-principle) — it tells you what *not* to build.
---
## 1. What exists today
A working, tested skill system built to a *placeholder* roster. The architecture is sound; only the
content is wrong.
### 1.1 File inventory
```
NightclubArcadia/Assets/Scripts/Skills/ (asmdef: NightclubArcadia.Skills)
Data/SkillDefinition.cs ScriptableObject: id, displayName, domain, description,
startingRank, accentColor. Has #if UNITY_EDITOR EditorSet(...).
Data/SkillDatabase.cs ScriptableObject: List<SkillDefinition> + case-insensitive
TryGet lookup + OnValidate duplicate/empty-id checks.
Data/SkillDomain.cs enum { Reason, Instinct, Presence, Body }
Data/DcBand.cs enum DcBand { Trivial=5, Easy=8, Standard=11, Hard=14,
Formidable=17, Legendary=20 } + DcBands.TryParse(string,out int)
which accepts band names (case-insensitive) OR raw integers.
Data/SkillCheckDegree.cs enum { CriticalFailure, Failure, Success, CriticalSuccess }
Data/SkillCheckResult.cs readonly struct: skill id/display, roll, rankMod, situationalMod,
stateMod, total, dc, success, degree.
Runtime/PlayerSkills.cs Plain C# class. Ranks only, seeded from SkillDatabase.StartingRank.
Runtime/SkillStateModifiers.cs Plain C#. Nested dict sourceKey -> (skillId -> delta).
Supports skillId "*" as a wildcard. TotalFor(skillId) sums all.
Runtime/SkillCheckSystem.cs Plain C#, no UnityEngine types. Resolve(skillId, dc, situationalMod):
d20 + rank + situational + state; nat 1 = CriticalFailure,
nat 20 = CriticalSuccess, else total >= dc.
Runtime/IRollSource.cs Roll(sides) -> [1,sides]
Runtime/UnityRollSource.cs Random.Range
Runtime/SeededRollSource.cs System.Random, for tests / determinism
Runtime/SkillRuntime.cs MonoBehaviour singleton + composition root. Holds Database,
DialogueRunner, ResultView, Commentary; constructs PlayerSkills,
SkillStateModifiers, SkillCheckSystem, IRollSource in Awake().
Yarn/YarnSkillCommands.cs static class. [YarnCommand("check")] Check(skillId, difficulty,
modifier=0) -> IEnumerator; [YarnFunction("skill_rank")].
Writes $check_result/_roll/_total/_dc/_modifier/_degree/_skill.
UI/SkillCheckResultView.cs Fading TMP banner. Prints roll, each of the three mods, total,
DC, and outcome word. Tints by SkillDefinition.accentColor.
Commentary/CommentarySystem.cs Passive barks. Looks for Yarn node "Commentary_{contextId}_{skillId}";
only skills whose node exists AND rank >= minRankToSpeak are
candidates. Global cooldown minSecondsBetween (20s), recency-window
weight halving, weight = max(1, rank). Public static Pick(...) is
unit-tested. Fires exactly one node per request.
Commentary/CommentaryTrigger.cs, Commentary/CommentaryIdleTimer.cs context sources.
NightclubArcadia/Assets/Skills/
SkillDatabase.asset references the 12 definition assets by GUID
Definitions/*.asset (+.meta) deduction, streetwise, recall, read_room, paranoia, reflexes,
persuasion, intimidation, rapport, endurance, sleight_of_hand, nerve
NightclubArcadia/Assets/Editor/
SkillSystemSetup.cs SOURCE OF TRUTH for the roster today: a static SkillSpec[] Roster,
a menu item "Tools/Nightclub Arcadia/Set Up Skill System" that
regenerates the definition assets + database and rewires
Assets/Scenes/DialogueTest.unity, and a [DidReloadScripts] hook
(EnsureSkillAssetsOnly) that regenerates the ASSETS ONLY once per
editor session. <-- see §7.1, this will overwrite hand-edits.
SkillSystemTestRunner.cs menu item to run the EditMode test assembly.
NightclubArcadia/Assets/Tests/EditMode/ (asmdef: NightclubArcadia.Skills.Tests)
DcBandTests.cs asserts "standard" -> 11
SkillCheckSystemTests.cs modifier math, nat 1/20, wildcard state mods, unknown skill
CommentaryPickerTests.cs weighted pick + recency halving
NightclubArcadia/Assets/Dialogue/
Common.yarn declares $check_* variables (never <<set>> by hand)
Commentary.yarn Commentary_Entrance_Queue_{paranoia,read_room,streetwise}
Scenes/Entrance.yarn Start / Entrance_Inside / Entrance_Outside — no skill references
Scenes/Debug.yarn <<check persuasion standard>>, <<check deduction 16 2>>,
<<check nosuchskill standard>> (deliberate error-path test)
Characters/Bouncer.yarn <<check persuasion standard>>, skill_rank("intimidation") >= 2,
<<check intimidation hard -1>>, branches on $check_degree
```
### 1.2 Every existing skill-name reference in the project (the complete change surface)
| File | Reference |
|---|---|
| `Assets/Editor/SkillSystemSetup.cs` | the 12-row `Roster` array |
| `Assets/Skills/Definitions/*.asset` | 12 assets, `id:` + `displayName:` fields |
| `Assets/Skills/SkillDatabase.asset` | 12 GUID references |
| `Assets/Dialogue/Scenes/Debug.yarn` | `persuasion`, `deduction` |
| `Assets/Dialogue/Characters/Bouncer.yarn` | `persuasion` ×1, `intimidation` ×2 |
| `Assets/Dialogue/Commentary.yarn` | node titles for `paranoia`, `read_room`, `streetwise` |
| `Assets/Tests/EditMode/*.cs` | `persuasion`, `reflexes`, `read_room`, `paranoia` as literals |
Nothing else in the project references a skill by name. The scene `DialogueTest.unity` references the
**database asset**, not individual definitions, so replacing definition assets does not break scene
wiring.
---
## 2. The reconciliation principle
The target system is described in two documents:
- **`/Users/lennart/Obsidian-Vaults/obsidian-vault/Projects/Active/NightclubArcadia/Skill System Draft.md`**
— the Skill Bible. Eleven named voices, four opposed pairs plus three specialists, with per-voice
prose (domain, blind spot, register, verbal tics, forbidden domains, success/failure/passive voice
samples), firing-frequency targets, and a trace-channel coverage audit in §6. **Read it.** Its full
content is not reproduced here; §4 below reproduces only the machine-relevant fields.
- An architecture **skeleton** (a rough C# sketch, reproduced in §3 below), which was handed over as
"a starting spec, not a finished implementation."
**The player never sees an implementation. They see a name on a banner and a line of prose.**
Therefore:
> Where the existing implementation already produces the target behaviour under a different
> internal name, **rename it. Do not rebuild it.** Only write new code where the target requires
> behaviour that does not exist.
The skeleton in §3 is *less* capable than what is already in the repo in several places (it has no
degrees of success, no seedable roll source, no error handling, no node-existence gate, and it puts
game logic in MonoBehaviours that are currently plain testable C# classes). **Do not regress the
implementation to match the skeleton.** Match its *vocabulary and its data model*, keep the existing
mechanics.
### 2.1 Decision table — what each difference actually costs
| Skeleton / Bible says | Repo has | Verdict |
|---|---|---|
| 11 named voices | 12 placeholder skills | **Data change.** Rewrite the roster; no runtime code touched. §5.2 |
| `DCBand { Trivial=8, Routine=11, Hard=14, Specialist=17, BuildDefining=20 }` | `DcBand { Trivial=5, Easy=8, Standard=11, Hard=14, Formidable=17, Legendary=20 }` | **Rename + drop one.** Same parser, same call convention. §5.3 |
| Axes Reason / Body / Social / Self | `SkillDomain { Reason, Instinct, Presence, Body }` | **Rename enum + add `Specialist`.** §5.4 |
| `targetFiringsPerHour` per skill | single global `minSecondsBetween` | **Real new behaviour.** Small, do it. §5.6 |
| `GetTotalModifier` = rank + situational + state | identical three sources, split across `PlayerSkills` (rank), the `<<check>>` argument (situational) and `SkillStateModifiers` (state) | **Already satisfied.** Keep. Add Yarn commands so writers can drive mods (§5.7). |
| `[YarnFunction("rank")]` | `[YarnFunction("skill_rank")]` | **Keep `skill_rank`.** Not player-visible; `rank` is a likely collision with future generic helpers, and `skill_rank` is already used in `Bouncer.yarn`. Note the deviation in the file header. |
| `<<check empathy 14>>` (raw int DC) | `<<check skill <band-or-int> [mod]>>` | **Already a superset.** Keep. Prefer band names in authored Yarn. |
| `CheckResult { success, roll, total, dc }` | `SkillCheckResult` with degrees + per-source mod breakdown | **Already a superset.** Keep. |
| "show the number, always" | `SkillCheckResultView` already prints roll / rank / situation / state / total / DC / outcome | **Already satisfied.** No change. |
| Bible §7 "Wrong Truth": failure grants a wrong deduction, *no failure signal in the prose*, "roll shown as failed in UI only" | UI shows the roll; prose is authored in Yarn | **Already satisfied — this is a writing rule, not a code feature.** Do not add a "quiet failure" mode. Document the convention in `Commentary.yarn` / a Yarn style comment. |
| Bible §6: documentary routes should be *skill-free*, gated on Act + errand state | n/a | **Authoring guidance, not code.** Record it in the doc header; build nothing. |
| Bible per-skill "Domains it must NOT comment on" | `CommentarySystem` only considers a skill if `Commentary_{context}_{skillId}` exists | **Already enforced structurally** — a voice with no node for a context cannot speak there. Zero code. Say so in the doc so nobody builds a forbidden-domain table. |
| Bible "cost gates, not ability gates" for PLACEMENT | n/a | **Authoring guidance.** Build nothing. |
| Bible trace channels + clue yield (§6 audit table) | n/a | **Data + an editor audit.** Cheap and it protects the coverage rule. §5.5 |
| Opposed / allied pairs | n/a | **Data + a symmetry validation.** §5.5 |
---
## 3. The skeleton, for reference
<details>
<summary>Architecture skeleton as handed over (do not implement literally — see §2)</summary>
```csharp
[CreateAssetMenu(menuName = "Skills/SkillDefinition")]
public class SkillDefinition : ScriptableObject
{
public string skillId; // e.g. "confluence" — matches flag naming convention
public string displayName; // e.g. "CONFLUENCE"
public string domain; // one-line, from the skill bible
[TextArea] public string notes; // full bible entry, for your reference in-editor
public float targetFiringsPerHour = 5f;
}
public enum DCBand { Trivial = 8, Routine = 11, Hard = 14, Specialist = 17, BuildDefining = 20 }
public class PlayerSkills : MonoBehaviour
{
private Dictionary<string, int> ranks = new();
private Dictionary<string, int> situationalMods = new(); // cleared per-scene
private Dictionary<string, int> stateMods = new(); // consumables/status effects
public int GetTotalModifier(string skillId) =>
GetRank(skillId) + situationalMods.GetValueOrDefault(skillId, 0)
+ stateMods.GetValueOrDefault(skillId, 0);
}
public struct CheckResult { public bool success; public int roll; public int total; public int dc; }
public class SkillCheckSystem : MonoBehaviour
{
public CheckResult RollCheck(string skillId, DCBand dc) { /* d20 + mod vs dc */ }
}
public class YarnSkillCommands : MonoBehaviour
{
[YarnCommand("check")] public void Check(string skillId, int dc) { /* sets $check_result, $check_roll */ }
[YarnFunction("rank")] public static int Rank(string skillId) { ... }
}
public class CommentarySystem : MonoBehaviour
{
public void RequestCommentary(string context, List<string> eligibleSkillIds)
{
// minInterval = 3600f / def.targetFiringsPerHour; skip if not refreshed;
// weighted pick; one passive per request
}
}
```
Two ideas from this skeleton are genuinely new and must be adopted: **`targetFiringsPerHour` as a
per-skill budget** (§5.6) and the **`domain` one-liner + `notes` bible blob** on the definition
asset (§5.5). Everything else is already present in better form.
</details>
---
## 4. The canonical roster
Ids are lowercase `^[a-z][a-z0-9_]*$` — they are used as Yarn command arguments and as suffixes in
Yarn node titles, so they cannot contain spaces.
**Article rule:** drop the leading "THE" from ids, **except `the_float`** — bare `float` is a C#
keyword and a Yarn type name, and would break any future generated id constants.
| # | displayName | id | axis | opposedTo | alliedWith | firings/hr | primary channel | secondary | clue yield |
|---|---|---|---|---|---|---|---|---|---|
| 1 | `PROVENANCE` | `provenance` | Reason | `confluence` | `the_float`, `room_tone` | 7 | Documentary | Physical | High |
| 2 | `CONFLUENCE` | `confluence` | Reason | `provenance` | `undisclosed`, `house_pour` | 9 | Behavioural | Testimonial | Low |
| 3 | `HOUSE POUR` | `house_pour` | Body | `long_shift` | `facework`, `amnesty` | 8 | Behavioural | Physical | Medium |
| 4 | `THE LONG SHIFT` | `long_shift` | Body | `house_pour` | `standing_order`, `room_tone` | 6 | Physical | Behavioural | Medium |
| 5 | `FACEWORK` | `facework` | Social | `placement` | `house_pour`, `room_tone` | 10 | Testimonial | Behavioural | High |
| 6 | `PLACEMENT` | `placement` | Social | `facework` | `the_float`, `confluence` | 6 | Testimonial | None | Medium |
| 7 | `AMNESTY` | `amnesty` | Self | `standing_order` | `house_pour`, `facework` | 4 | Behavioural | None | Low |
| 8 | `STANDING ORDER` | `standing_order` | Self | `amnesty` | `long_shift`, `provenance` | 5 | Testimonial | None | Low |
| 9 | `UNDISCLOSED` | `undisclosed` | Specialist | `provenance` | `confluence`, `placement` | 3 | Physical | Behavioural | Low |
| 10 | `THE FLOAT` | `the_float` | Specialist | `amnesty` | `provenance`, `placement` | 5 | Documentary | Physical | High |
| 11 | `ROOM TONE` | `room_tone` | Specialist | `placement` | `facework`, `long_shift` | 8 | Physical | Testimonial | High |
Sums and tallies that a validation pass must reproduce (Bible §6):
- Firing total = **71 / hour**.
- Primary-channel tally: physical **3**, testimonial **3**, behavioural **3**, documentary **2**.
- `opposedTo` is **symmetric within the four pairs** (1↔2, 3↔4, 5↔6, 7↔8). The three specialists'
`opposedTo` entries are **deliberately asymmetric** — `undisclosed→provenance`,
`the_float→amnesty`, `room_tone→placement` are one-directional thematic oppositions stated in the
Bible, and the targets already oppose their own pair partner. Validation must allow this: assert
symmetry only when both skills have `axis != Specialist`.
- `startingRank`: **1 for every skill.** The Bible does not assign starting ranks; character
creation is out of scope. One exception for the demo — see §5.9.
- `accentColor`: pick something plausible per axis (Reason cool blue, Body amber, Social violet,
Self pale green, Specialists desaturated). Purely cosmetic; the current values are placeholders too.
The one-line `domain` strings and the long-form `notes` blob come verbatim from the Bible's per-skill
`domain:` field and the whole `SKILL` block respectively.
### 4.1 Old → new mapping for the five existing call sites
The roster is a **replacement, not a migration**; there is no 1:1 map for all 12. Only these
call sites need a decision, and these are the decisions:
| Where | Old | New | Why |
|---|---|---|---|
| `Bouncer.yarn` — "I'm on the list" | `persuasion` | `placement` | PLACEMENT's domain is literally "the ask, the favour, the comp, **the door**". |
| `Bouncer.yarn` — "Let me through" (gate + check) | `intimidation` | `undisclosed` | UNDISCLOSED is leverage never spelled out — "He never threatened you." Exactly this beat. |
| `Debug.yarn` | `persuasion` | `placement` | as above |
| `Debug.yarn` | `deduction` | `confluence` | CONFLUENCE is the deduction engine. |
| `Commentary.yarn` — "hasn't looked at his phone once" | `paranoia` | `facework` | Reading one individual. |
| `Commentary.yarn` — "pretending they're not cold" | `read_room` | `long_shift` | Bodies paying a cost. |
| `Commentary.yarn` — "that's a price, not a queue" | `streetwise` | `the_float` | Money in motion. |
| Tests | `persuasion`, `reflexes`, `read_room`, `paranoia` | `placement`, `long_shift`, `facework`, `house_pour` | arbitrary; tests only need valid ids |
`<<check nosuchskill standard>>` in `Debug.yarn` is a **deliberate** bad-id error path. Keep the bad
id; only update its DC band name.
---
## 5. Work items
Do these in order. Each is independently compilable.
### 5.1 Add a JSON bible as the roster source of truth
**Problem:** the roster currently lives as a C# `SkillSpec[]` in an editor script. The new roster
carries ~800 characters of authored prose per voice (`domain`, `notes`). Eleven verbatim string
literals inside an editor script is unmaintainable, and the writer who owns this content should not
have to edit C#.
**Do:** create `NightclubArcadia/Assets/Skills/skill_bible.json` as the machine-readable mirror of
the Bible, and make `SkillSystemSetup` read it instead of a hardcoded array.
```jsonc
{
"skills": [
{
"id": "provenance",
"displayName": "PROVENANCE",
"axis": "Reason",
"domain": "Chains of custody. Paperwork, licences, deeds, freight stamps, serial numbers, rebuilds, previous owners. Reading an object as a descendant of the decisions that moved it.",
"notes": "<full SKILL block from the Bible, newline-escaped>",
"targetFiringsPerHour": 7,
"opposedTo": "confluence",
"alliedWith": ["the_float", "room_tone"],
"primaryChannel": "Documentary",
"secondaryChannel": "Physical",
"clueYield": "High",
"startingRank": 1,
"accentColor": { "r": 0.62, "g": 0.72, "b": 0.95, "a": 1 }
}
// ... 10 more
]
}
```
Notes:
- `JsonUtility` cannot deserialize a top-level array, hence the `skills` wrapper object. Use a
`[Serializable]` DTO; enum-valued fields are strings in JSON and parsed with `Enum.TryParse`
(fail loudly on a typo — this file is content, and a silent default is a content bug).
- Put the file under `Assets/` so it is an addressable project asset; it is editor-time-only input,
so a `TextAsset` load via `AssetDatabase.LoadAssetAtPath<TextAsset>` is fine.
- Keep the human-facing Bible in the Obsidian vault as the source of the prose; the JSON is a
derived, checked-in mirror. State this in a comment at the top of `SkillSystemSetup.cs`.
### 5.2 Rewrite `SkillDefinition`
Add fields; keep the existing style (private `[SerializeField]` + public getters + the
`#if UNITY_EDITOR EditorSet(...)` writer).
```
id string (unchanged)
displayName string (unchanged)
axis SkillAxis <- renamed from `domain`, see §5.4
domain string <- NEW: the Bible's one-line domain
notes string [TextArea] <- NEW: full Bible entry (replaces `description`)
targetFiringsPerHour float <- NEW
opposedTo string <- NEW (skill id)
alliedWith string[] <- NEW (skill ids)
primaryChannel TraceChannel <- NEW
secondaryChannel TraceChannel <- NEW
clueYield ClueYield <- NEW
startingRank int (unchanged)
accentColor Color (unchanged)
```
- Drop `description`; `notes` supersedes it. The only consumer of `description` is the placeholder
string in `SkillSystemSetup`.
- The `EditorSet(...)` signature grows past readability. Replace the positional parameter list with a
single `[Serializable]` DTO parameter (reuse the JSON DTO from §5.1) — this also makes the three
test files' helpers shorter, not longer.
- New enums, in `Data/`:
`enum TraceChannel { None, Documentary, Behavioural, Testimonial, Physical }`
`enum ClueYield { Low, Medium, High }`
Keep the British `Behavioural` spelling — the Bible uses it and it will be grepped against the
Bible constantly.
### 5.3 `DcBand` → the five target bands
```csharp
public enum DcBand
{
Trivial = 8,
Routine = 11,
Hard = 14,
Specialist = 17,
BuildDefining = 20,
}
```
- The old `Trivial = 5` is **removed**, and `Trivial` is **reused at 8**. No call site uses
`trivial` today, so nothing silently changes value — but flag this in the commit message, because
it is exactly the kind of rename that would silently rebalance an existing script.
- `DcBands.TryParse` keeps accepting raw integers (`<<check confluence 16 2>>` is used in
`Debug.yarn`, and the skeleton's own example passes a raw int). The Bible says "never use
in-between values", so **add a `Debug.LogWarning` when a parsed integer is not one of the five band
values** — a nudge for the author, not an error. Do not make it throw.
- Do **not** rename the C# type to `DCBand`; `DcBand` matches the project's existing casing
convention and is not player-visible.
### 5.4 `SkillDomain` → `SkillAxis` (a terminology collision worth fixing)
The Bible uses "domain" to mean *the subject matter a voice covers* ("Chains of custody…"), while the
current enum uses it to mean *the axis a skill sits on*. Leaving both named `domain` will cause
persistent confusion for the writer.
```csharp
public enum SkillAxis { Reason, Body, Social, Self, Specialist }
```
- Rename the file `Data/SkillDomain.cs` → `Data/SkillAxis.cs` (and its `.meta`).
- Rename the `SkillDefinition` field `domain` → `axis` and free the name `domain` for the Bible's
one-liner (§5.2). Because the definition assets are regenerated wholesale in §5.8,
`[FormerlySerializedAs]` is not needed — but the *serialized value* changes meaning
(old `domain: 2` meant `Presence`), so the old assets **must be deleted, not edited in place**.
### 5.5 Roster validation in `SkillDatabase.OnValidate`
Extend the existing duplicate/empty-id checks (do not create a new validator class) with:
1. Every `id` matches `^[a-z][a-z0-9_]*$`.
2. `opposedTo` and every `alliedWith` entry resolve to a skill in this database.
3. `opposedTo` is symmetric **when both skills have `axis != Specialist`**.
4. Exactly one pair per axis for `Reason` / `Body` / `Social` / `Self` (2 skills each), and 3
`Specialist`.
5. `targetFiringsPerHour > 0` for every skill; log the **sum** at info level so drift from the
Bible's 71 is visible.
6. Log the primary-channel tally so drift from `physical 3 / testimonial 3 / behavioural 3 /
documentary 2` is visible. Bible §6 flags documentary as the scarce channel and the whole
coverage rule depends on it.
Rules 5 and 6 are **informational logs, not errors** — they are design budgets the writer is
expected to adjust deliberately.
### 5.6 Per-skill firing budgets in `CommentarySystem`
This is the one genuinely new runtime behaviour. Keep everything else about the class — including
the node-existence gate, the `_recent` recency halving, and the public static `Pick(...)` that the
tests exercise.
Change `RequestCommentary(contextId, repeatable)`:
- Add `readonly Dictionary<string, float> _lastFiredBySkill`.
- A skill is a candidate only if, in addition to the existing gates
(non-null, non-empty id, `rank >= minRankToSpeak`, node exists):
`Time.time - _lastFiredBySkill.GetValueOrDefault(id, float.NegativeInfinity) >= 3600f / def.TargetFiringsPerHour`.
- Guard `TargetFiringsPerHour <= 0` (treat as "never fires", and let `OnValidate` complain).
- Keep the existing global `minSecondsBetween` floor. It is the aggregate limiter; 71 firings/hour is
roughly one per minute, so a 20s global floor and per-skill budgets are complementary, not
redundant.
- On a successful fire, set `_lastFiredBySkill[picked.Id] = Time.time` alongside the existing
`_lastFiredAt` / `_firedContexts` / `_recent` bookkeeping.
- Keep `Pick(...)`'s signature and weighting unchanged so `CommentaryPickerTests` stays valid.
Budget eligibility belongs in the candidate filter, not in the weighting.
**Optional, only if it costs nothing:** an ambient fallback. If no `Commentary_{context}_{id}` node
exists for any eligible skill, look for `Commentary_Ambient_{id}` — the Bible gives every voice one
`PASSIVE VOICE` line that is context-free. Implement only if it does not complicate the candidate
loop; otherwise leave it out and note it as future work.
### 5.7 Yarn commands for situational / state modifiers
The skeleton exposes `SetSituationalMod` / `ClearSituationalMods`. The repo's `SkillStateModifiers`
(source-keyed, wildcard-capable) is strictly more capable, but there is currently **no way for a
writer to set a modifier from Yarn** — the only situational input is the third argument to
`<<check>>`. Close that gap in `YarnSkillCommands`:
```
<<skill_mod scene facework 2>> // set: source key, skill id (or "*"), delta
<<clear_skill_mods scene>> // clear one source key
```
Convention (document it in the file header and in `Common.yarn`):
**source key `scene` is the "situational" bucket** and must be cleared on scene exit;
any other source key (`drunk`, `injured`, …) is the "state" bucket. This preserves the
"exactly three modifier sources" rule from the design doc without adding a fourth dictionary.
Both commands must fail soft (log an error, do not throw) when `SkillRuntime.Instance` is null,
matching the existing `<<check>>` error handling.
### 5.8 Regenerate the assets
The definition assets must be **replaced**, not edited — the meaning of the serialized `domain`
integer changes (§5.4) and the roster is not a 1:1 map.
**Preferred path (Unity available):**
1. Delete `NightclubArcadia/Assets/Skills/Definitions/` entirely (all 12 `.asset` + `.meta`).
2. Add a cleanup step to `SkillSystemSetup`: after generating, delete any `.asset` in
`Definitions/` whose filename is not in the roster. This prevents the exact orphan problem that
made this step manual.
3. Run the menu item headlessly:
```bash
/Applications/Unity/Hub/Editor/6000.5.8f1/Unity.app/Contents/MacOS/Unity \
-batchmode -quit -nographics \
-projectPath /Users/lennart/Dev/nightlcub-arcadia/NightclubArcadia \
-executeMethod SkillSystemSetup.SetUpSkillSystem \
-logFile -
```
Note `SetUpSkillSystem` also opens and re-saves `Assets/Scenes/DialogueTest.unity`. That is fine
and idempotent, but it will produce scene diff noise — check the scene diff before committing and
revert it if it is pure reserialization.
**Fallback path (Unity unavailable):** hand-author the YAML. Each definition is a single
`MonoBehaviour` asset; copy the shape of an existing file
(`Assets/Skills/Definitions/persuasion.asset`), keep
`m_Script: {fileID: 11500000, guid: 9a26230b6f20436891aa772e8f858f3f, type: 3}` (the
`SkillDefinition` script GUID — verify it against `Scripts/Skills/Data/SkillDefinition.cs.meta`),
and write a fresh 32-hex-char `guid:` into each `.meta`:
```yaml
fileFormatVersion: 2
guid: <32 hex chars>
NativeFormatImporter:
externalObjects: {}
mainObjectFileID: 11400000
userData:
assetBundleName:
assetBundleVariant:
```
Then rewrite `Assets/Skills/SkillDatabase.asset`'s `skills:` list to
`- {fileID: 11400000, guid: <that guid>, type: 2}` in roster order. Multi-line `notes` in YAML is
the only fiddly part — use a block scalar or escape newlines as `\n` in a double-quoted scalar,
matching how Unity already escapes the em-dash in `persuasion.asset`.
### 5.9 Update the Yarn call sites
Apply the §4.1 mapping.
- `Debug.yarn`: `<<check placement routine>>`, `<<check confluence 16 2>>`,
`<<check nosuchskill routine>>` (keep the deliberate bad id).
- `Bouncer.yarn`: `<<check placement routine>>`; `skill_rank("undisclosed") >= 2`;
`<<check undisclosed hard -1>>`. Give `undisclosed` `startingRank: 2` in the JSON bible so the
gated option is reachable in the demo — this is the one starting-rank exception, and it replaces
the identical existing exception for `intimidation`. Comment it in the JSON.
- `Commentary.yarn`: rename the three nodes to `Commentary_Entrance_Queue_{facework,long_shift,the_float}`
and **rewrite the three lines in each voice's register**. The current lines are placeholders. Use the
Bible's `PASSIVE VOICE` samples as the model — e.g. ROOM TONE gives direction before content and
transcribes with the gaps marked; THE FLOAT converts everything to a nightly rate and never
moralises. Also drop the `"Paranoia: "` / `"Read the Room: "` speaker prefixes if the display name
is already surfaced elsewhere; if not, use the exact `displayName` (`THE FLOAT:`, `FACEWORK:`).
- `Common.yarn`: no `$check_*` change needed. Add a short comment block recording the two writing
rules that the code deliberately does not enforce (Bible §7 / §2.1 above):
*Wrong Truth failures carry no failure signal in the prose — the UI banner is the only tell;*
*documentary routes are gated on Act and errand state, never on a voice.*
### 5.10 Update the tests, then add three
Existing tests need mechanical fixes:
- `DcBandTests`: `"standard" -> 11` becomes `"routine" -> 11`. Add a case asserting `"trivial" -> 8`
(not 5) so the reused name is pinned.
- `SkillCheckSystemTests`, `CommentaryPickerTests`: the `EditorSet(...)` helper calls change to the
DTO form (§5.2), and the literal skill ids change per §4.1. No assertions about check maths should
change — if one does, you have regressed the implementation. Stop and re-read §2.
New tests worth the keystrokes:
1. **Firing budget gating** (`CommentarySystem`): a skill with `targetFiringsPerHour = 3600` (one per
second) is eligible again immediately; a skill with `targetFiringsPerHour = 1` is not. This needs
the budget check to be reachable without a `DialogueRunner` — extract the eligibility predicate
into a `public static bool IsOffCooldown(float lastFired, float now, float targetPerHour)` and
test that, rather than trying to instantiate the MonoBehaviour.
2. **Roster shape**: load `Assets/Skills/SkillDatabase.asset` in an EditMode test and assert 11
skills, id regex, pair symmetry (non-specialists), the axis counts, the 71 firing total, and the
channel tally. This is the test that catches a JSON typo before it reaches the writer.
3. **Id/display divergence**: assert `displayName` is the Bible's uppercase form and `id` is the
snake_case form for all 11 — the two are intentionally different and easy to accidentally unify.
Run them:
```bash
/Applications/Unity/Hub/Editor/6000.5.8f1/Unity.app/Contents/MacOS/Unity \
-batchmode -quit -nographics \
-projectPath /Users/lennart/Dev/nightlcub-arcadia/NightclubArcadia \
-runTests -testPlatform EditMode -testFilter "NightclubArcadia.Skills.Tests" \
-testResults /tmp/skill-tests.xml -logFile -
```
---
## 6. Explicit non-goals
Do not build any of these. Each is either already satisfied or is a writing convention:
- A "quiet failure" / suppressed-failure-signal check mode. The UI banner *is* the tell; the prose
is the writer's job (Bible §7).
- A forbidden-domains table per skill. Node absence already enforces it (§2.1).
- Clue / deduction / evidence objects (`CL-0xx`, `DED-0xx`, criticality, routes). The Bible §7
worked example references a clue ledger from a separate handbook. **That system does not exist in
this repo and is out of scope.** Do not invent a schema for it.
- Act / errand state, or any gating for documentary routes.
- Character creation, rank progression, or skill-point spending.
- Converting `PlayerSkills` / `SkillCheckSystem` into MonoBehaviours to match the skeleton. They are
plain C# classes precisely so they can be unit-tested; the skeleton is a sketch, not a target.
- Renaming `skill_rank` to `rank`, or `SkillCheckResult` to `CheckResult`.
---
## 7. Traps
### 7.1 `[DidReloadScripts]` will overwrite hand-edited assets
`SkillSystemSetup.AutoSetupAfterReload` calls `EnsureSkillAssetsOnly()` once per editor session,
which regenerates every definition asset and the database from the roster. **If you hand-edit an
`.asset` without updating the roster source (now `skill_bible.json`), your edit will be silently
reverted the next time scripts reload.** The JSON is the source of truth; assets are build output.
Consider adding that sentence as a comment at the top of every generated asset's owning script.
### 7.2 Skill ids are Yarn node-title fragments
`CommentarySystem` builds `Commentary_{contextId}_{skillId}`. Yarn node titles must be valid
identifiers. This is the reason for the id regex in §5.5 — an id with a space or a hyphen produces
a node title that either fails to compile or silently never matches.
### 7.3 Lookups are case-insensitive, node names are not
`SkillDatabase` and `PlayerSkills` use `StringComparer.OrdinalIgnoreCase`; `CommentarySystem`'s
`NodeExists` uses `StringComparison.Ordinal`. So `<<check PLACEMENT routine>>` works but
`Commentary_Entrance_Queue_PLACEMENT` does not. Keep every authored id lowercase.
### 7.4 The scene references the database, not the definitions
`Assets/Scenes/DialogueTest.unity` wires `SkillRuntime.database` → `SkillDatabase.asset`, and
`CommentarySystem.database` likewise. Replacing definition assets does not break the scene, provided
`SkillDatabase.asset` keeps its own GUID. **Do not delete or recreate `SkillDatabase.asset`** —
rewrite its `skills:` list in place.
### 7.5 `float` is a keyword
Covered in §4, restated because it will be tempting: the id for THE FLOAT is `the_float`.
---
## 8. Definition of done
- [ ] `skill_bible.json` contains all 11 voices with every field in §4, prose copied from the Bible.
- [ ] `Assets/Skills/Definitions/` contains exactly 11 assets, named `<id>.asset`; no orphans.
- [ ] `SkillDatabase.asset` keeps its original GUID and lists the 11 in roster order.
- [ ] `DcBand` has exactly the five target bands; a non-band integer DC logs a warning.
- [ ] `SkillAxis` replaces `SkillDomain`; `SkillDefinition.domain` is now the Bible one-liner.
- [ ] `CommentarySystem` enforces a per-skill `3600 / targetFiringsPerHour` budget on top of the
existing global floor.
- [ ] `<<skill_mod>>` / `<<clear_skill_mods>>` exist and fail soft.
- [ ] No Yarn file, test, or script references any of the 12 old skill names.
- [ ] EditMode tests pass, including the three new ones in §5.10.
- [ ] `grep -ri` for each old id across `NightclubArcadia/Assets` (excluding `Library/`) returns
nothing.
- [ ] The check maths in `SkillCheckSystem` is byte-for-byte unchanged apart from the DC band
constants. If it is not, §2 was violated.