Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

Add higher education institutions - #352

Merged
philoserf merged 5 commits into
mainfrom
agent/chargen-education
Jul 22, 2026
Merged

philoserf merged 5 commits into
mainfrom
agent/chargen-education

Conversation

@philoserf

Copy link
Copy Markdown
Owner

Summary

  • transcribe the complete seven-column Available Skills matrix
  • add Army, Navy, and Marine service academies
  • add Medical School, Law School, Honors, OTC/NOTC, and Flight School
  • add assigned Army/Navy/Marine schools and Command College
  • record graduation, honors, commission, and Flight status in education history
  • preserve the established College/University/graduate-school flow and dice order

Why

Character education previously stopped at the academic ladder and Trade School. The remaining institutions need explicit, testable operations so policy can select them without hiding branch choice, waivers, or assigned military timing inside the existing generator.

Impact

Existing character generation remains unchanged unless a caller explicitly invokes AttendInstitution. Institution selection and waiver decisions remain policy-controlled.

The source labels both Medical School and Marine Academy with M; this implementation gives them distinct typed columns and regression tests so their skill lists cannot silently merge.

Validation

  • task check
  • zero lint issues
  • all repository tests pass

Closes #193

@philoserf

Copy link
Copy Markdown
Owner Author

Reviewed — leaving in draft, not merge-ready yet.

task check passes clean, scope matches issue #193, and the tests aren't fail-open (e.g. TestPrerequisiteWaiverIsPolicy scripts exactly one die so a removed gate would panic rather than silently pass). But internal/chargen/education_institutions.go breaks this repo's own citation convention: every sibling function in education.go cites a Book 1 page for its numeric rules, while this 367-line file has exactly one citation total (the file header). Every new magic number is uncited — the Edu<6 admission floor, Medical/Law School durations and skill bumps, the Honors BA+Int gate, Flight School's Dex-check, Command College's admission/check/skill mechanic, and the ANM School's Dex/End check plus an invented intersection-of-columns skill rule with no citation at all.

Given this codebase's history with mis-transcribed tables (drive-cost, IsBelt, etc.), these need page citations before a reviewer can verify them against the book. Minor/non-blocking: OTC and the Army/Marine ANM-School branches are untested (only NOTC/NavySchool are exercised).

Suggest adding "Book 1 p.N" citations to each institution's rule and re-requesting review.

education_institutions.go had one book citation for its entire 367 lines,
against sibling education.go's convention of citing a specific page per
numeric rule. Add Book 1 p.59-62 citations to every institution's
prerequisite, check, duration, and provides rule, verified against the
PDF (not the markdown extract). The ANM School skill-list composition
(School-column intersect service-column) turns out to have no printed
basis — the p.60 chart just says "School=ANM" — so it is now documented
as this package's own rule rather than given a fabricated citation.

Co-Authored-By: Claude
@philoserf
philoserf marked this pull request as ready for review July 22, 2026 21:58
Kill duplication flagged by /simplify: attendProfessional hand-rolled the
same Edu-bump/degree logic graduate() already provides; awardAcademicPass
repeated the same available-pool fallback for Major and Minor; AvailableSkills
and attendANMSchool each rescanned the matrix with a per-entry []byte
allocation instead of a shared string-scan helper; matrixColumn's
MarineAcademy case duplicated its own default branch.
/code-review high found five confirmed issues, all fixed:
- one-shot institutions (Honors, Service Academy, Medical/Law School,
  Flight School) had no guard against being attended twice, letting a
  repeat AttendInstitution call silently stack a one-time bonus
- Command College pooled skills from both the Military and Naval
  Academy regardless of the character's own service; it now infers the
  right academy from the most recent Commission-bearing EducationRecord
- attendHonors granted a Waiver on a failed check, contradicting its own
  cited rule ("Failure has no effect", Book 1 p.59) — every other
  one-shot check in this file correctly omits a waiver
- three matrix/skill-list entries used non-canonical skill names
  (Astrogator, Hvy Wpns, Naval Arch) instead of this codebase's
  Astrogation/Heavy Weapons/Naval Architect, silently orphaning any
  skill grant that picked one of them
- added a test for the untested Edu<6 Service-Academy waiver-gate branch

Each fix is backed by a regression test; each was verified by temporarily
reintroducing the bug and confirming the new test fails.
@philoserf
philoserf merged commit d03eb79 into main Jul 22, 2026
1 check passed
@philoserf
philoserf deleted the agent/chargen-education branch July 22, 2026 22:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chargen — higher / professional / military education institutions (#14)

1 participant