Uh oh!
There was an error while loading. Please reload this page.
Hold an interpreted string as bytes, the way the game does - #1238
Conversation
Warcraft III counts and indexes a string in bytes, so a slice may stop between the bytes of one character, and Lua agrees because its strings are byte arrays. The interpreter held a Java string and counted UTF-16 code units, which is the same answer only for ascii. The standard library depends on the difference rather than avoiding it: it cuts a character in half on purpose to find out how the engine represents a partial slice, and slices a 64 character literal byte by byte to enumerate every continuation byte. Under UTF-16 that detection concludes the engine has no multibyte characters, so anything computed at compiletime is built from lengths the game will not agree with, and nothing reports it. The value is now held one char per byte, so Java's own length and substring give the game's answers. Text is encoded coming in and decoded going back out to a file or a screen; a half character has no text to decode to, which is the point. Two things follow from the representation. StringCase folds only ascii letters, because the bytes of a multibyte character are not letters and folding them rewrites the character. StringHash is computed over the bytes: the library's takes text and encodes it as UTF-8 itself, so it cannot hash half a character, and decoding first would collapse every partial slice onto one value and take the continuation bytes with it. The Lua test runtime never implemented StringLength or SubString, so the generated fallback raised an error and the Lua half of such a test could not have been passing for the reason it appeared to. Both are in the shim now.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:caebb0ecd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return JassIm.ImRealVal("" + ((ILconstReal) value).getVal()); | ||
| } else if (value instanceof ILconstString) { | ||
| return JassIm.ImStringVal(((ILconstString) value).getVal()); | ||
| return JassIm.ImStringVal(((ILconstString) value).text()); |
There was a problem hiding this comment.
When a compiletime expression returns a deliberately partial UTF-8 slice, text() decodes the invalid byte sequence to U+FFFD before embedding it in the transformed IM. For example, compiletime(SubString("ä", 0, 1)) becomes a three-byte replacement character, so a subsequent StringLength returns 3 instead of 1; the new tests consume slices entirely inside the interpreter and do not exercise this compiletime-to-runtime boundary. Materialize the byte view without lossy decoding and add a regression that uses the returned value after transforms.
AGENTS.md reference: AGENTS.md:L62-L64
Useful? React with 👍 / 👎.
| // from an empty string / zero value. | ||
| String val = rs.getString(index.getVal() + 1); | ||
| return new ILconstString(val == null ? "" : val); | ||
| return ILconstString.fromText(val == null ? "" : val); |
There was a problem hiding this comment.
When SQLite paths, queries, or bound values contain non-ASCII text, this return conversion is not paired with conversion at the outbound JDBC boundary: sqlite_open, sqlite_prepare, sqlite_bind_string, and sqlite_exec still pass getVal(), which now exposes the ISO-8859-1 byte view rather than host text. Consequently, binding "ä" stores "ä", and reading it here applies another UTF-8 encoding so the value no longer equals the original; non-ASCII database paths and SQL literals are similarly mangled. Use text() for the outbound SQLite calls and cover a non-ASCII bind/read round trip.
AGENTS.md reference: AGENTS.md:L62-L64
Useful? React with 👍 / 👎.
… literal Every value handed to the driver is text, so the four outbound calls decode the way the inbound read already did. Binding "ä" without it stored the bytes as though each were a character, and it came back a different string than the one that went in. A compiletime expression's result becomes a literal in the generated script, which is written as UTF-8, and neither Jass nor the escaping here can write a byte down numerically. Half a character therefore went in as the replacement character and came back three bytes long where the interpreter counted one. Refused with a message pointing at the fix instead of carried across at a different length. Whole characters cross unchanged, which is what the standard library does at compiletime.
Frotty
commented
Aug 16, 2026
Both addressed. Decode interpreter strings before passing them to SQLite — my mistake, and exactly as described: I converted the inbound read and left the outbound side on
Preserve partial bytes when materializing compiletime strings — the diagnosis is right and the test gap was real, but lossless materialisation is not reachable from here. The value has to become an So it refuses rather than mangles: Two tests at that boundary, since it was untested either way:
Flagging the judgement call rather than burying it: refusing is not what was asked for, and if the larger byte-through-the-backend change is wanted, this is one line to swap out and the test documents precisely what would change. Targeted tests green; full suite still running and I will report if it turns. @codex review |
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Uh oh!
There was an error while loading. Please reload this page.
Warcraft III counts and indexes a string in bytes:
StringLengthreturns a byte count andSubStringtakes byte offsets, so a slice may stop between the bytes of one character. Lua agrees, its strings being byte arrays. The interpreter held a Java string and counted UTF-16 code units, which gives the same answer only for ascii —StringProviderwasstring.getVal().length()andstr.substring(s, e).Why it matters now
The standard library depends on the difference rather than avoiding it.
String.wurstsetsENABLE_MULTIBYTE_SUPPORT = trueand:"ä".substring(0, 1)to find out how the engine represents half a character,0x80–0xBF, keeping them apart by hash,Under UTF-16 the first of those returns the whole character, so
PARTIAL_CHAR_DETECTABLEgoes false and the detection concludes the engine has no multibyte characters. That is the graceful degradation its author intended for a future engine change, not a compiletime semantics we should ship:ChunkedStringand object editor text are built from lengths the game will not agree with, and nothing reports it.The representation
The value is held one char per byte, so every char is below 256 and Java's own
lengthandsubstringalready give the game's answers. Text is encoded coming in (fromText) and decoded going back out to a file, a program literal, or a screen (text()). A half character has no text to decode to, which is the point — it keeps its byte until the other half is added back.The constructor is private, so every one of the 21 construction sites had to be classified as text or as bytes rather than left to inspection.
Two consequences of the representation:
StringCasefolds only ascii letters. The bytes of a multibyte character are not letters; folding one the way a latin-1 char folds rewrites the character into a different one.StringHashis computed over the bytes. The library'sStringHash.hashtakes text and doesgetBytes("UTF-8")itself, so it cannot hash half a character, and itsbyte[]overload is private. Decoding first is not a way out either: every partial slice would decode to the same replacement character and collapse onto one hash, taking the 64 continuation bytes with it — exactly the thing the standard library tells apart.Wc3StringHashimplements the same function (Bob Jenkins' lookup2) over bytes.Test runtime
The Lua runtime never implemented
StringLengthorSubString. The generated fallback for an undefined native raises an error, so the Lua half of any test using them could not have been passing for the reason it appeared to. Both are inwc3shim.luanow, as the plain Lua byte operations the game performs.Tests
StringByteSemanticsTests— length of a two byte character, a slice cutting a character in half, and the halves rejoining into the original. Each runs on the interpreter and on Lua, so the two are pinned against each other rather than against an assumption.Wc3StringHashTest— the byte hash against the library's across ascii of every length up to 40, strings needing case and slash normalisation, and whole multibyte text, where both are defined. Then the two properties the library cannot express: the halves of a character hash apart, and all 64 continuation bytes stay distinct.Full suite green.
Not covered
The engine collapses every half-character slice to one marker string with a constant hash. This does not emulate that quirk: byte-accurate, a
0xD0lead byte hashes as itself rather than matchingPARTIAL_CHAR_HASH. The standard library still gets right answers, by a different route —isCharBoundaryfalls through to the continuation byte table, does not find a lead byte there, and reports a boundary — butPARTIAL_CHAR_DETECTABLEis true at compiletime while matching a narrower set of slices than in game. Reproducing the collapse is a deliberate further step and wants its own test.