Module track change take 2

[bzztbomb]

heh, Another attempt at adding the ability to change what track a QTimelineItem belongs to. I've currently only exposed it via context menu on ModuleItems. The drag and drop just doesn't feel that solid and it's nice to be able to drag items within their tracks without worrying about bumping them up and down.

[q-depot]

got it. that's been merge.

[q-depot]

ah I'm, also considering to change the unit from seconds to bmp only, at the end if you want to use it in seconds you can always set the bmp to 60, it makes sense now especially with the support for audio tracks, let me know your thoughts.

[q-depot]

I merged almost everything, the only left is 8994f72 (change module track). I'll try to check it out over the weekend.
I've also renamed activeChanged to stateChanged and I'm thinking to do something similar for mActiveItem, I did that because soon or later I'll add the possibility to enable/disable a module and "active" would be confusing, as usual let me know your thoughts.

[bzztbomb]

I think the using bpm as the units for the timeline is a good choice. I also like "stateChanged" more than "activeChanged"!

[q-depot]

these are my thoughts on your change track commit:

moveModuleItem can be replaced by addModuleItem with an extra boolean variable that tells whether the item is being moved or created, if it's moved we don't cache the waveform or re-create the actual module in the main app(callback)

The more we add to the item UX and the more I feel like there is a problem that we must deal with. everytime we move an item we have to check the range, this logic should be abstracted somewhere else, I find it pretty annoying to repeat the same code in many different functions, especially if at some point we want to change our policy with the items, ie we want to crop the source module, the destination module or stop the action.

So if this is not urgent I'd say let's hold it feature a little longer and tidy up the code first.

[bzztbomb]

Makes sense to me. I did feel a little gross duplicating the logic the way
I did.

I did like having a separate method for moving verses adding. The add case
has much more going on (creation, init, etc). Move just involves adding
the module item to a list. I felt it was clearer to have different methods.

None of this pressing for me at the moment. Im just working on tools for
future performances.

Traveling at the moment, so sorry if this is short. Heh

On Friday, January 18, 2013, Andrea wrote:

these are my thoughts on your change track commit:

moveModuleItem can be replaced by addModuleItem with an extra boolean
variable that tells whether the item is being moved or created, if it's
moved we don't cache the waveform or re-create the actual module in the
main app(callback)

The more we add to the item UX and the more I feel like there is a problem
that we must deal with. everytime we move an item we have to check the
range, this logic should be abstracted somewhere else, I find it pretty
annoying to repeat the same code in many different functions, especially if
at some point we want to change our policy with the items, ie we want to
crop the source module, the destination module or stop the action.

So if this is not urgent I'd say let's hold it feature a little longer and
tidy up the code first.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-12412311.