On Thu, Apr 29, 2010 at 9:48 AM, Joe Wreschnig <[email protected]>wrote:
> ....
> (I thought we were going to have the discussions of the patches on the
> issue tracker?)
>
>
This group is perfect for rapidfire discussions like this one, especially
since they are partly about more general topics than these specific issues.
Whatever part of this discussion needs to become part of the "permanent
record of the discussion of a specific issue" (according to whoever thinks
it's needed there) can be copied to the issue tracker, or summarized there,
or this thread can be referred to there.
It's very helpful to have the specific issues broken out as you did, even if
not much of the discussion gets copied into them, as long as the rationale
for the ultimate resolution gets mentioned there.
==
As for the specific issues:
480 -- I think the "3x speedup" should be mentioned in the issue itself, and
also you should make clear if this is just measured in the specific function
(as your timeit example suggests), or in the speed of some realistic whole
example which uses this function. The issue also claims the current code is
"too slow" without providing any evidence for that. ("Slower than it could
be" does not imply "too slow".)
But the change is so simple (and correct-looking) that it seems ok (IMHO) to
commit it even if the speedup is not so great. But someone more familiar
than me with this code should make that decision (and might have additional
requests for info to be reported in the issue).
481 -- sprites are not part of my area of experience in pyglet, but some
general comments:
- the issue ought to make it completely clear whether or not this is fully
backwards-compatible, whether a documentation change is required (probably
yes), and whether there are any potential downsides that might influence
someone's opinion about this change (e.g. a slowdown, or any change in which
graphics drivers would support the change).
- the issue doesn't document the new width and height properties and their
relation to the scale properties (if any). Maybe this would be obvious to
someone familiar with pyglet sprites, I don't know.
I agree with you that backwards compatibility is very important for this
proposed change.
482 -- the issue doesn't document *how* you set position and rotation at the
same time. Since this is a proposed API enhancement, it ought to be
documented in the issue (and ultimately in documentation) and not just in
the new code.
==
All three issues might benefit from a very clear initial line or phrase
classifying them as "optimization" or "API enhancement" or something else.
(Unless there is an issue tracker field for that which I missed.)
For the enhancements which require documentation changes, who will make
those changes? (Or maybe this is already covered, if the function docstrings
are changed and this is sufficient -- I don't know.)
Since there is still some confusion about branches in which development
should occur, you should also mention which branch (1.2dev or
1.1-maintenance) these patches were made against, and which ones you are
asking someone to make commits in (or whether you're leaving that entirely
up to someone else's decision).
==
As for the python version required by pyglet, which someone else raised in
this thread, IIRC this is already documented as 2.5.
- Bruce
--
You received this message because you are subscribed to the Google Groups
"pyglet-users" 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/pyglet-users?hl=en.
