review

name

review

description

General self-review for OpenChaos — runs after ANY code changes, not just before commits. Use this skill whenever work is done: after a migration iteration, after fixes, after any edits. The user commits manually and may not commit at all — review is still mandatory every time code is touched. Also use when the user explicitly asks for review. Trigger on: "проверяй", "ревью", "готово", "самопроверка", "review", "check", "проверь", "посмотри что поменял", or automatically after completing any batch of code changes. For Stage 4 migration, also trigger stage4-review.

Self-Review — mandatory after any code changes

Run this after finishing ANY batch of edits. Not just before commits — after every piece of work. No shortcuts. Every file, every hunk, fresh eyes.

Step 1: Gather all changes

git diff          # staged + unstaged changes in existing files
git status        # new untracked files — also part of the review scope

Read the full output. Open and read every changed and new file. Review as a reviewer, not as the author.

Step 2: General checks

Dangling if/else — the silent killer

Before ANY line deletion, check: is it the sole body of if/else/while/for without braces?

if (condition)
    deleted_line();   // you delete this
next_statement();     // now captured by the if! Logic silently changed.

The compiler won't warn. This silently changes program logic.

When deleting the body of a braceless if/else — delete the if/else itself too, or add {} / ;.

This is especially dangerous in mass operations. Never delete lines in bulk without checking every context (+-5 lines).

Code correctness

• Logic unchanged — not simplified, not "improved", not refactored beyond what was requested
• No accidental deletions of needed code
• No broken control flow from removed lines
• String/path literals correct

Reuse / no unjustified duplication

Scan the changed code for the same logic repeated across several cases — multiple paths that each do "the same thing" (e.g. open the popup + equip a weapon copied into five different handlers). That belongs in ONE place (or a small set of shared helpers) that every case calls, not copy-pasted per case. Watch for:

• The same sequence of calls repeated with only the arguments differing → extract a helper taking those arguments.
• Two functions identical except one extra step → make one call the other.
• A "do X" path and a "show X without doing it" path → usually the same primitive, parameterised.

Why: duplicated logic drifts — a fix or tweak lands in one copy and silently not the others, and the cases diverge over time. One shared path means one place to change and one behaviour to reason about. Reuse to the maximum; keep things separate only when consolidating would genuinely obscure intent.

Documentation consistency

• If you changed behavior during the session (e.g. added then reverted something, changed approach) — re-read ALL documentation updates made in the same session and verify they describe the final state, not an intermediate one
• Common mistake: doc says "ASSERT replaced with skip" but code still has ASSERT because it was restored later. The doc was written for an intermediate state and never updated after the final change

Compilation

• If code was changed: make build-release must pass
• If only docs/config changed: compilation check not needed

Step 3: Verify completeness

• Every file that should have been changed — was changed
• Every file that should NOT have been changed — was not changed
• No stray debug prints or temporary code left behind

Gameplay changes → GAMEPLAY_CHANGES.md

If changes affect gameplay, controls, cheats, mechanics, or anything the player would notice while playing — update GAMEPLAY_CHANGES.md in the project root.

What to write: description from the player's perspective. What changed, how it works now, what buttons to press, what the player will see on screen.

What NOT to write: implementation details, function names, flags, variable names, file paths, technical reasoning. The player doesn't care how it's coded. Graphics/rendering changes also don't go there — only gameplay, controls, cheats, mechanics. Don't detail how existing features work (e.g. button layouts, stick behavior) — only record what was CHANGED, ADDED or REMOVED compared to the original. Detailed feature descriptions belong in separate documentation, not here.

Step 4: Report

After confirming all checks pass:

"Самопроверка пройдена: [N] файлов проверено, всё ок."

If any issue is found — fix it first. Never suggest committing with known issues.

Stage-specific extensions

On Stage 4 (migration): also run stage4-review which adds checks A-H (uc_orig, globals, entity mapping, include guards, comments, CMake, empty files, DAG order).