Refactoring

.vscode/tasks.json

@@ -321,6 +321,54 @@
             ],
             "problemMatcher": []
         },
+        {
+            "label": "Analyze :: Script Quality",
+            "type": "shell",
+            "command": "py",
+            "windows": {
+                "command": "py",
+                "args": [
+                    "-3",
+                    "${workspaceFolder}/Tools/ScriptQuality/validate_scripts.py",
+                    "--summary"
+                ]
+            },
+            "linux": {
+                "command": "python3",
+                "args": [
+                    "${workspaceFolder}/Tools/ScriptQuality/validate_scripts.py",
+                    "--summary"
+                ]
+            },
+            "options": {
+                "cwd": "${workspaceFolder}"
+            },
+            "problemMatcher": []
+        },
+        {
+            "label": "Analyze :: Script Quality (Ratchet)",
+            "type": "shell",
+            "command": "py",
+            "windows": {
+                "command": "py",
+                "args": [
+                    "-3",
+                    "${workspaceFolder}/Tools/ScriptQuality/validate_scripts.py",
+                    "--ratchet"
+                ]
+            },
+            "linux": {
+                "command": "python3",
+                "args": [
+                    "${workspaceFolder}/Tools/ScriptQuality/validate_scripts.py",
+                    "--ratchet"
+                ]
+            },
+            "options": {
+                "cwd": "${workspaceFolder}"
+            },
+            "problemMatcher": []
+        },
         {
             "label": "Bake Resources",
             "type": "shell",

AGENTS.md

@@ -189,6 +189,7 @@ AngelScript embedded in screens lives in the `.fogui` JSON: `OnGlobalMouseDown`,
 
 - Use `Format :: Scripts`, `Format :: Prototypes`, `Format :: Main Config`, or `Format :: All` before handing off when touched files need formatting.
 - `FormatSource.bat` is a smaller formatter path for `Scripts/*.fos`, `Scripts/Json/*.fos`, `SourceExt/*`, and `Gui/*.fogui`.
+- `Tools/ScriptQuality/validate_scripts.py` is a quality *validator* (reports only; not a formatter) for `Scripts/*.fos`: banner tags, Cyrillic comments, magic text-pack ids, hand-rolled-util calls, redundant bool returns, commented-out code, `namespace`==filename, `#if` balance, component `== null` probes, trailing blank line. Run `Analyze :: Script Quality` for a summary; `--ratchet` fails only on new violations vs `Tools/ScriptQuality/baseline.json`; `--fix` applies the few safe autofixes. See `Tools/NullableEstimate/validate_nullable.py` for the complementary `?`/FO_NULLABLE checks.
 - Do not hand-edit generated files: `Scripts/Content.fos`, generated `Scripts/GuiScreens.fos` without the matching `.fogui` update, baked output under `Baking/`, cache files under `Cache/`, or generated `VERSION`.
 - Local working trees such as `TLA-Dev/`, `Baking/`, `Cache/`, and build folders are outputs/debug state, not canonical authored inputs.
 
@@ -215,6 +216,7 @@ Pick the boundary before reaching for a heavy interactive session:
 ## Quick Reference
 
 - `README.md` - repo-root overview.
+- `Docs/Refactoring.md` - Scripts/*.fos refactoring plan, phases, and running status.
 - `TLA.fomain` - main config and subconfigs (`LocalTest`, `PublicGame`, resource baking subconfigs).
 - `.vscode/tasks.json` - authoritative build/generate/format/launch tasks.
 - `.vscode/launch.json` - debugger launch profiles when present.

Critters/TimCain.focr

@@ -2,7 +2,6 @@
 $Name = TimCain
 $Parent = nmpeas
 
-TalkDistance = 8
 StrengthBase = 10
 PerceptionBase = 9
 EnduranceBase = 10
@@ -39,6 +38,7 @@ IsNoDrop = True
 IsNoBarter = True
 IsNoSteal = True
 IsNoLoot = True
+TalkDistance = 8
 
 $Text russ = "Тим Кейн"
 $Text engl = "Tim Cain"

Docs/Refactoring.md

@@ -0,0 +1,118 @@
+# Scripts Refactoring Plan
+
+This document is the plan and running status for the end-to-end refactor of the TLA AngelScript
+gameplay layer (`Scripts/*.fos`). The code is old, has been through many engine migrations, and is
+inhomogeneous (mixed idioms, dead/relic code, latent bugs). The goal is tidy, reliable, correct
+code: ordered modules, clearer naming and readability, fewer stray comments (more where they help),
+and bugs fixed with the original intent restored.
+
+Scope is `Scripts/*.fos` (≈262 editable files) plus the supporting validators under
+`Tools/ScriptQuality/`. `Scripts/Content.fos` and `Scripts/GuiScreens.fos` are generated — do not
+hand-edit (see [AGENTS.md](../AGENTS.md)).
+
+## Principles and constraints
+
+- **lf-30 (`H:/lf-30`) is a STYLE/idiom reference, not a source of content or names.** TLA owns its
+  serialized properties (e.g. `CurrentHp`/`MaxLife`); do not rename toward lf-30 equivalents.
+- **Committed content is English** (code, comments, docs), even though working discussion is in
+  Russian. Do not mass-translate existing Russian rationale comments; translate only when already
+  editing the surrounding line.
+- **Refactor carefully — no thoughtless bulk edits.** In particular, never bulk-delete commented-out
+  code: some of it is a migration breadcrumb that may still need porting. Surface, don't delete.
+- **Server holds authoritative gameplay state.** Keep behavior-preserving changes behavior-preserving;
+  fix bugs deliberately, verifying each against the code (and git history where load-bearing).
+- **Do not commit/stage/push** unless explicitly asked; the repo owner reviews and commits.
+- **Every step is verified:** Compile AngelScript (0 warnings — warnings are failures) → Bake
+  Resources → relevant `Build :: TLA_*`. Behavior-changing server work additionally runs
+  `TLA_ServerHeadless` to `"Start server complete!"`. After moving `///@ Property` declarations use
+  **Force Bake** (incremental bake leaves stale proto/map layout).
+
+## Approved decisions
+
+1. **Split `Tla.fos`** (a ~2400-line god-module holding most of the project's `///@` metadata plus
+   shared helpers) into domain modules, with full bake + headless verification.
+2. **Serialized-name alignment** to a single TLA standard and typo fixes, packaged through
+   `///@ MigrationRule`. Cross-project-sensitive renames are confirmed case by case.
+3. **Process:** validators → full audit → phased implementation.
+
+## Phases
+
+- **Phase 0 — Tooling & audit.** Quality validators (report-only, `--baseline`/`--ratchet`/`--fix`),
+  a full module audit, and a baseline snapshot.
+- **Phase 1 — Safe cleanups.** Banner/divider removal, obvious dead code, comment hygiene — strictly
+  no behavior change. Commented-out code is preserved (surfaced, not deleted).
+- **Phase 2 — Idiom modernization.** Replace hand-rolled utilities with native/engine equivalents
+  (`UtilsForArray`→native array ops, `Tla::` math→`Math::`), prefer named keys over magic numbers,
+  flag-soups → enums (non-serialized only). Behavior-preserving.
+- **Phase 3 — Bug fixes.** Each fix re-verified by reading the code and, where load-bearing,
+  cross-checked against git history to restore the original intent.
+- **Phase 4 — Structural.** Split god-modules; (future) parallel arrays → structs, `any[]` tables →
+  typed data.
+
+Module order: low-coupling leaves first, core (`Tla`/`Main`/`Parameters`) last. Each change is
+compiled, baked, and smoke-tested as above.
+
+## Validators and verification
+
+`Tools/ScriptQuality/validate_scripts.py` is a report-only quality validator (not a formatter) for
+`Scripts/*.fos`: trailing-blank-line, `namespace`==filename, preprocessor-guard balance, component
+`== null` probes (errors); banner tags, textpack magic ids, hand-rolled utils, Cyrillic comments,
+redundant bool returns, commented-out code, file-too-large (warnings). Modes: `--summary`,
+`--baseline`, `--ratchet` (fail only on new violations vs `baseline.json`), `--fix` (safe autofixes).
+Run via the VS Code task `Analyze :: Script Quality`. See also `Tools/NullableEstimate/`.
+
+Adversarial bug-hunting uses read-only finder agents over modules (whole-file or line-range chunks
+for the giants), then independent skeptic agents that try to refute each finding; only findings that
+survive majority verification are applied, after a manual re-check.
+
+## Status
+
+- **Phase 0 — done.** Validators in `Tools/ScriptQuality/`; full audit recorded under `Build/_audit/`
+  (gitignored, local source of truth); baseline established.
+- **Phase 1 — done** (decluttering: banners/dividers removed; commented-out code preserved).
+- **Phase 2 — done** for the high-value items: `UtilsForArray.fos` deleted (8 callers → native
+  `.find`/`.insertLast`); ≈145 `Tla::Clamp/Min/Max/Abs` → `Math::` (type-aware; a few `any`/`double`
+  call sites intentionally left on `Tla::`). Remaining low-value idioms (textpack magic ids, some
+  flag-soups) deferred.
+- **Phase 3 — done.** Audit-driven passes fixed ≈87 verified crit/high bugs across ≈63 files; a later
+  adversarial bug-hunt (rounds over under-reviewed and giant modules) added ≈33 more, plus a
+  systematic cluster of **26 `? StopChain : StopChain` EventResult-polarity** fixes and 4 architectural
+  fixes (`ArroyoMynocDefence` stale timer, `NrWriKidnap` Location→Critter quest property, `SfInvasion`
+  re-enabled `OnDead`, `Patrol` per-instance registry). Highlights include `ItemMovement` (all item
+  moves were blocked), `Entrance::GetFreeHex` (out-of-bounds crash), NPC plane-AI inversions, AP
+  scaling, perk loss, and several economy/quest defects. All verified (compile + bake + headless smoke).
+- **Phase 4 — done.** `Tla.fos` split from ~2410 lines to a 675-line core, with metadata and helpers
+  relocated to domain modules:
+  - Metadata (zero caller churn — properties/enums/settings/events are accessed unqualified):
+    `CritterProps`, `ItemProps`, `GameProps`, `GameSettings`, `GameEvents`, `GameEnums`.
+  - Helpers (`Tla::` references renamed to the new namespace): `AnimHelpers`, `GameTime` (merged into
+    the pre-existing module), `WeaponHelpers`, `Flags`.
+  - Cross-cutting core kept in `Tla.fos` (`MaxSkillValue`, `RootContainerStack`, `AP_DIVIDER`,
+    `GetCritPropsDict`, the `Chosen*` action ids, the `Min/Max/Clamp/Abs` math, `Elevator*`,
+    `Fixboy*`, dialog helpers, `GlobalProcess*`).
+
+  Save-safety was confirmed against the engine: disk/DB persistence is keyed by property **name**
+  (`PropertiesSerializator::SaveToDocument`), so relocating a `///@ Property` declaration (without
+  renaming it) does not change the serialized contract; `regIndex` is used only for same-build network
+  sync. No `MigrationRule` was required for the relocation.
+
+## Remaining / deferred
+
+These need runtime checks, design decisions, or content/owner input and were deliberately not changed:
+
+- **Idioms (low value):** textpack magic ids → named keys; flag-soups → enums; the few `any`/`double`
+  `Tla::Clamp` call sites.
+- **Migration debt (disabled subsystems):** racing event (`Main.fos` init commented out, plus a
+  cross-file dialog property rename), `Caravan`/`GameEvent`/`NoPvpMaps` inits, lost `Say`/`SayMsg`
+  player feedback, broken `CustomCall` hotkeys. Re-enabling needs to confirm *why* each was disabled.
+- **Per-module backlog:** medium/low findings in `Build/_audit/`.
+
+## Notes / lessons
+
+- After relocating `///@ Property` declarations, run **Force Bake** — the incremental bake reports
+  "baked 0 files" and leaves protos/maps on the old layout, producing transient startup errors.
+- When creating a new domain module, verify the file does not already exist
+  (`git cat-file -e HEAD:Scripts/<name>.fos`) before writing it — there is a pre-existing `GameTime`
+  module; new metadata went into genuinely new files.
+- Moved helpers that call symbols still resident in `Tla.fos` must qualify those calls (e.g.
+  `Tla::MaxSkillValue`, `Tla::Max`); the compiler catches any that are missed.

Gui/Aim.fogui

@@ -44,9 +44,9 @@
             Item? item = chosen.GetItem(ItemProperty::CritterSlot, CritterItemSlot::Main);
             ProtoItem? proto = CritterItem::GetSlotItemProto(chosen);
             int mode = (item == null ? chosen.HandsItemMode : item.Mode);
-            int use = Tla::WeaponModeUse(mode);
-            int aim = Tla::WeaponModeAim(mode);
-            mode = Tla::MakeWeaponMode(use, loc);
+            int use = WeaponHelpers::WeaponModeUse(mode);
+            int aim = WeaponHelpers::WeaponModeAim(mode);
+            mode = WeaponHelpers::MakeWeaponMode(use, loc);
             int hit = ClientMain::ToHit(chosen, cr, proto, mode);
             return (hit == 0 ? "-" : "" + hit);
         }

Gui/Game.fogui

@@ -118,7 +118,7 @@
                 Item? realWeapon = chosen.GetItem(ItemProperty::CritterSlot, CritterItemSlot::Main);
                 int mode = (realWeapon != null && useItem != null ? useItem.Mode : chosen.HandsItemMode);
                 if (cr != null) {
-                    if (isAttack && Tla::WeaponModeAim(mode) != 0) {
+                    if (isAttack && WeaponHelpers::WeaponModeAim(mode) != 0) {
                         //IsCritterCanAim( chosen.ModelName ) &&
                         if (!chosen.IsNoAim) {
                             dict<string, any> dict = {};
@@ -132,12 +132,12 @@
                                                          0,
                                                          ClientDefines::TARGET_CRITTER,
                                                          cr.Id,
-                                                         isAttack ? mode : Tla::USE_USE,
+                                                         isAttack ? mode : WeaponHelpers::USE_USE,
                                                          0});
                     }
                 }
                 else if (item != null && useItem != null) {
-                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, useItem.Id, 0, ClientDefines::TARGET_ITEM, item.Id, Tla::USE_USE, 0});
+                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, useItem.Id, 0, ClientDefines::TARGET_ITEM, item.Id, WeaponHelpers::USE_USE, 0});
                 }
             }
         }
@@ -2615,16 +2615,16 @@
             ProtoItem? proto = CritterItem::GetSlotItemProto(chosen);
             int mode = (useItem == null ? chosen.HandsItemMode : useItem.Mode);
             if (proto != null) {
-                int use = Tla::WeaponModeUse(mode);
+                int use = WeaponHelpers::WeaponModeUse(mode);
                 if (proto.Type == ItemType::Weapon) {
-                    if (use == Tla::USE_RELOAD) {
+                    if (use == WeaponHelpers::USE_RELOAD) {
                         usePic = "art/intrface/RELOAD.FRM".hstr();
                     }
-                    else if (use == Tla::USE_USE) {
+                    else if (use == WeaponHelpers::USE_USE) {
                         usePic = "art/intrface/USEON.FRM".hstr();
                     }
-                    else if (use < Tla::MAX_USES) {
-                        usePic = Tla::WeaponPicUse(proto, use);
+                    else if (use < WeaponHelpers::MAX_USES) {
+                        usePic = WeaponHelpers::WeaponPicUse(proto, use);
                     }
                 }
                 if (usePic == EMPTY_HSTRING && proto.IsCanUseOnSmth) {
@@ -2900,7 +2900,7 @@
                 mode = Chosen.HandsItemMode;
             }
 
-            GuiScreensExt::NeedSprite(BackgroundImage).Hidden = (Tla::WeaponModeAim(mode) == HitLocations::LocationNone);
+            GuiScreensExt::NeedSprite(BackgroundImage).Hidden = (WeaponHelpers::WeaponModeAim(mode) == HitLocations::LocationNone);
         }
     }
 }

Gui/Use.fogui

@@ -37,13 +37,13 @@
             if (TargetCritterId != ZERO_IDENT) {
                 Critter? targetCr = Game.GetCritter(TargetCritterId);
                 if (targetCr != null) {
-                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, item.Id, 0, ClientDefines::TARGET_CRITTER, TargetCritterId, Tla::USE_USE, 0});
+                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, item.Id, 0, ClientDefines::TARGET_CRITTER, TargetCritterId, WeaponHelpers::USE_USE, 0});
                 }
             }
             else if (TargetItemId != ZERO_IDENT) {
                 Item? targetItem = Game.GetItem(TargetItemId);
                 if (targetItem != null) {
-                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, item.Id, 0, ClientDefines::TARGET_ITEM, TargetItemId, Tla::USE_USE, 0});
+                    ChosenActions::SetChosenActions({Tla::ChosenUseItem, item.Id, 0, ClientDefines::TARGET_ITEM, TargetItemId, WeaponHelpers::USE_USE, 0});
                 }
             }
             Gui::HideScreen();

Items/advanced_power_armor.foitem

@@ -1,7 +1,6 @@
 [ProtoItem]
 $Name = advanced_power_armor
 
-IsGeck = True
 Weight = 40823
 Volume = 15
 Armor_CrTypeMale = art/critters/hanpwr
@@ -25,6 +24,7 @@ SoundId = 48
 Material = 1
 IsCanPickUp = True
 Type = Armor
+IsGeck = True
 IsFlat = True
 IsNoBlock = True
 IsShootThru = True

Items/advanced_power_armor_mk2.foitem

@@ -1,7 +1,6 @@
 [ProtoItem]
 $Name = advanced_power_armor_mk2
 
-IsGeck = True
 Weight = 45359
 Volume = 15
 Armor_CrTypeMale = art/critters/hanpwr
@@ -25,6 +24,7 @@ SoundId = 48
 Material = 1
 IsCanPickUp = True
 Type = Armor
+IsGeck = True
 IsFlat = True
 IsNoBlock = True
 IsShootThru = True

Items/alien_forward.foitem

@@ -1,9 +1,6 @@
 [ProtoItem]
 $Name = alien_forward
 
-IsCanOpen = True
-Container_Volume = 10
-Container_CannotPickUp = True
 Weight = 4535
 Volume = 1
 GroundLevel = True
@@ -12,6 +9,9 @@ Material = 5
 IsCanUse = True
 Type = Container
 Container_MagicHandsGrnd = True
+IsCanOpen = True
+Container_Volume = 10
+Container_CannotPickUp = True
 IsNoBlock = True
 IsShootThru = True
 IsLightThru = True

Items/alien_side.foitem

@@ -1,9 +1,6 @@
 [ProtoItem]
 $Name = alien_side
 
-IsCanOpen = True
-Container_Volume = 10
-Container_CannotPickUp = True
 Weight = 4535
 Volume = 1
 GroundLevel = True
@@ -12,6 +9,9 @@ Material = 5
 IsCanUse = True
 Type = Container
 Container_MagicHandsGrnd = True
+IsCanOpen = True
+Container_Volume = 10
+Container_CannotPickUp = True
 IsNoBlock = True
 IsShootThru = True
 IsLightThru = True

Items/backpack.foitem

@@ -1,8 +1,6 @@
 [ProtoItem]
 $Name = backpack
 
-IsCanOpen = True
-Container_Volume = 40
 Weight = 2267
 GroundLevel = True
 Cost = 100
@@ -12,6 +10,8 @@ IsCanUse = True
 IsCanPickUp = True
 Type = Container
 Container_MagicHandsGrnd = True
+IsCanOpen = True
+Container_Volume = 40
 IsFlat = True
 IsNoBlock = True
 IsShootThru = True