-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add rotatability to sprites. #214
Changes from all commits
4d8e3f8
f0a8566
c8e246f
71adae2
f29ed82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,7 +140,34 @@ def _attribute_gate(self, attribute, bad_sides): | |
raise AttributeError(message) | ||
|
||
|
||
class BaseSprite(EventMixin): | ||
class Rotatable: | ||
""" | ||
A simple rotation mixin. Can be included with sprites. | ||
""" | ||
_rotation = 0 | ||
# This is necessary to make facing do the thing while also being adjustable. | ||
basis = Vector(0, -1) | ||
# Considered making basis private, the only reason to do so is to | ||
# discourage people from relying on it as data. | ||
|
||
@property | ||
def facing(self): | ||
return Vector(*self.basis).rotate(self.rotation).normalize() | ||
|
||
@property | ||
def rotation(self): | ||
return self._rotation | ||
|
||
@rotation.setter | ||
def rotation(self, value): | ||
self._rotation = value % 360 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can still be negative; should this use the same convention as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the case of this interface, dunno if it matters. The tests have some negative rotation and it gives the equivalent positive rotation, which is a reasonable result, but if both you and @astronouth7303 feel like I should port that over here, I will. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pathunstrom I don't think it's much of an issue either way, I just wanted to check that was intentional. |
||
|
||
def rotate(self, degrees): | ||
"""Rotate the sprite by a given angle (in degrees).""" | ||
self.rotation += degrees | ||
|
||
|
||
class BaseSprite(EventMixin, Rotatable): | ||
""" | ||
The base Sprite class. All sprites should inherit from this (directly or | ||
indirectly). | ||
|
@@ -155,7 +182,6 @@ class BaseSprite(EventMixin): | |
image = None | ||
resource_path = None | ||
position: Vector = Vector(0, 0) | ||
facing: Vector = Vector(0, -1) | ||
size: Union[int, float] = 1 | ||
|
||
def __init__(self, **kwargs): | ||
|
@@ -164,15 +190,14 @@ def __init__(self, **kwargs): | |
# Make these instance properties with fresh instances | ||
# Don't use Vector.convert() because we need copying | ||
self.position = Vector(*self.position) | ||
self.facing = Vector(*self.facing) | ||
|
||
# Initialize things | ||
for k, v in kwargs.items(): | ||
# Abbreviations | ||
if k == 'pos': | ||
k = 'position' | ||
# Castings | ||
if k in ('position', 'facing'): | ||
if k == 'position': | ||
v = Vector(*v) # Vector.convert() when that ships. | ||
setattr(self, k, v) | ||
|
||
|
@@ -226,9 +251,6 @@ def bottom(self, value): | |
def _offset_value(self): | ||
return self.size / 2 | ||
|
||
def rotate(self, degrees: Number): | ||
self.facing.rotate(degrees) | ||
|
||
def __image__(self): | ||
if self.image is None: | ||
self.image = f"{type(self).__name__.lower()}.png" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rotatable
sounds weird to me, but I'm a silly non-native speaker. shrugsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Native speaker, sounds weird to me, too, but lots of mixins are named based on their capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-able is approximately the right convention, but it leads to lots of weird words.
I would also accept CanBe*? Also weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So like
CanBeRotated
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just
CanRotate
? In any case, I don't thinkRotatable
is a big issue, I was just very confused whether there was a typo, or it was just weird, or I was just bad at English.