Alpha cleanup: strip debug output, dedupe config, document what's broken - #6
Open
Daraan wants to merge 10 commits into
Open
Alpha cleanup: strip debug output, dedupe config, document what's broken#6Daraan wants to merge 10 commits into
Daraan wants to merge 10 commits into
Conversation
Brings in the long-diverged Scripts-in-progress history: removes the legacy DScript.nut / DSEditorScripts.nut monolith files, adds DSConfigFix/DSConfigMyFM "Example" variants, updates DSConfigDefault.nut, DScript SFX/Overlays/File&Blob.nut and DScript_ModdingTools.nut, and adds squirrel_script reference docs under DOC/.
Development traces that reached players: with kUseIngameLog the log tail is drawn on screen, otherwise they spam monolog.txt / Thief2.log. - DCheckString: remove the unconditional prints in the '/' ping-back, '>' file and radius branches (hottest function in the framework). Where a print was the only body of a loop or if, the scaffolding goes with it; the FindFileInPath branches keep their structure and get TODO comments. - DBaseTrap constructor: drop the never-true DTrigger compare and its print. - DBaseFunction: "_script NOT SET" now goes through DPrint(..., kDoPrint, ePrintTo.kMonolog) instead of a bare print. - DScript.SetQVar: comment out the ungated INFO print. The string was built on every QVar write. - DScriptHandler: remove the stray self/MissionInitialized/Object.Exists prints. - DTrapSetQVar: remove the DID BEGIN/DID SIM/InitQVarFromProp traces and the OnBeginScript override that existed only for its print. Its two "Setting x to y" DPrints lose kDoPrint, so they honour <ScriptName>Debug like the rest of the framework - the first one defaulted to mode kMonolog|kUI and wrote an on-screen message in the shipped game. - DScript.Quest: remove the per-subscribe and per-QVar-change traces. - DTrigQVar.OnDarkGameModeChange: print replaced by a comment; the empty handler is kept on purpose so the message is not passed on to OnMessage. No gameplay behaviour changed. Refs T-60, T-61, T-62, T-67.
DScript SFX.nut
- DDirector: remove the seven dev prints (speed, link data, waypoint index,
"Will not start", "Path[0]", ...).
- DUseInventoryMaster.OnContained: fold the message into the DPrint that
already gated it, instead of a raw print behind if (DPrint("")).
DScript File&Blob.nut
- dCSV.createCSVMatrix: remove the per-construction separator print. dump()
keeps its prints, printing is what it is for.
- DPersistentSaveSimple / DPersistentSave: remove the IsOn, map/name, raw
slot data and MissData dumps.
- cDSaveHandler.GetSaveRaw: the two slot-shuffling diagnostics are kept but
gated behind DMissionDebug, following the DMissionFingerPrint convention a
few lines above.
- DPersistentSaveTrap: drop kDoPrint from the "Event Data is" DPrint (mode
defaulted to kMonolog|kUI, so it wrote an on-screen message in game) and
remove the typeof print next to it.
DScript_ModdingTools.nut
- Remove the leftover scratch snippet at the end of the file. It ran at
top level on every compile and printed "yes"/"nope" unconditionally.
- The remaining prints in this editor-only file are the tools' own output
(DDumpModels progress, DumpTable, DImportObj errors, DPerformanceTest
results) and are left alone.
No gameplay behaviour changed. Refs T-63.
- DSConfigDefault.nut: remove the Auto Texture Replacement block (lines 117-227). It was a verbatim duplicate of DSConfigDefAutoTxt.nut, which means enum eDAutoTxtRepl was declared twice in the const table and gDModTable/gDTexTable were built twice on every load. The dedicated file is the one that stays; a pointer comment is left behind. The only content the duplicate had that the dedicated file lacked - the note that FindFileInPath does not check subfolders - is ported over. - DSConfigMyFM.nut: kReplyMessage was declared const here and in DSConfigFix.nut. The Fix layer keeps the declaration, this file now shows the override syntax as a comment, like DSConfigFix Example.nut does. - .gitignore: backup\ / obj\ used backslashes, so neither directory was actually ignored. Now backup/ and obj/. Refs T-02, T-03, T-05.
New: docs/KNOWN_ISSUES.md - the user-facing half of the review findings. An alpha with 141 statically-confirmed defects has to tell mission authors which script classes are known-broken (DHub, DHitScanTrap, persistent save, DTrigQVar, DImUndercover's modes, DRay's second activation, the Copies breakage, the SS2 gaps) so they don't spend a day debugging their Design Note for our bug. README: rewrote for the alpha. It still advertised "v1.0 is coming" and the scripts-in-progress branch. Now: what V2 is, the pre-alpha caveat with a link to KNOWN_ISSUES, the intended shipped file set (including why DT2UndercoverWeapons.nut stays - its own header makes it the opt-in companion of DImUndercover, not legacy), the config-layer load order, the DromEd commands and the Notepad++ language file. Comment corrections, no code touched: - Core.nut:3 named a nonexistent #include (DConfigDefault.nut) and referenced the deleted DScript.nut monolith. - Core.nut DBaseTrap Help2 advertised DBaseTrapBlockMessage=, which does not exist. The real parameter is ExclusiveMessage. - General.nut DoOn carried a pasted pre-API-11 ObjRaycast signature (BOOL bSkipMesh) while the code below depends on the API 11 int flags meaning - RenderedOnly + IgnoreAI are summed into that slot. Noted explicitly so nobody "fixes" the code against the comment. - General.nut DHitScanTrap docstring now documents the ignore_set parameter and why its odd spelling cannot be renamed. - General.nut StackToQVar: the comment claimed Create uses the script object directly, which is exactly what the if below does not do. - File&Blob.nut _typeof: loud note that typeof reports the wrapped stream type on purpose and instanceof is the only reliable test. - SFX.nut DHudCompass: put the docstring's */ and the # banner on separate lines like the rest of the file. - CLAUDE.md: the engine reference lives in DOC/squirrel_script/, not docs/squirrel_script/. Also tracks the alpha cleanup docs themselves (the plan, the two review waves and OPEN_TASKS, which were untracked) and flips the OPEN_TASKS rows this pass closed: T-02, T-03, T-05, T-60, T-61, T-62, T-63, T-86 done; T-04 and T-67 partially, with what is left noted in the row. Refs T-04, T-67, T-86, T-87.
- ALPHA_CLEANUP_PLAN.md: status header - which batches ran, in which commits, every deviation from the plan and why, and what was left for the bug-fix wave (Batch 3 and 5 are untouched and still valid as written). - CLAUDE.md: the encoding note was wrong. Byte histograms say only DScript Core.nut is still Latin-1 (45x §, 37x °); DScript File&Blob.nut and everything else already decode as UTF-8, though grep still needs -a on both. Added the concrete hazard: UTF-8 editors turn Core's high bytes into U+FFFD and break the case '§' label, so that one file needs a byte-safe latin-1 patch script. - CLAUDE.md: refreshed the two config rows the dedupe made stale, and reclassified DT2UndercoverWeapons.nut - not dead legacy but the opt-in companion of DImUndercover, matching the README and T-01. - OPEN_TASKS.md: T-93 re-checked and corrected the same way.
Two small scripts, written for this cleanup pass and worth keeping - there is no other automated check in this repo. - tools/latin1_patch.py - edits DScript Core.nut as latin-1 instead of UTF-8. Every replace asserts its pattern matches exactly once, and save() refuses to write if the high-byte census (45x §, 37x °) changed. The § in DCheckString's case label is load-bearing, and UTF-8-assuming editors silently turn it into U+FFFD. - tools/check_files.py - run after any .nut edit. Reports encoding, line endings, high-byte census and bracket balance against a git ref, and fails on U+FFFD or on a balance that drifted from the baseline. That second check is the cheap way to catch a deleted debug print that was the entire body of a loop or an if, which silently re-parents the next statement. Running it immediately found pre-existing damage in DScript File&Blob.nut: 6 U+FFFD, from whichever earlier pass converted that file to UTF-8. Restored from backup/DScript File&Blob.nut, which is still CP1252 - the fold-marker banner on line 6 (4x §) and the comment about normalising „“ quotes. Comments and a Notepad++ fold marker only, no code involved. CLAUDE.md documents both tools and the hazard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Executes
docs/ALPHA_CLEANUP_PLAN.md— batches 1, 2 and 4. No gameplay behaviour changes and no bug fixes; those stay tracked indocs/OPEN_TASKS.md.Batches 3 (dead code) and 5 (naming/style) were deliberately left out, scoping this pass to what a player or mission author actually sees. Both are still valid as written in the plan.
1. Log spam — the release blocker
~35 development
print()sites removed or gated acrossDScript Core.nut,DScript SFX.nut,DScript File&Blob.nutandDScript_ModdingTools.nut. WithkUseIngameLog = trueplayers saw these on screen; everyone else got a spammedmonolog.txt/Thief2.log. Genuine diagnostics were converted toDPrint(or theDMissionDebuggate the persistence code already uses) rather than deleted.Three sites beyond the plan's list, same defect class:
DTrapSetQVar's twokDoPrintDPrints andDPersistentSaveTrap's one defaulted to modekMonolog|kUI— i.e. an on-screenDarkUI.TextMessagein the shipped game. NowDebug-gated. A leftover top-level scratch snippet at the end ofDScript_ModdingTools.nutprinted"nope"on every compile; removed.Where a print was the entire body of a loop, an
if, or a handler override, the scaffolding went with it — a bareforeachwould otherwise have swallowed the followingreturn. Where the structure carries meaning it was kept and commented:FindFileInPath's branches (T-67's bug half untouched) andDTrigQVar.OnDarkGameModeChange, whose empty handler exists to stop the message reachingOnMessage.2. Packaging and config
DSConfigDefault.nut:117-227deleted — a verbatim duplicate ofDSConfigDefAutoTxt.nut, soenum eDAutoTxtReplwas in the const table twice and the model/texture tables were built twice per load. The only change in this PR that alters what gets compiled —script_reloadright after it.const kReplyMessagenow declared only in the Fix layer;DSConfigMyFM.nutshows the override syntax as a comment, like its Example file..gitignore:backup\/obj\used backslashes, so neither was ignored.3. Docs
docs/KNOWN_ISSUES.md— the user-facing half of the two review waves. An alpha with 141 statically-confirmed defects has to tell authors thatDHub,DHitScanTrap, the persistent-save family,DTrigQVarandDImUndercover's modes do not work, thatDRaydies on its second activation, which scripts break underCopies, and that SS2 is untested.ObjRaycastsignature (BOOL bSkipMesh) that contradicts the code below it, and aHelp2string advertising a parameter that does not exist.4. Tooling
tools/latin1_patch.py—DScript Core.nutis ISO-8859-1 and§is a literalcaselabel inDCheckString. UTF-8-assuming editors silently turn it into U+FFFD; this happened during the pass and was caught and reverted. This script edits as latin-1 and refuses to write if the high-byte census changes.tools/check_files.py— run after any.nutedit. Reports encoding, line endings, high-byte census and bracket balance against a git ref.Running the checker immediately found pre-existing damage: 6 U+FFFD in
DScript File&Blob.nutfrom an earlier UTF-8 conversion. Restored from the still-CP1252backup/copy — a fold-marker banner and one comment, no code.Verification
Nothing here has been run — nothing in this repo can be. What was checked automatically: every
.nutfile's bracket balance is byte-identical to the pre-PR baseline, encodings and line endings are unchanged, and no file contains U+FFFD.Still needed in DromEd:
Acceptance for the log work: start a test mission with several DScript objects, play two minutes, confirm
monolog.txtis empty unless a Design Note setsDebug=1.Follow-ups left open
>operator still opens an unvalidated path; only the prints wentOPEN_TASKS.mdDConfig*vsDSConfig*naming scheme; only the comment was correctedOPEN_TASKS.mdKNOWN_ISSUES.mdin sync as Group B/E bugs get fixedOPEN_TASKS.mdT-87OPEN_TASKS.md,docs/review/🤖 Generated with Claude Code