----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/231/#review506 -----------------------------------------------------------
Ah, a good issue to tackle! Thanks for looking into it. indra/newview/llgesturemgr.h <http://codereview.secondlife.com/r/231/#comment377> Please use tabs for indentation and spaces for alignment. (Here, that'd be only one tab followed by 32 spaces.) Whatever you did here looks wrong whether I set tab display length to 4 or 8. indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment376> If the iterator will not be used outside the loop, please declare it with the assignment in the for's braces. indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment378> I'm not sure what you mean by "if it stops". Maybe that the currently handled gesture step stops the animation? If so, maybe write // Don't request the animation if this step stops it or if it is already in Static VFS indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment379> Why are STEP_CHAT and STEP_WAIT mentioned when they don't do anything and we do have a default step that also doesn't do anything? If all cases are supposed to be explicitly handled, we might want to make sure of that with: // [...] case STEP_CHAT: case STEP_WAIT: { break; } default: { // We should never get here. llassert(false); } } (... or maybe just some terminal output.) indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment374> s/is/if/ indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment375> Hmm ... so with the current change, a gesture's playback could potentially be delayed forever due to assets for other gestures being loaded? indra/newview/llgesturemgr.cpp <http://codereview.secondlife.com/r/231/#comment373> Can a gesture reference other asset types than animations and sounds? And if it can, shouldn't those be removed from mLoadingAssets, too, once loaded? If it can but shouldn't maybe some terminal output would be appropriate here. - Boroondas On March 25, 2011, 5:33 p.m., Seth ProductEngine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/231/ > ----------------------------------------------------------- > > (Updated March 25, 2011, 5:33 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > First pass implementation of syncing the animations and sounds before the > gesture starts playing. > The actual playing of animations and sounds of a gesture starts only when all > needed animations and sound files are loaded into viewer cache. This reduces > the delay between animations and sounds meant to be played simultaneously but > may increase the delay between the moment a gesture is triggered and the > moment it starts playing. > > > This addresses bug STORM-380. > http://jira.secondlife.com/browse/STORM-380 > > > Diffs > ----- > > indra/newview/llgesturemgr.h 6c15f820c3b9 > indra/newview/llgesturemgr.cpp 6c15f820c3b9 > > Diff: http://codereview.secondlife.com/r/231/diff > > > Testing > ------- > > > Thanks, > > Seth > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
