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

Bound's bottom right pt isn't updated after calling e.g. add #213

Closed
ii41 opened this issue May 6, 2024 · 0 comments · Fixed by #215
Closed

Bound's bottom right pt isn't updated after calling e.g. add #213

ii41 opened this issue May 6, 2024 · 0 comments · Fixed by #215

Comments

@ii41
Copy link
Contributor

ii41 commented May 6, 2024

const p = Bound.fromGroup(Group.fromArray(
  [
    [0, 0], [1, 1]
  ]
)).add(1).bottomRight

One would expect p to be Pt(2, 2), but it's actually Pt(1, 1).

I suspect that this is because unlike the top left point, the bottom right point is not initialized using the setter, but uses an direct assignment to the member variable, so it's not one of the points that are indexable using numbers (setters make the points indexable from 0 and 1). Methods such as add are inherited from Group, and they rely on integer indexing to get points.

I'm using 0.11.3, but this appears to also exist in master.

pts/src/Pt.ts

Lines 1021 to 1022 in 47de4b7

this.topLeft = a.$min( b );
this._bottomRight = a.$max( b );

For anyone else who is facing this issue and don't want to wait for a fix to land, there's an update method on Bound that updates the point.

ii41 added a commit to ii41/pts that referenced this issue May 7, 2024
…illiamngan#214

2. `Bound` stores top left and bottom right `Pt`s separately. Close williamngan#213

For 2, top left and bottom right `Pt`s were separate from `Pt`s that are
indexable using integers.

This causes that, when a calculation method such as `add` inherited from
`Group` is used on a `Bound`, the `Bound`'s corner `Pts`s are sometimes
not updated, as `Group` uses integer indices for getting the member
`Pt`s.

In order to fix this, the separate members `_topLeft` and `_bottomRight`
are removed from `Bound`. `topLeft` and `bottomRight` are now just
getters for integer-indexable `Pt`s.
ii41 added a commit to ii41/pts that referenced this issue May 7, 2024
1. `Group.toBound`'s return type was wrongly marked `Group`. Closes williamngan#214
2. `Bound` stores top left and bottom right `Pt`s separately. Close williamngan#213

For 2, top left and bottom right `Pt`s were separate from `Pt`s that are
indexable using integers.

This causes that, when a calculation method such as `add` inherited from
`Group` is used on a `Bound`, the `Bound`'s corner `Pts`s are sometimes
not updated, as `Group` uses integer indices for getting the member
`Pt`s.

In order to fix this, the separate members `_topLeft` and `_bottomRight`
are removed from `Bound`. `topLeft` and `bottomRight` are now just
getters for integer-indexable `Pt`s.
williamngan pushed a commit that referenced this issue May 19, 2024
1. `Group.toBound`'s return type was wrongly marked `Group`. Closes #214
2. `Bound` stores top left and bottom right `Pt`s separately. Close #213

For 2, top left and bottom right `Pt`s were separate from `Pt`s that are
indexable using integers.

This causes that, when a calculation method such as `add` inherited from
`Group` is used on a `Bound`, the `Bound`'s corner `Pts`s are sometimes
not updated, as `Group` uses integer indices for getting the member
`Pt`s.

In order to fix this, the separate members `_topLeft` and `_bottomRight`
are removed from `Bound`. `topLeft` and `bottomRight` are now just
getters for integer-indexable `Pt`s.
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 a pull request may close this issue.

1 participant