{"action":"created","issue":{"url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/42","labels_url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/42/labels{/name}","comments_url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/42/comments","events_url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/42/events","html_url":"https://github.com/mini2Dx/mini2Dx/pull/42","id":113346031,"number":42,"title":"Added interpolators","user":{"login":"wleroux","id":7277279,"avatar_url":"https://avatars.githubusercontent.com/u/7277279?v=3","gravatar_id":"","url":"https://api.github.com/users/wleroux","html_url":"https://github.com/wleroux","followers_url":"https://api.github.com/users/wleroux/followers","following_url":"https://api.github.com/users/wleroux/following{/other_user}","gists_url":"https://api.github.com/users/wleroux/gists{/gist_id}","starred_url":"https://api.github.com/users/wleroux/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wleroux/subscriptions","organizations_url":"https://api.github.com/users/wleroux/orgs","repos_url":"https://api.github.com/users/wleroux/repos","events_url":"https://api.github.com/users/wleroux/events{/privacy}","received_events_url":"https://api.github.com/users/wleroux/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-10-26T12:36:19Z","updated_at":"2015-10-26T18:07:10Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mini2Dx/mini2Dx/pulls/42","html_url":"https://github.com/mini2Dx/mini2Dx/pull/42","diff_url":"https://github.com/mini2Dx/mini2Dx/pull/42.diff","patch_url":"https://github.com/mini2Dx/mini2Dx/pull/42.patch"},"body":"Hello,\r\n\r\nI noticed that Mini2Dx has a great emphasis on the interpolation step, however aside from the basic interpolation provided by Mini2Dx, it's a bit left to the developer's imagination. Additionally, the 'previous value' changes when you lerp it on the current implementation (Ow!) For instance, the following fails:\r\n\r\n```\r\n    @Test\r\n    public void testLerping() {\r\n        CollisionBox box = new CollisionBox();\r\n        box.set(1,2,3,4);\r\n        box.rotateAround(new Point(22, 55), 22);\r\n\r\n        box.preUpdate();;\r\n        box.set(5,6,7,8);\r\n        box.rotateAround(new Point(55, 22), -22);\r\n\r\n        box.interpolate(null, 0.55f);\r\n        float renderX = box.getRenderX();\r\n        float renderY = box.getRenderY();\r\n\r\n        box.interpolate(null, 0.55f);\r\n        Assert.assertEquals(renderX, box.getRenderX());\r\n        Assert.assertEquals(renderY, box.getRenderY());\r\n    }\r\n```\r\n\r\nI understand the 'mucking' with the values is to reduce the object allocation, so I built an interpolation system that is both easy to implement new types, doesn't muck with your values, and supports the flyweight approach.\r\n\r\nHere's a sample of usage with primitives (i.e.: no flyweights):\r\n\r\n```\r\n  @Test\r\n  public void testInterpolatedValues() {\r\n    InterpolatedFloatValue value = new InterpolatedFloatValue(Interpolators.linearFloat());\r\n    value.setInitialValue(initialValue);\r\n    value.setTargetValue(targetValue);\r\n    for (float alpha = 0.0f; alpha <= 1.0f; alpha += 0.05f) {\r\n      value.setAlpha(alpha);\r\n      Assert.assertEquals(initialValue + alpha * (targetValue - initialValue), value.getInterpolatedValue(), 0.0f);\r\n    }\r\n  }\r\n```\r\n\r\nHere's a sample of usage with non-primitives:\r\n\r\n```\r\n  @Test\r\n  public void testInterpolateValues() {\r\n    interpolatedValue.setInitialValue(initialValue);\r\n    interpolatedValue.setTargetValue(targetValue);\r\n\r\n    Circle flyweight = new Circle(0f, 0f, 0f);\r\n    for (float alpha = 0.0f; alpha <= 1.0f; alpha += 0.05f) {\r\n      interpolatedValue.setAlpha(alpha);\r\n\r\n      float x = initialValue.getX() + alpha * (targetValue.getX() - initialValue.getX());\r\n      float y = initialValue.getY() + alpha * (targetValue.getY() - initialValue.getY());\r\n      float radius = initialValue.getRadius() + alpha * (targetValue.getRadius() - initialValue.getRadius());\r\n\r\n      interpolatedValue.getInterpolatedValue(flyweight);\r\n      Assert.assertEquals(\"x at \" + alpha, x, flyweight.getX(), 0.05f);\r\n      Assert.assertEquals(\"y at \"+ alpha, y, flyweight.getY(), 0.05f);\r\n      Assert.assertEquals(\"radius at \"+ alpha, radius, flyweight.getRadius(), 0.05f);\r\n    }\r\n  }\r\n```\r\n\r\nCurrently, the InterpolatedValue class has a 'initial value' and 'target value' which may be a bit strange. Maybe 'initial value' and 'current value' makes more sense?\r\n\r\nIn any case, I haven't dabbled in the serialization aspects of it (CollisionCircle doesn't seem to have serialization-specific details.) Maybe implements Serializable is enough?\r\n\r\nA side note, I couldn't replicate the CollisionBox linear interpolation. The Rectangle class under the hood does some wicked things with the rotation. As soon as I add in linear interpolation of a moving rotational center, the x-y coordinates get 'rotated' and I lose my 'initial values'. For the time being, I left it out. CollisionBox uses some private methods to set up the interpolated value.\r\n\r\nThanks! I appreciate your thoughts on the matter.\r\n"},"comment":{"url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/comments/151230280","html_url":"https://github.com/mini2Dx/mini2Dx/pull/42#issuecomment-151230280","issue_url":"https://api.github.com/repos/mini2Dx/mini2Dx/issues/42","id":151230280,"user":{"login":"wleroux","id":7277279,"avatar_url":"https://avatars.githubusercontent.com/u/7277279?v=3","gravatar_id":"","url":"https://api.github.com/users/wleroux","html_url":"https://github.com/wleroux","followers_url":"https://api.github.com/users/wleroux/followers","following_url":"https://api.github.com/users/wleroux/following{/other_user}","gists_url":"https://api.github.com/users/wleroux/gists{/gist_id}","starred_url":"https://api.github.com/users/wleroux/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wleroux/subscriptions","organizations_url":"https://api.github.com/users/wleroux/orgs","repos_url":"https://api.github.com/users/wleroux/repos","events_url":"https://api.github.com/users/wleroux/events{/privacy}","received_events_url":"https://api.github.com/users/wleroux/received_events","type":"User","site_admin":false},"created_at":"2015-10-26T18:07:10Z","updated_at":"2015-10-26T18:07:10Z","body":"Glad you liked the idea! I agree on all counts.\r\n\r\nIs the Rectangle class tested for performance or implementation correctness only? I noticed there were no tests for rotation when I was looking at the tests for it. I'm considering having a stab at it.\r\n\r\nIn regards for `Interpolator#interpolate`, there are benefits to having the flyweight be returned by the function, however I believe they do not outweigh the costs. I'll list them here in case someone looks up why we did it this way:\r\n\r\nThe benefits to having the return value as both an argument and return value:\r\n\r\n1. It is a 'more' consistent interface as primitives (primitives always return the values)\r\n2. While the flyweight is an argument, it is not necessarily required. (i.e.: the function doesn't use the flyweight and instantiates a new object as the return value.)\r\n\r\nThe drawbacks are:\r\n\r\n1. The user of the API is never sure to use the flyweight or the returned value\r\n2. The flyweight and the returned value could be different depending on the implementation\r\n3. By using return values, it increases the likelihood of object allocation (which should definitely be avoided in the interpolation step.)\r\n\r\nBy far, having a consistent and easily understood API that mitigates a tendency to allocate objects is a better choice for the framework... Despite primitives having a slightly different pattern.\r\n"}}