Skip to content

fix: validate values passed to isPowerOfTwo() - #1814

Closed
mmporong wants to merge 1 commit into
melonjs:masterfrom
mmporong:fix/power-of-two-validation
Closed

mmporong wants to merge 1 commit into
melonjs:masterfrom
mmporong:fix/power-of-two-validation

Conversation

@mmporong

Copy link
Copy Markdown
Contributor

Description

isPowerOfTwo() relied on JavaScript bitwise coercion without validating its input. This made zero, negative powers, fractional values, and some non-powers beyond the 32-bit range return true.

This change requires a positive integer, computes its base-2 exponent, and verifies that reconstructing the power produces the original value. Regression tests cover zero, a negative value, a fraction, values beyond the 32-bit range, and large non-powers whose logarithms round to integers.

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Performance improvement
  • Refactoring (no functional changes)

Checklist

  • I have read the Contributing Guide
  • My code follows the existing code style (pnpm lint passes)
  • I have tested my changes locally (pnpm test passes)
  • I have added tests that cover my changes (if applicable)
  • The build succeeds (pnpm build)

The focused Chromium suite passes all 72 math tests. The complete browser suite was not run locally.

Related issues

None.

obiot added a commit that referenced this pull request Nov 1, 2026
…idation (#1821)

Three fixes by @mmporong, from #1812, #1813 and #1814:

- Line#contains() checks the segment endpoints as well as collinearity; a point on the line beyond either endpoint was reported as contained
- Color#parseHex() keeps the exact alpha byte of #RRGGBBAA / #RGBA colours instead of rounding it to one decimal (#0000FF01 was fully transparent); toHex8() output is unchanged, and every hex alpha round-trips
- isPowerOfTwo() rejects zero, negative and fractional values, and is correct beyond the 32-bit range

Co-authored-by: mmporong <mmporong@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JnzwA4xBNBw7cEZzG1bUru

obiot commented Nov 1, 2026

Copy link
Copy Markdown
Member

Thank you, @mmporong! This fix is now on master via #1821, which brought in your three PRs together. Your commit went in as you wrote it, and you're credited as co-author on the merge commit and in the changelog.

I'm closing this one since #1821 merged it. Thanks again for the fix and the edge-case tests, including the large values whose logarithms round to integers.


Generated by Claude Code

@obiot obiot closed this Nov 1, 2026
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