Repository navigation
Conversation
rahul05ranjan
left a comment
There was a problem hiding this comment.
Solid feature PR — the CompoundAssignmentVariants dispatch table is a clean way to generalize the existing +=-style transform without special-casing six new ops, and I verified the rename from CompoundArithmetic to CompoundAssignment has no stale references left in parser.lua or compiler.lua. Tokenization also checks out: adding to E2Lib.optable_inv is sufficient because E2Lib.optable is built from it, and the per-char longest-match loop in Tokenizer:Operator will pick >>=/&&= over >>/&& correctly.
Verified points:
- The compile path reuses
handleInfixOperationvia the mapped node variant, soX %= E,X >>= Eetc. resolve through exactly the sameGetOperatorlookup as the plain binary ops — behavior follows the existing operators by construction, no new runtime dispatch to get wrong. - The runtime tests encode the right semantics: after
X %= 3leaves 1,X ||= 10asserting 11 is correct because||is bitwise OR for numbers (1|10=11), and likewiseX &&= 8→ 8,X ^^= 12→ 4. Worth saying out loud in the PR description/changelog that these are bitwise compound assignments — anyone coming from C/JS will expect||=/&&=to be logical and get bitten by the naming. - Per-type mod overloads (angle/vector/vector2/vec4/complex/quaternion) are component-wise and consistent with each file's existing operator style (vec4 uses the legacy
registerOperator("mod", ...)matching its other ops; the new-style files usee2function operator%).
Issues:
- CI lint is red, and it's this PR's problem to solve. The lint workflow runs GluaFixer only on changed files, and by touching
quaternion.luayou've surfaced its pre-existing warnings: unusedsinh/coshat the top of the file and the//-style comments it mixes with--. Either fix those in this PR or the gate stays failing and the PR can't merge as-is. - Small typo in
e2lib.lua: the new comment aboveAband = 13reads--&&=`` with a stray extra backtick.
One thing I couldn't verify: the .txt tests under data_static/expression2/tests/ only run as in-game runtime tests (no CI workflow executes them), so I checked the assertions against E2's operator semantics manually rather than executing them.
|
Fixed the typo. I would prefer keeping unrelated changes out of this PR unless the maintainers want me to include them. |
Initially I just wanted to add
%=as I found it odd that Expression 2 doesn't already have it, then I thought why not also add compound bitwise operators for completeness. Also added modular arithmetic operators for more types.To summarize, this PR:
%=,&&=,||=,^^=,>>=, and<<=compound assignment operatorsvector2,vector,vector4,angle,complex, andquaternion:number % vector2vector2 % numbervector2 % vector2number % vectorvector % numbervector % vectornumber % vector4vector4 % numbervector4 % vector4number % angleangle % numberangle % anglecomplex % numberquaternion % numberNodeVariant.CompoundArithmetictoNodeVariant.CompoundAssignment