Fixed #36 - Allow assignment to Actor's width and height to scale the Actor #92
Conversation
…scale the Actor
pgzero/actor.py
Outdated
|
|
||
| @scale_x.setter | ||
| def scale_x(self, x): | ||
| self._adjust_scale_and_angle(x, 1.0) |
There was a problem hiding this comment.
I would not expect setting scale_x to reset scale_y to 1.0.
actor.scale_x = 0.5
actor.scale_y = 0.5should be equivalent to
actor.scale = 0.5
pgzero/actor.py
Outdated
| self._adjust_scale_and_angle(1.0, y) | ||
|
|
||
| @property | ||
| def size(self): |
There was a problem hiding this comment.
I think we're taking out size, right?
There was a problem hiding this comment.
@a96tudor just stumbled across this PR and looks like this is the only requested change that hasn't been addressed?
| return self._scale_x, self._scale_y | ||
|
|
||
| @scale.setter | ||
| def scale(self, scale): |
There was a problem hiding this comment.
I'd quite like scale to accept a single int/float, as a shortcut for setting both values.
Actually I would expect that is the more common case.
| new_width = int(self._orig_surf.get_size()[0] * abs(x)) | ||
| new_height = int(self._orig_surf.get_size()[1] * abs(y)) | ||
|
|
||
| self._surf = pygame.transform.scale(self._orig_surf, (new_width, new_height)) |
There was a problem hiding this comment.
We should special-case the no-op case for performance for each of these - a check on whether scale_ == scale_y == 1 is much cheaper than doing the transform, which will copy a surface. Same with rotation and flip.
test/test_actor.py
Outdated
| self.assertEqual((actor.width, actor.height), (originial_size[1]/2, originial_size[0]/2)) | ||
| self.assertEqual(actor.topleft, (-13, 13)) | ||
|
|
||
| # Test rasing exception for invalid scale parameters |
test/test_actor.py
Outdated
|
|
||
| # Scaling on x-axis only | ||
| actor.scale_x = 2 | ||
| self.assertEqual(actor.size, (originial_size[0] * 2, originial_size[1])) |
There was a problem hiding this comment.
These are great looking tests!
| orig = images.load('alien') | ||
| exp = pygame.transform.flip(orig, True, False) | ||
| actor.scale = (-1, 1) | ||
| self.assertImagesEqual(exp, actor._surf) |
There was a problem hiding this comment.
A tip is "one assert per test". This means that if a test fails, all of the other tests still run, which gives more information, when refactoring, about what you might have broken.
There was a problem hiding this comment.
Hey!
Here is my first attempt at solving this issue. Looking forward for you feeedback!
Thanks,
Tudor.