Closed GoogleCodeExporter closed 9 years ago
This is as designed. If you want to show the boxes for multiple systems, you
could use the concat function of array:
var allBoxes = system1.boxes.concat(system2.boxes);
hemi.curve.showBoxes(allBoxes);
Original comment by erik.kit...@gmail.com
on 20 May 2011 at 11:01
[deleted comment]
[deleted comment]
This solution doesn't work if each particle system has different parents.
Can't we just go back to the previous system? Every time these function
signatures change it breaks my code.
Then takes time to test out the anew function that you are suggesting and write
unit tests, and now I see that it doesn't work for what I need.
I have a fix that I am going to commit along with a new unit test 8. This
doesn't stomp on your code, but it does add back the showBoxes() and
hideBoxes() functions to hemi.curve.ParticleSystem
Original comment by raj...@gmail.com
on 21 May 2011 at 5:38
I understand that it is frustrating to have the code signatures changing, and
we try to limit that as much as we can. But the API is still being developed
and very much in flux, so some change is going to happen.
I intentionally try not to have classes cluttered with a lot of debugging code,
which is why I moved the code to the utility function. It seems like you have a
legitimate use case though, so I'll see if I can move the logic out of the
functions you added, but leave them there as convenience functions.
Original comment by erik.kit...@gmail.com
on 23 May 2011 at 4:13
Updated hemi.curve.showBoxes and hemi.curve.hideBoxes to be able to support
multiple particle systems at the same time that have different transform
parents. Moved logic out of ParticleSystem but left convenience functions.
Original comment by erik.kit...@gmail.com
on 23 May 2011 at 9:55
Note: I was unable to verify my changes with the unit tests. It looks like
unit7.js and unit8.js were not committed or pushed up to the site.
Original comment by erik.kit...@gmail.com
on 23 May 2011 at 9:58
Hi Erik,
Yeah I forgot to commit the unit tests before. They are now committed and
pushed to the Mercurial repo. Thanks for doing that fix. Unit tests 8 and 9
work and I am switching this issue to 'Verified'
Yeah I felt bad about that commit all weekend. I know in cases where I have
taken upon myself to clean up code, and then have someone else commit more code
into it the class, ehhh well I know I wouldn't like it. It's a bit like when
I spend a bunch of time to clean out the living room and my roommate comes in
and just dumps a bunch of boxes in the corner.
So Sorry Erik. So maybe I should have spent the day working on something
else... and waited for a fix from you. I tend to get kind of OCD and just
obsess about resolving one issue. Thanks for not taking offense.
So yeah, as of now, I am spending like 80% of my time writing unit tests and to
some extent doing regression testing on this 1.3.2 release. Actually, its fun
to be part of the project, but I have a big workload right now and have to soon
reduce the unit test writing to maybe 20% of my time.
Cheers -- Raj
Original comment by raj...@gmail.com
on 23 May 2011 at 11:16
No problem, I know how it goes when you're working on an issue and want to see
it out. Thanks again for writing up these unit tests, I know they'll help the
stability of the project.
Original comment by erik.kit...@gmail.com
on 24 May 2011 at 8:05
Original issue reported on code.google.com by
raj...@gmail.com
on 20 May 2011 at 7:06