love icon indicating copy to clipboard operation
love copied to clipboard

BezierCurve:insertControlPoint() cannot insert at the last index

Open slime73 opened this issue 5 years ago • 2 comments

Original report by Krunklehorn (Bitbucket: Krunklehorn, GitHub: Krunklehorn).


From the wiki...

BezierCurve:insertControlPoint
Insert control point as the new i-th control point. Existing control points from i onwards are pushed back by 1. Indices start with 1. Negative indices wrap around: -1 is the last control point, -2 the one before the last, etc.

If I'm reading this correctly, the following code...

-- a simple curve with four points in a counter-clockwise motion
-- default index is -1 (last), but was included for clarity
curve:insertControlPoint(0, 0, -1)	-- bottom left
curve:insertControlPoint(100, 0, -1)	-- bottom right
curve:insertControlPoint(100, -100, -1)	-- top right
curve:insertControlPoint(0, -100, -1)	-- top left

print(curve:getControlPoint(1))
print(curve:getControlPoint(2))
print(curve:getControlPoint(3))
print(curve:getControlPoint(4))

...should yield the following result.

0	0
100	0
100	100
0	100

Since each point is supposed to be inserted at the last index, we expect the points to be in order as shown.
Here's what happens instead...

100	0	-- (2nd)
100	100	-- (3rd)
0	100	-- (4th)
0	0	-- (1st)

Attempting to insert a point at the last index (-1) actually inserts it at the second last index.

Okay, so we think to try inserting at index 0 maybe?
Unfortunately that doesn't work....

0	100	-- (4th)
100	100	-- (3rd)
100	0	-- (2nd)
0	0	-- (1st)

This gets treated as if you were inserting at index 1, which keeps your points in line but reverses their order since you're constantly pushing the whole list forward.

Additionally: the wiki's description, "points from i onwards are pushed back by 1" should actually say that the points are pushed forward, at least that's the intended result, right?

slime73 avatar Jun 14 '19 04:06 slime73

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).


Since each point is supposed to be inserted at the last index, we expect the points to be in order as shown.

Well.. the docs suggest otherwise, though I do agree it’s not the behaviour you’d want. What happens is:

  • Insert a at -1 into an empty list: the list becomes your first element a
  • Insert b at -1 aka 1 into [a], this means b will be at the new index 1, and everything from 1 on is pushed back, result is [b, a]
  • Insert c at -1 aka 2, c will be at 2, 2 and on will be pushed back, result is [b, c, a]
  • Insert d at -1 aka 3, d will be at 3, 3 and on will be pushed back, result is [b, c, d, a]

Which is exactly what you found. You can specify a past-the-end index to actually make the new element be the last element, rather than the second-to-last element. The following code does the make the sequence you’d expect:

	curve:insertControlPoint(0, 0, 1)
	curve:insertControlPoint(100, 0, 2)
	curve:insertControlPoint(100, -100, 3)
	curve:insertControlPoint(0, -100, 4)

Additionally: the wiki's description, "points from i onwards are pushed back by 1" should actually say that the points are pushed forward, at least that's the intended result, right?

I’d say they do get pushed back, if we consider index 1 the front. I guess strictly speaking they’re pushed towards the end.

I think the reasonable behaviour for insertControlPoint is to insert after the specified element, not before, if the index is negative, but maybe someone else has input on that?

slime73 avatar Jun 21 '19 06:06 slime73

Original comment by Krunklehorn (Bitbucket: Krunklehorn, GitHub: Krunklehorn).


Ahh, I understand now. I misunderstood thinking the default behavior of -1 would operate the same as the default for lua’s table.insert(), essentially a push operation. Thus I think the wraparound mechanic is fine, but the default behavior is all that needs changing. Unless there is some other reason for this, I think it should default to the number of control points plus one, just as table.insert() does.

slime73 avatar Jun 22 '19 02:06 slime73