Skip to content

Add new compound and modulo operators to E2 - #3633

Open
CornerPin wants to merge 3 commits into
wiremod:masterfrom
CornerPin:new-ops
Open

CornerPin wants to merge 3 commits into
wiremod:masterfrom
CornerPin:new-ops

Conversation

@CornerPin

Copy link
Copy Markdown
Contributor

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:

  • adds %=, &&=, ||=, ^^=, >>=, and <<= compound assignment operators
  • adds modular arithmetic operators to vector2, vector, vector4, angle, complex, and quaternion:
    • number % vector2
    • vector2 % number
    • vector2 % vector2
    • number % vector
    • vector % number
    • vector % vector
    • number % vector4
    • vector4 % number
    • vector4 % vector4
    • number % angle
    • angle % number
    • angle % angle
    • complex % number
    • quaternion % number
  • renames NodeVariant.CompoundArithmetic to NodeVariant.CompoundAssignment
  • adds tests for the new operators.

@rahul05ranjan rahul05ranjan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 handleInfixOperation via the mapped node variant, so X %= E, X >>= E etc. resolve through exactly the same GetOperator lookup 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 %= 3 leaves 1, X ||= 10 asserting 11 is correct because || is bitwise OR for numbers (1|10=11), and likewise X &&= 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 use e2function operator%).

Issues:

  1. 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.lua you've surfaced its pre-existing warnings: unused sinh/cosh at 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.
  2. Small typo in e2lib.lua: the new comment above Aband = 13 reads -- &&=`` 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.

@CornerPin

Copy link
Copy Markdown
Contributor Author

Fixed the typo. I would prefer keeping unrelated changes out of this PR unless the maintainers want me to include them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants