Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(325)

Issue 6374063: Updates for Dart language changes + bugfixes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by rupertk
Modified:
11 years, 8 months ago
Reviewers:
dominich, dominic
Base URL:
http://dartbox2d.googlecode.com/git/
Visibility:
Public.

Description

Updated for Dart's removal of string-concatenation. CanvasDraw, DebugDraw, World: - Documented that CanvasDraw mutates its arguments. - Fixed DebugDraw.e_* comments to apply to correct lines. - Fixed broken drawJoint() (CanvasDraw's argument mutation broke joints when they were rendered). - Fixed broken drawPolygon() (CanvasDraw's argument mutation broke AABB rendering). Did this by refactoring with drawSolidPolygon(). - Refactored drawCircle(), drawSolidCircle() and drawPoint() in same way. - Renamed drawPoint()'s argument names to be consistent with API. - Removed redundant DebugDraw.drawPolygon() implementation. - Implemented drawTransform(). - Reordered CanvasDraw methods to group more logically. - Made drawSolidCircle()'s unused "axis" argument optional and API-consistant with drawCircle(). - Added DebugDraw.e_lineDrawingBit and use for selecting line-drawn debug drawing (usually clearer for diagnosis). Color3f: Added == implementation. IViewportTransform: Added missing scale setter. Added extra demo "BoxTest" solely demonstrating a bouncing box. Minor bug fixes. Have regenerated docs and demo JS files locally but omitted from review since they obscure actual changes. Doc generated with: dart $DART_SDK/lib/dartdoc/dartdoc.dart --out=doc lib/box2d.dart (which generates a *lot* of files including PNGs (which may want omitting from Git repo?)) JS generated (without -c) for all demo/*.dart bar demo.dart. Would assume to include all of these in final commit.

Patch Set 1 #

Total comments: 30

Patch Set 2 : Updated changes from review + recent master changes. #

Total comments: 11

Patch Set 3 : Changes from CR round 3. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+235 lines, -120 lines) Patch
M demos.html View 1 1 chunk +1 line, -0 lines 0 comments Download
M demos/Bench2d.dart View 1 1 chunk +1 line, -1 line 0 comments Download
A demos/BoxTest.dart View 1 1 chunk +93 lines, -0 lines 0 comments Download
M lib/callbacks/CanvasDraw.dart View 1 2 3 chunks +52 lines, -40 lines 0 comments Download
M lib/callbacks/DebugDraw.dart View 1 2 3 chunks +28 lines, -42 lines 0 comments Download
M lib/collision/AxisAlignedBox.dart View 1 chunk +1 line, -1 line 0 comments Download
M lib/collision/Features.dart View 1 1 chunk +3 lines, -2 lines 0 comments Download
M lib/collision/SimplexVertex.dart View 1 chunk +1 line, -2 lines 0 comments Download
M lib/common/CanvasViewportTransform.dart View 1 chunk +1 line, -1 line 0 comments Download
M lib/common/Color3.dart View 1 chunk +8 lines, -0 lines 0 comments Download
M lib/common/IViewportTransform.dart View 1 chunk +6 lines, -0 lines 0 comments Download
M lib/common/Matrix22.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M lib/dynamics/World.dart View 1 9 chunks +30 lines, -21 lines 0 comments Download
M lib/dynamics/contacts/ContactConstraint.dart View 1 1 chunk +2 lines, -2 lines 0 comments Download
M lib/dynamics/contacts/ContactConstraintPoint.dart View 1 chunk +4 lines, -4 lines 0 comments Download
M lib/dynamics/joints/ConstantVolumeJoint.dart View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lib/dynamics/joints/FrictionJoint.dart View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16
dominich
http://codereview.appspot.com/6374063/diff/1/demos.html File demos.html (right): http://codereview.appspot.com/6374063/diff/1/demos.html#newcode15 demos.html:15: var demos = ['BoxTest', 'BallCage', 'Bench2d', 'BlobTest', 'CircleStress', nit: ...
11 years, 8 months ago (2012-07-12 20:19:04 UTC) #1
rupertk
Responded to all comments. Will likely action changes tomorrow. (Sorry I didn't keep the original ...
11 years, 8 months ago (2012-07-12 20:51:58 UTC) #2
dominich
http://codereview.appspot.com/6374063/diff/1/lib/common/Color3.dart File lib/common/Color3.dart (right): http://codereview.appspot.com/6374063/diff/1/lib/common/Color3.dart#newcode43 lib/common/Color3.dart:43: if (other is! Color3) { On 2012/07/12 20:51:58, rupertk ...
11 years, 8 months ago (2012-07-12 20:56:57 UTC) #3
rupertk
2 conversational responses -- perhaps better moved to the forum? http://codereview.appspot.com/6374063/diff/1/lib/common/Color3.dart File lib/common/Color3.dart (right): http://codereview.appspot.com/6374063/diff/1/lib/common/Color3.dart#newcode43 ...
11 years, 8 months ago (2012-07-12 21:21:56 UTC) #4
dominich
On Thu, Jul 12, 2012 at 2:21 PM, <Rupert.Key@gmail.com> wrote: > 2 conversational responses -- ...
11 years, 8 months ago (2012-07-12 22:12:23 UTC) #5
rupertk
All changes made but will upload at weekend since something came up today. (specifically, a ...
11 years, 8 months ago (2012-07-13 22:01:59 UTC) #6
dominic_google.com
I hope the hedgehog is doing ok. I inadvertently pushed a change today that includes ...
11 years, 8 months ago (2012-07-16 22:04:05 UTC) #7
rupert.key_gmail.com
Man, you do keep doing this to me ;-) I was about to upload.py when ...
11 years, 8 months ago (2012-07-16 23:44:34 UTC) #8
dominic_google.com
On Mon, Jul 16, 2012 at 4:44 PM, Rupert Key <rupert.key@gmail.com> wrote: > Man, you ...
11 years, 8 months ago (2012-07-16 23:50:30 UTC) #9
rupert.key_gmail.com
(inline) On Tue, Jul 17, 2012 at 12:50 AM, Dominic Hamon <dominic@google.com> wrote: > On ...
11 years, 8 months ago (2012-07-17 00:06:11 UTC) #10
rupertk
New version of changes (without generated bits) based on your recent changes. Hopefully all set ...
11 years, 8 months ago (2012-07-17 01:21:30 UTC) #11
dominich
On 2012/07/17 01:21:30, rupertk wrote: > New version of changes (without generated bits) based on ...
11 years, 8 months ago (2012-07-17 03:04:31 UTC) #12
dominich
close - just a few nits. http://codereview.appspot.com/6374063/diff/11002/lib/callbacks/CanvasDraw.dart File lib/callbacks/CanvasDraw.dart (right): http://codereview.appspot.com/6374063/diff/11002/lib/callbacks/CanvasDraw.dart#newcode34 lib/callbacks/CanvasDraw.dart:34: * Draw a ...
11 years, 8 months ago (2012-07-17 03:04:55 UTC) #13
rupertk
Bit of a rush but that's all bar 1 change (what's the CL?) I'm out ...
11 years, 8 months ago (2012-07-17 15:56:48 UTC) #14
dominich
LGTM Let me confirm that we've received the signed CLA and then you I'll add ...
11 years, 8 months ago (2012-07-17 16:19:12 UTC) #15
dominich
11 years, 8 months ago (2012-07-17 17:07:00 UTC) #16
Congratulations - you're a committer!

- dominic



On Tue, Jul 17, 2012 at 9:19 AM, <dominich@google.com> wrote:

> LGTM
>
> Let me confirm that we've received the signed CLA and then you I'll add
> you as a contributor. At that point you'll be able to commit patches.
>
> When you submit drawTransform you can mark bug 7 as fixed.
>
>
>
> http://codereview.appspot.com/**6374063/diff/11002/lib/**
>
callbacks/CanvasDraw.dart<http://codereview.appspot.com/6374063/diff/11002/lib/callbacks/CanvasDraw.dart>
> File lib/callbacks/CanvasDraw.dart (right):
>
> http://codereview.appspot.com/**6374063/diff/11002/lib/**
>
callbacks/CanvasDraw.dart#**newcode125<http://codereview.appspot.com/6374063/diff/11002/lib/callbacks/CanvasDraw.dart#newcode125>
> lib/callbacks/CanvasDraw.dart:**125: void drawTransform(Transform xf,
> Color3 color) {
> Sorry, the Changelist description. That means the commit message that
> you use to introduce drawTransform.
>
>
> On 2012/07/17 15:56:48, rupertk wrote:
>
>> What's CL?  (probably being dumb)
>>
>
>  On 2012/07/17 03:04:55, dominich wrote:
>> > can you edit the CL description to include BUG=7
>>
>
>
>
http://codereview.appspot.com/**6374063/<http://codereview.appspot.com/6374063/>
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b