feat(utf8): KOI8-R→UTF-8 migration groundwork (#3681) - #3682
Closed
kvirund wants to merge 22 commits into
Closed
Conversation
…3681) First step of the KOI8-R -> UTF-8 migration (track A0). Adds an encoding-agnostic UTF-8 layer that later turns byte semantics into character semantics -- no runtime behaviour change yet; nothing in the engine calls it. - src/utils/utf8.{h,cpp}: decode/encode, is_valid, length, byte_offset, char_at, substr and ASCII+Cyrillic (incl. Yo) case folding. Code-point based, no external deps (no iconv/ICU); ASCII passes through unchanged. - tests/utf8.cpp: 13 GTest cases (empty, incomplete sequences, 4-byte, BOM, overlong, surrogates, range edges, Cyrillic length/substr/case folding). - tools/audit_utf8_migration.py: triage of byte-vs-char sites, categorised and prioritised by files that contain Russian text. New sources are pure ASCII (UTF-8 test data spelled as \xNN escapes), so they satisfy the KOI8-R working-tree attribute unchanged.
19 tasks
#3681) Track A1 of the KOI8-R -> UTF-8 migration. Introduces the build-time `internal_encoding` switch and a native-encoding dispatch layer, then routes the text reformatter through it. No behaviour change on the default KOI8-R build (char_count == byte count, capitalize_first == UPPER of the first byte, truncate_offset == maxlen); the utf8 build gets code-point semantics. - meson_options.txt / meson.build: internal_encoding=koi8r|utf8, the utf8 value defines INTERNAL_ENCODING_UTF8. - src/utils/native_text.{h,cpp}: char_count / capitalize_first / truncate_offset with a byte-identical KOI8-R branch and a utf8:: branch, plus native_is_utf8(). - src/utils/utils.cpp: format_text() width accounting, first-letter capitalisation and maxlen truncation no longer assume 1 byte == 1 character. - tests/native_text.cpp: adaptive GTest suite branching on native_is_utf8().
kvirund
marked this pull request as draft
August 3, 2026 01:07
kvirund
changed the title
…text (#3681) Continues track A1. next_page() now counts one column per character and steps over a multibyte character's trailing bytes; string_add()'s truncation points snap to a character boundary so a cut never splits a character. No-op under KOI8-R (char_bytes == 1, truncate_offset == the byte limit) -- byte-identical behaviour. - native_text: add char_bytes() (byte length of the character starting at a pointer). - src/engine/ui/modify.cpp: next_page() column counting; string_add() max_str/80 truncation offsets. - tests/native_text.cpp: char_bytes cases (ASCII, Cyrillic lead, 4-byte, truncated lead).
Track A2. The case-insensitive comparison family (~600 str_cmp + ~67 strn_cmp call sites) now folds per character instead of per byte -- without touching a single call site. Under KOI8-R the original LOWER() byte loops are left verbatim and a guarded early return sends the UTF-8 build to the code-point fold, so the default build is byte-identical (28 added lines, 0 removed in utils_string.cpp). - native_text: add compare_ci / ncompare_ci (KOI8-R: byte-wise via a_lcc_table, preserving the magnitude callers propagate; UTF-8: code-point fold via utf8::). - native_text no longer includes utils.h -- it declares the two case tables directly, which also keeps the module standalone-testable. - tests: ASCII ordering/prefix/budget cases, Cyrillic case-insensitivity, and a reference test pinning the KOI8-R branch against the original LOWER() byte loop.
Track A2. isname() (~157 call sites) drives a backtracking state machine over bytes:
classification, the case-insensitive match and every advance assumed 1 byte == 1
character. The structure -- including each `curstr = laststr` backtrack -- is kept
verbatim; only those three operations now go through native_text.
Under KOI8-R this is an identity (is_alnum_char == a_isalnum, chars_equal_ci ==
LOWER == LOWER, char_bytes == 1); a differential harness comparing the old and new
implementations over ASCII and Cyrillic inputs reports 0 differences.
Under UTF-8 it fixes the byte logic in both directions, also verified differentially:
* false positive: "shchit" matched the unrelated name list "mech dlinnyi",
because only the shared D0 lead byte was ever compared;
* false negative: "MECH" no longer matched "mech", because the lead bytes fold
equal but the trail bytes do not.
- native_text: add is_alnum_char() and chars_equal_ci().
- tests: cases for both primitives, incl. the Cyrillic case-fold regression.
…3681) Track A2, continued: the command-argument splitters and the Russian name declension no longer assume 1 byte == 1 character. Both are identities under KOI8-R (verified differentially, 0 differences) and fix real breakage under UTF-8. Splitters -- one_argument/any_one_arg/half_chop (mud_string.cpp) and one_word (utils_string.cpp) lowercased byte by byte, which under UTF-8 leaves Cyrillic untouched (the byte table maps neither the lead nor the trail bytes), so Russian command arguments would stop being folded and command lookup would miss. They now fold one whole character per step. The a_isspace()/quote tests stay byte-based on purpose: they only run at a character boundary and every delimiter is ASCII. GetCase (genchar.cpp) picked the declension from name[len - 1] / name[len - 2] and cut the stem with substr(0, len - 1) -- all byte offsets. Under UTF-8 the last-letter test never matched, so names simply stopped declining ("Anya" stayed "Anya" in every case); a differential run over 10 names x 2 genders x 6 cases shows 0/120 differences under KOI8-R and 65/120 corrections under UTF-8. - native_text: add copy_lower_char(), last_char_offset() and list_contains_char() (the strchr-over-a-letter-list replacement), plus a bounded char_bytes_at() so the view-based helpers are safe on a non-null-terminated string_view. - tests: cases for each new primitive, incl. in-place folding and partial-sequence rejection in list_contains_char.
…3681) Adds tests/text_semantics.cpp: behavioural coverage for the routines the migration touched -- isname, str_cmp/strn_cmp, one_argument/half_chop and GetCase. Several of these had no unit tests at all, so this also pays down existing debt rather than only guarding the new code. The Russian literals are written as literals, not byte escapes: the file compiles in whatever encoding the engine is built with, and the routines under test work in that same native encoding, so the expectations hold both before and after the flip. That makes this file the guard that flipping the encoding does not change behaviour. Covered regressions (each one silently broken by byte semantics under UTF-8): * isname matched unrelated Cyrillic words that shared a lead byte, and lost case-insensitive matching for Russian keywords; * argument splitting left Russian arguments unfolded, so command lookup missed; * GetCase stopped declining names entirely. Also renames the fixture in tests/native_text.cpp (kPrivet -> kNtPrivet): the test sources are unity-built, so it collided with the one in tests/utf8.cpp.
The byte ctype tables break under UTF-8 in one specific shape: a multibyte letter
reads as "alnum lead byte + non-alnum trail bytes", so every loop that scans a *run*
of letters stops in the middle of one. Audited all 82 a_is* call sites; only the
alpha/alnum/upper family can misclassify (isspace/isdigit/isxdigit are ASCII-only and
answer correctly for a trail byte), which narrowed the work to five real scanners:
* fname() -- first keyword of a name list
* im.cpp -- crafting alias extraction
* dg_scripts.cpp -- script token boundaries
* cut_one_word() -- word splitting
* do_gen_comm.cpp -- the anti-caps filter, which could not see uppercase Cyrillic
at all (a UTF-8 lead byte is outside the table's range), and
whose percentage denominator counted bytes, not characters
Deliberately left byte-based: IsValidShopId (ids are ASCII identifiers by design) and
the single-character checks in login.cpp / interpreter.cpp, which run at a character
boundary. pred_separator is dead code -- no users outside utils.h -- so it was not
migrated; it should simply be deleted.
Debt paid while here: fname() and the im.cpp alias loop copied into fixed buffers
(30 and 16 bytes) with no bounds check at all -- multibyte text reaches the end twice
as fast, so both now stop short of the buffer. dg_scripts passed a raw (negative) char
to isspace(), which is undefined behaviour.
- native_text: add is_alpha_char() and is_upper_char().
- tests: predicate coverage plus behavioural tests for fname() (incl. the overflow
guard) and cut_one_word(), neither of which had any before.
Verified: full suite 648 passed / 0 failed; live boot on the production world
(world.20260802.tgz) with declension prompts still correct.
Track A3: a printf "%-Ns" field pads by bytes, so a column of Russian text comes out
half as wide once the text is multibyte. Adds pad_right()/pad_left() (width in
characters -- byte-identical under KOI8-R) and converts the most visible player-facing
columns: WHO short list, WHERE, skills and affects.
Also makes the libfort wrapper pick its base class by encoding: char_table measures a
cell in bytes, utf8_table in code points. Neither is right for both -- under KOI8-R the
byte table is correct and utf8_table would misread KOI8-R as UTF-8 -- so the choice now
follows INTERNAL_ENCODING_UTF8 instead of being hardcoded.
Scope finding: fmt::format's "{:<20}" needs NO migration. Measured against the vendored
fmt -- it counts code points for valid UTF-8 and falls back to one-unit-per-byte for
KOI8-R (which is invalid UTF-8), so it is already correct in both encodings. That takes
every fmt-based column, including where_format, out of this track; only printf-style
fields remain.
Deliberately unchanged: colour codes embedded in a padded string still count toward the
width, exactly as with "%-Ns" today. That skew is pre-existing and orthogonal.
…#3681) Continues A3 with the player-facing subsystems: score (the grouping line, which also capped its text with substr(0, 76) -- a byte cut), spellbook, features, exits, PK list, crafting recipes and item creation, glory stats, parcels and named items. Adds native_text::truncate_to_chars() for the "cap a display string" case, so a cap never lands inside a character. Skipped after checking: do_levels pads only digit strings (thousands_sep), so its "%13s" fields are ASCII-only and need nothing. Verified: full suite 649 passed / 0 failed, clean build, and a live boot on the production world with both a UTF-8 and a legacy KOI8-R client rendering correctly.
…#3681) Finishes A3 with the immortal-facing listings: show (rune spells, linkdrop, snoop), last, users, tabulate, liblist, commands and alias, plus the trigger listing in dg_scripts. The substr(0, N) display caps in show and liblist now go through truncate_to_chars so they cannot split a character. do_users shares one format string across three call sites; it becomes plain "%s" fields with each column padded explicitly. Checked and deliberately skipped: * fmt::sprintf (do_stat) -- measured like fmt::format, already correct in both encodings, so the "%-21s" there needs nothing; * "%-s" in exchange -- a left-align flag with no width, so it never pads; * show_fields/set_fields command names and do_levels' thousands_sep columns -- ASCII-only content; * do_toggle's "%-3s" -- the values are all the same width, so nothing shifts. Verified: full suite 649 passed / 0 failed, clean build.
bylins
self-requested a review
August 3, 2026 16:53
bylins
marked this pull request as ready for review
August 3, 2026 16:53
CONTRIBUTING.md claimed C++17 while the build has used C++20 for a while (meson.build: cpp_std=c++20, and -std=c++20 in compile_commands.json). CLAUDE.md already said C++20; this brings the two in line.
kvirund
marked this pull request as draft
August 4, 2026 02:10
…lper (#3681) Replaces all 63 native_text::pad_right/pad_left/truncate_to_chars call sites with fmt, which puts the width back where it belongs -- inside the format string -- instead of wrapping every argument: sprintf(buf, "%s - %s\r\n", native_text::pad_right(GET_NAME(i), 20).c_str(), room); SendMsgToChar(buf, ch); becomes SendMsgToChar(fmt::format("{:<20} - {}\r\n", GET_NAME(i), room), ch); This is possible because fmt measures width AND precision in code points for valid UTF-8 and falls back to one-unit-per-byte for KOI8-R (which is not valid UTF-8) -- measured, not assumed -- so it is already correct in both encodings and needs no change at the flip. printf cannot do this: its width/precision are byte-based by definition, which is what made the helper necessary in the first place. pad_right/pad_left/truncate_to_chars are removed from native_text along with their tests; the character primitives the scanners need (char_bytes, is_alnum_char, ...) stay. Several sites also lose their intermediate char buffer entirely. Drive-by, on a line this change already touches: do_show's mob listing had "{:<31}s" in its format string -- a leftover 's' from an earlier %-31s conversion that printed a stray letter after the name. Removed. Verified: clean build, full suite 648 passed / 0 failed.
…missed (#3681) A full audit sweep showed track A was closed prematurely: beyond the hotspots already converted, 55 more per-byte case sites were left, and they are the same class of bug. * 36 "capitalise the first letter" sites (name[0] = UPPER(name[0]) and friends across 19 files) now go through native_text::capitalize_first, which folds a whole character instead of mangling a multibyte lead byte. * The shared string helpers are fixed at the source rather than per call site: ConvertToLow (both overloads), SubstToLow, SubstStrToLow, SubstStrToUpper, colorLOW, colorCAP and IsAbbr. Fixing ConvertToLow alone covers its many callers. Adds the API the call sites were missing, so they stop doing pointer arithmetic: * native_text::chars(s) -- range-for over characters, each element a string_view covering exactly one character: for (auto letter : native_text::chars(argument)) { ... } * native_text::to_lower/to_upper for std::string and char* -- whole-string case conversion, which collapses several hand-rolled loops to one line. * copy_upper_char, the counterpart of copy_lower_char. Note on naming: character count stays an explicit call (char_count) rather than a "length()", because both lengths are genuinely needed -- characters for display width, bytes for buffer sizing -- and conflating them is how this whole bug class started. Left as-is deliberately: SubstStrToLow raises case despite its name. That mismatch predates this work; only its byte semantics changed. Verified: clean build, full suite 648 passed / 0 failed.
Benchmarking the character-based case conversion showed it was ~17x slower than the
byte loop it replaces under UTF-8. That was not inherent to per-character work; it was
this implementation:
* a std::string was constructed per character just to hold the re-encoded result;
* the fold was reached through a function pointer, so nothing inlined;
* char_bytes() called utf8::sequence_length() across a translation unit, once per
character, in every scan.
Fixed by folding ASCII and the two-byte Cyrillic block directly on the bytes, with the
general decode/encode path kept only as a fallback for characters this codebase never
carries. Whole-string conversion is now one tight loop with no calls in the hot path.
Measured against a like-for-like (also out-of-line) copy of the old ConvertToLow:
KOI8-R: 32 ms -> 33 ms (the production path today: no regression)
UTF-8 : 140 ms -> 211 ms (1.5x, and the 140 ms baseline does not even fold Cyrillic)
The benchmark also caught a real bug this work had introduced: the shared fold loop
treated 0xD0/0xD1 as UTF-8 lead bytes, but under KOI8-R those are the letters "p" and
"r" -- it would have corrupted text on the current build. The loops are now per
encoding. Verified exhaustively: under KOI8-R the result is identical to the raw table
for all 255 byte values; under UTF-8 the folds are correct including Yo/yo.
Also adds utf8::encode(char32_t, char*), an allocation-free encode into a caller buffer.
Menus and OLC editors dispatch on a Cyrillic character literal (231 case labels across
19 files). Those cannot survive the flip: with UTF-8 sources a Cyrillic 'x' is a
multi-character constant, so the compiler folds it to an implementation-defined value
and the menu silently stops responding -- and no single literal spelling works for both
encodings.
Rather than give up the switch (option A, if/else chains) or bundle 231 rewrites into
the flip commit (option B), the letters become numeric constants defined per encoding:
switch (native_text::first_char_code(arg)) {
case 'y': case 'Y': case rus::kDe: case rus::kDeUpper:
* src/utils/russian_keys.h -- all 33 letters, upper and lower, as byte values under
KOI8-R and code points under UTF-8. Both tables were generated and checked against
the actual codecs rather than written from memory: 0 mismatches in 66 entries.
* native_text::first_char_code() -- the raw byte under KOI8-R, the code point under
UTF-8, so the switch expression means the same thing in both.
* ASCII cases stay ordinary literals; they are identical in either encoding.
This lands now as a no-op (under KOI8-R the constant is exactly the byte the old literal
compiled to) and leaves the flip a one-flag change. At cleanup the KOI8-R half of the
table goes away and the constants can collapse into plain U'...' literals.
Includes the first converted switch (medit's save prompt) as a worked example, and a
test pinning every constant against first_char_code -- verified in both branches, since
a drift here would disable a menu key without any visible error.
Remaining: ~229 labels in 18 files, mechanical from here.
…nts (#3681) Completes the A4 sweep: 227 case labels across 15 files plus the remaining single-letter comparisons now dispatch on native_text::first_char_code() against the rus:: constants, so menus and OLC editors keep working after the flip instead of silently going deaf. * 29 switch expressions rewritten: *arg / *argument -> first_char_code(), LOWER(...) -> first_char_code_lower(), UPPER(...) -> first_char_code_upper(). The script refused to touch any switch shape it did not recognise, so nothing was converted blind; do_who's `switch (mode)` was handled through its initialiser. * Comparison sites in named_stuff, modify and login converted the same way. * iosystem's legacy zMUD 'z' -> Cyrillic substitution now writes the letter from a *string* literal: string literals are byte-transparent, so the same code is correct whether the letter is one byte (KOI8-R) or two (UTF-8). A character literal cannot be, which is the whole point of this step. Left untouched on purpose: six occurrences inside comments (prose, not code). One site is deliberately NOT converted -- db.cpp's get_filename(). It transliterates a player name into the save-file name byte by byte, so under UTF-8 it would produce a different filename and every existing player's files would stop being found. That is a data-loss risk, not a mechanical edit: it needs character-wise transliteration plus a test pinning the filename across the flip. Recorded in the issue as C1a. Verified: clean build, full suite 651 passed / 0 failed.
bylins
removed their request for review
August 4, 2026 03:57
bylins
marked this pull request as ready for review
August 4, 2026 03:57
get_filename() derives a player's save-file name by transliterating the character name
byte by byte. Under UTF-8 a Russian letter is two bytes, so the loop would have produced
a different name and every existing player's files -- saves, aliases, depots, all of
which go through this function -- would have stopped being found. This is the one place
in the migration where a mistake costs characters, so it is fixed with the mapping
pinned rather than re-derived.
* The mapping was extracted from the actual tables (AltToLat + the lowercase table)
rather than written from memory, so it reproduces today's output exactly: a->a,
zh->1, ya->q, yo->9, and upper/lower collapse together because the original
lowercased after transliterating.
* native_text::translit_to_filename() implements it in both encodings -- byte-wise
under KOI8-R (unchanged behaviour), code-point-wise under UTF-8.
* A test pins all 33 letters in both cases plus a whole name, and it is the kind of
test that must fail loudly: a silent drift here orphans player files.
Verified in both branches: the full alphabet transliterates to the identical string
("abvgde91zijklmnoprstyfhc74683250q") and "Vasya" in Cyrillic gives "vasq" either way.
Writing the check also caught a bug the normal build cannot see: the UTF-8 branch used
char_bytes_at() before its declaration, so that branch did not compile at all -- it
would only have surfaced at the flip. It now takes the length from utf8::decode().
Full suite 652 passed / 0 failed.
The UTF-8 branch of native_text had no CI coverage at all -- nothing built it, which is
how a branch that did not even compile survived until a hand-written check found it. A
unit test cannot close that gap on its own: it only ever exercises the branch its build
selected.
Adds a Linux job that builds the *coherent* UTF-8 configuration. Sources and runtime
flip together, never separately: git already stores src/ and tests/ as UTF-8, so
dropping working-tree-encoding and checking out again yields UTF-8 sources, and
-Dinternal_encoding=utf8 switches the runtime to character semantics. That is exactly
what C2 will do, so the job is a continuous rehearsal of the flip rather than a synthetic
configuration.
It paid for itself immediately -- two real flip blockers, neither of which any existing
test or the audit script could see:
* utils.cpp sized the Russian month names as char[12][10]. That fits KOI8-R ("Sentyabrya"
is 8 bytes) and overflows in UTF-8 (16). Now an array of pointers, so the cell width
stops depending on the encoding at all.
* where_format indented continuation lines by prefix.size() -- bytes, not columns -- so
the location column drifted apart for names with Cyrillic in them. The test that was
supposed to catch this measured alignment in bytes too, so it agreed with the bug;
both now measure characters.
Verified in both configurations: 652 passed / 0 failed, no warnings.
Incidentally confirms the literal-repertoire rule this migration imposes: a comment
written with guillemets could not be encoded back to KOI8-R, and git refused it.
Adds native_text::from_koi8() -- identity under KOI8-R, a transcode under UTF-8 -- and
routes the YAML loader's GetText() through it. That is the single point all world text
passes, so the loader stops being encoding-specific and the world files can stay KOI8-R
on disk (track B remains deferred).
No-op on the current build; 652 passed / 0 failed.
Booting the flip build on the production world surfaced two things worth recording now
rather than on the day of the flip:
* The server aborted during boot in HelpSystem::SetsHelp -> libfort, on data from an
XML config. So converting the world loader is not sufficient: every KOI8-R source
(cfg/**, help text, boards, mail, saves) needs the same boundary before C2.
* libfort's utf8_table does not degrade on malformed input -- the visible-width
calculation underflows and the allocation check aborts the process. An unconverted
string reaching any table is therefore a crash, not a cosmetic defect, which raises
C3 from "convert what players see" to "convert every source".
Neither is reachable from unit tests; both came out of running the real binary against
the real world, which is what the rehearsal job is for.
Конфликт только в VERSION.txt: master ушёл на 0.1.31, ветка была на 0.1.28. Разрешено в 0.1.32 -- мерж этой ветки сам по себе является фиче-мержем и по политике версий требует инкремента патча поверх master.
kvirund
marked this pull request as draft
August 5, 2026 03:44
kvirund
force-pushed
the
kvirund/utf8-migration-plan-16b188
branch
from
August 5, 2026 04:52
130653b to
b0cf4a8
Compare
This was referenced Aug 5, 2026
Collaborator
Author
|
Объединено в #3709: одна ветка |
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
2 participants
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.
Фундамент миграции KOI8-R → UTF-8 (#3681). Поведение на дефолтной KOI8-R-сборке не меняется — это no-op-рефактор, который «оживёт» только при флипе кодировки (трек C).
A0 — символьный слой + аудит
src/utils/utf8.{h,cpp}— encoding-agnostic хелперы по кодовым точкам, без внешних зависимостей (без iconv/ICU), ASCII насквозь:decode/encode,is_valid(строгая проверка по Unicode Table 3-7),length/byte_offset/char_at/substr, регистр ASCII + кириллица (вкл. Ё/ё).tests/utf8.cpp— 13 GTest-кейсов (пустые, неполные последовательности, 4-байтовые, BOM, overlong, суррогаты, границы диапазонов, кириллица).tools/audit_utf8_migration.py— триаж byte-vs-char мест по категориям с приоритетом файлов с русским текстом.A1 — флаг
internal_encoding+ первый роутингmeson_options.txt/meson.build— опцияinternal_encoding(koi8rпо умолчанию /utf8→-DINTERNAL_ENCODING_UTF8).src/utils/native_text.{h,cpp}— операции в нативной рантайм-кодировке (char_count/capitalize_first/truncate_offset): KOI8-R-ветка байт-в-байт, UTF-8-ветка черезutf8::, плюсnative_is_utf8().src/utils/utils.cpp—format_text()(счёт ширины, заглавная первая буква, обрезка поmaxlen) больше не предполагает «1 байт = 1 символ».tests/native_text.cpp— адаптивный набор (ветвится поnative_is_utf8()), корректен и под KOI8-R, и под UTF-8 сборку.Проверка
UTF-8-ветки прогнаны автономно с
-DINTERNAL_ENCODING_UTF8(18/18 зелёные, чисто под-Wall -Wextra); KOI8-R-ветки (дефолт) проверит CI.utils.cppправился через iconv, кодировка KOI8-R сохранена.Дальше в этот же PR (до первого мержа)
Остаток A1 — пейджер
next_pageиstring_add; затем A2 (регистр/ctype/сравнение). Каждый — отдельным коммитом.VERSION.txt→0.1.28.