Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIXI.Polygon#reverseOrientation documentation is misleading. #8992

Closed
1 of 6 tasks
dev7355608 opened this issue Mar 4, 2023 · 2 comments
Closed
1 of 6 tasks

PIXI.Polygon#reverseOrientation documentation is misleading. #8992

dev7355608 opened this issue Mar 4, 2023 · 2 comments
Assignees
Labels
bug Functionality which is not working as intended documentation Improvements or additions to documentation

Comments

@dev7355608
Copy link

What happened?

Documentation claims that the points are mutated in-place:

Reverse the order of the polygon points in-place mutating the current polygon.

What ways of accessing Foundry can you encounter this issue in?

  • Native App (Electron)
  • Chrome
  • Firefox
  • Safari
  • Other

Reproduction Steps

poly = new PIXI.Polygon(0, 0, 0, 100, 100, 0);
p1 = poly.points;
poly.reverseOrientation();
p2 = poly.points;
console.assert(p1 === p2); // fails

What core version are you reporting this for?

Version 11 Prototype 2 (build 293)

Relevant log output

No response

Bug Checklist

  • The issue occurs while all Modules are disabled
@dev7355608 dev7355608 added the bug Functionality which is not working as intended label Mar 4, 2023
@Feu-Secret Feu-Secret self-assigned this Mar 5, 2023
@Feu-Secret Feu-Secret added the documentation Improvements or additions to documentation label Mar 5, 2023
@aaclayton
Copy link
Contributor

Is this a documentation bug and we want the transformation to return a new polygon? or do we want the mutation to occur in-place? I think it's probably a documentation bug and we are okay with the points being a new array?

@Feu-Secret Feu-Secret changed the title PIXI.Polygon#reverseOrientation doesn't mutate the points in-place PIXI.Polygon#reverseOrientation documentation is not accurate. Mar 10, 2023
@Feu-Secret Feu-Secret changed the title PIXI.Polygon#reverseOrientation documentation is not accurate. PIXI.Polygon#reverseOrientation documentation is misleading. Mar 10, 2023
@dev7355608
Copy link
Author

Looking at the title of this issue, I assume just the documentation has been changed and the "in-place" part has been removed. But I recommend to reverse in-place: that's much more efficient. A new point array is almost a new polygon. I'd expect reverseOrientation to reverse in-place if the documentation doesn't mention anything in this regard. Assume you want a copy of the polygon in the the reversed orientation: you'd do polygon.clone().reverseOrientation() (#8991). This creates a clone the points and then creates another point array with reversed orientation from cloned array. It isn't efficient at all. I don't see any benefits from reverseOrientation creating a new array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality which is not working as intended documentation Improvements or additions to documentation
Projects
Status: Done
Development

No branches or pull requests

3 participants