On Tue, Jul 6, 2010 at 9:31 AM, claudio canepa <[email protected]> wrote:
>
> I want to discuss to changes in actions code.
> I specifically want Richard's feedback, because is related to code checked
> in by him, but if someone else has an opinion please chime in.
>
> 1) I want to remove actions.instant_actions.DoAction
> Rationale:
> I think it was introduced to deal with limitations in the old (0.3.0)
> sequence operator, like the DoAction docstring tells:
>
> "...Useful if you want to sequence actions of infinite duration."
I'll admit I actually don't understand what DoAction's use-case is. I
fixed the docstring as best I could because it was broken, as far as I
could tell :-)
> The cocos 0.4 sequence operator was upgraded and now directly supports
> sequencing when one of the actions can have potential infinite duration.
>
> Thus, the example in the docstring:
>
> action = Repeat( dance )
> sprite.do( go_home + DoAction( action ) )
>
> can be written
> action = Repeat( dance )
> sprite.do( go_home + Repeat( dance ) )
>
> If you want an example with stock cocos,
>
> rotate = RotateBy(360, 1 )
> sprite.do( MoveTo(home, 2) + Repeat(rotate))
>
> Thus, DoAction duplicates existing functionality.
> And I think it is more clear 'go_home + Repeat( dance)' than 'go_home +
> DoAction( Repeat(dance) )'
>
> As it was only recently added in trunk, and never made to release, we don't
> need to preserve for compatibility reasons.
Yeah, I'm not sure I see a point to DoAction either.
> 2) I want to remove actions.interval_actions.Mover
LOL! I didn't even know that was there!
I like the idea of a standard Mover, but I don't like that
implementation, for different reasons. For what it's worth I've just
implemented this mover:
class Move(actions.Action):
"""Move the target based on parameters on the target.
For movement the parameters are::
target.position = (x, y)
target.velocity = (dx, dy)
target.acceleration = (ddx, ddy) = (0, 0)
target.gravity = 0
And rotation::
target.rotation
target.dr
target.ddr
"""
def step(self, dt):
x, y = self.target.position
dx, dy = self.target.velocity
ddx, ddy = getattr(self.target, 'acceleration', (0, 0))
gravity = getattr(self.target, 'gravity', 0)
dx += ddx * dt
dy += (ddy + gravity) * dt
self.target.velocity = (dx, dy)
x += dx * dt
y += dy * dt
self.target.position = (x, y)
dr = getattr(self.target, 'dr', 0)
ddr = getattr(self.target, 'ddr', 0)
if dr or ddr:
dr = self.target.dr = dr + ddr * dt
if dr:
self.target.rotation += dr * dt
class WrappedMove(Move):
"""Move the target but wrap position when it hits certain bounds.
Wrap occurs outside of 0 < x < width and 0 < y < height taking into
account the dimenstions of the target.
"""
def init(self, width, height):
"""Init method.
:Parameters:
`width` : integer
The width to wrap position at.
`height` : integer
The height to wrap position at.
"""
self.width, self.height = width, height
def step(self, dt):
super(WrappedMove, self).step(dt)
x, y = self.target.position
w, h = self.target.width, self.target.height
# XXX assumes center anchor
if x > self.width + w/2:
x -= self.width + w
elif x < 0 - w/2:
x += self.width + w
if y > self.height + h/2:
y -= self.height + h
elif y < 0 - h/2:
y += self.height + h
self.target.position = (x, y)
class BoundedMove(Move):
"""Move the target but limit position when it hits certain bounds.
Position is bounded to 0 < x < width and 0 < y < height taking into
account the dimenstions of the target.
"""
def init(self, width, height):
"""Init method.
:Parameters:
`width` : integer
The width to bound position at.
`height` : integer
The height to bound position at.
"""
self.width, self.height = width, height
def step(self, dt):
super(BoundedMove, self).step(dt)
x, y = self.target.position
w, h = self.target.width, self.target.height
# XXX assumes center anchor
if x > self.width - w/2:
x = self.width - w/2
elif x < w/2:
x = w/2
if y > self.height - h/2:
y = self.height - h/2
elif y < h/2:
y = h/2
self.target.position = (x, y)
This is what I'll be using in the tutorial I'm presenting at
EuroPython. It seems nicer to me for a couple of reasons:
- the movement control vars are on the sprite (well, cocosnode) being
controlled instead of the action, and
- the movement control is more flexible, allowing acceleration and
movement along a different vector to the rotation.
Note the base class is actions.Action.
I'd like to put this into the cocos library as cocos/actions/move_actions.py
> Rationale:
>
> The intentions for this code seems to be:
> - allow external code to get access to the performing action
> - control from external code some actions parameters
> - move over a parabolic trajectory, have heading in consideration
>
> The first point is achieved by cheating __deepcopy__ to return the action
> itself, not a copy
> But deepcopy is central to the current actions implementation:
> i. used in all operators, and in particular Loop and Repeat will fail
> with an action cheating as such
> ii. it allows code like
> action = MoveBy( (100, 0), 3)
> node1.do(action)
> node2.do(action)
> to perform as expected, both nodes moving left by 100
> An action cheating deepcopy will broke that
I don't believe it makes sense to Loop or Repeat a Mover. Nor does it
make sense to have multiple objects sharing the same Mover instance.
> An there is an alternative way to allow external code to control action's
> parameters without touching deepcopy:
> cocosnode.do returns the action that will perform the changes, so the
> external code can do:
> action = ActionNotCheatingDeepcopy()
> worker_action = node.do(action)
> external code operates over worker_action
>
> (Your code was intended to do:
> mover = Mover()
> external code operates over mover
> )
>
> I suspect that controlled actions (when external code changes actions
> parameters in the action life) opens a whole new can of worms, like really
> not working good with operators.
Indeed.
> It can make sense at the app level, where the app context made clear there
> are special actions, not intended to be operated, not to be performed by two
> nodes, other restrictions my apply. And even at the app level I would not
> use the deepcopy cheat: the alternative reminds what is the normal idiom to
> refer to the performing action (like I need to know if I want to do
> node.remove_action : node.remove_action(worker_action) works )
>
> To summarize:
> 1) the deepcopy trick clashes with basic asumptions about actions
> behavior
> 2) the normal way to acces the performing action is ignored
> 3) controlled actions are unchartered territory; to play fair with the
> base actions we will need a base class for actions even more basic that
> Actions, and see which operators can be allowed ( top of my head spawn is
> plausible, but needs other implementation, Loop and Repeat seems less
> plausible, and very tricky to implement)
> 4) it is normal for users to look the lib code to write his own custom
> classes, and this will be steering them in the wrong direction in most of
> cases
>
> Should this action be retained in some form, it must be Action subclass, not
> IntervalAction, and some warning added about why is a special case and not a
> typical action.
Agreed, like I've done with my new Move class.
> I attach the example in test that uses Mover, modified to not use the
> deepcopy cheat (drop in the test dir to have resources)
>
> I you are ok, Richard, I can perform the relevant changes
I think it's reasonable to lose Mover as it does pose some problems as
you've outlined and also I find it less useful than it could be :-)
Richard
--
You received this message because you are subscribed to the Google Groups
"cocos2d discuss" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/cocos-discuss?hl=en.
