Code review comment for lp://qastaging/~aacid/unity-2d/unity-2d_pointer_barrier

Revision history for this message
Gerry Boland (gerboland) wrote :

Functionally this is perfect, nice!

Code-wise, I can find no problems with the math anywhere. There is a little more going on that I had expected, but I can see no way to avoid everything you've done. So again, nice job!

Just some names I think could be clearer:
- "breakp1" could simply be "p1" and so on.
- "trigger" more understandable with "triggerZone" maybe?
- DecayedValue::add maybe clearer with DecayedValue::addAndCheckExceedingTarget since it returns an important bool.

I also want to see more tests of this barrier. Could you write a couple of unit tests, which run under a fake X server (xvfb) - as all tests in libunity-2d-private/tests do right now. We'd need to verify that the barrier works inside xvfb first however.

You can use XTest to move the mouse around. The source code of xdotool is a nice place to see it being used.

review: Needs Fixing

« Back to merge proposal