fix NPE in PolygonShape.set when no Vec2ArrayPool is given #724
Conversation
| if (vertices[i] == null) { | ||
| vertices[i] = Vec2() | ||
| } |
There was a problem hiding this comment.
because it's provably unnecessary
There was a problem hiding this comment.
What makes you think it is unnecessary in all the cases?
| val ps : Array<Vec2?> | ||
| if (vecPool != null) | ||
| ps = vecPool[Settings.maxPolygonVertices] as Array<Vec2?> | ||
| else | ||
| arrayOfNulls<Vec2>(Settings.maxPolygonVertices) | ||
| ps = arrayOfNulls<Vec2>(Settings.maxPolygonVertices) |
There was a problem hiding this comment.
This change shouldn't be required, right?
There was a problem hiding this comment.
Above:
Note that the cast on line 133 is necessary for Kotlin to figure out that the actual fix on line 147 is valid
There was a problem hiding this comment.
Now the declaration and the assignment is split but it looks the same to me and it was compiling before. What happens if you restore the original lines there?
|
Can you also add your failing test case here? Something like: class PolygonShapeTest {
@Test
fun testPolygonShapeTest() {
val shape = PolygonShape()
var zero = Vec2(0f, 0f)
shape.set(arrayOf(zero, zero, zero), 3) // This shouldn't throw
assertEquals(...)
}
} |
|
Fixes korlibs/korge#445 |
|
test added to PR |
| val vertexArray = arrayOf(Vec2(1f, 1f), Vec2(1f, 0f), Vec2(0f, 0f)) | ||
| shape.set(vertexArray, vertexArray.size) | ||
|
|
||
| assertEquals(shape.count, vertexArray.size) |
|
Just checked that vectors: Array is already initialized, so indeed not needed. Going to merge it now. |
Note that the cast on line 133 is necessary for Kotlin to figure out that the actual fix on line 147 is valid