[improvement] [ChannelN], channel and engine.isScratching(channel)

Bug #1683021 reported by Wihola IT
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
Won't Fix
Undecided
Unassigned

Bug Description

Doesn't it looks like some kind of inconsistency with all that Channels numbers in code?

I am pretty understand that some changes in this area will break a lot of stuff, but still...

In Mixxx Controls we have
[𝗖𝗵𝗮𝗻𝗻𝗲𝗹𝟭], [𝗖𝗵𝗮𝗻𝗻𝗲𝗹𝟮], [𝗖𝗵𝗮𝗻𝗻𝗲𝗹𝗡], ... (start from 1)

in MyController.someFunction = function (channel, control, value, status, group) {
    print(channel); //start from 0

in `engine.setValue('[Channel1]', 'loop_in', 1);` we use string '[Channel1]'

in `engine.scratchDisable(int channel);` we use integer
thus we should do something like this:

MyController.someFunction = function (channel, control, value, status, group) {
    engine.scratchDisable(channel++);

or

MyController.getChannelN = function(value) {
    switch (value) {
        case '[Channel1]': return 0;
        case '[Channel2]': return 1;
        case '[Channel3]': return 2;
        case '[Channel4]': return 3;
        default: return 0;
    }
}

what is weird.

My proposition is to make channels start from 0
and pass in all functions '[ChannelN]' not integer or other stuff

Because I couldn't understand what's wrong with this function for 30 min
engine.scratchEnable(channel, 128, 33+1/3, alpha, beta);

All the time I had this error
𝐂𝐨𝐮𝐥𝐝 𝐧𝐨𝐭 𝐠𝐞𝐭𝐂𝐨𝐧𝐭𝐫𝐨𝐥𝐎𝐛𝐣𝐞𝐜𝐭𝐒𝐜𝐫𝐢𝐩𝐭()
and
𝐖𝐚𝐫𝐧𝐢𝐧𝐠 [𝐂𝐨𝐧𝐭𝐫𝐨𝐥𝐥𝐞𝐫]: 𝐂𝐨𝐧𝐭𝐫𝐨𝐥𝐃𝐨𝐮𝐛𝐥𝐞𝐏𝐫𝐢𝐯𝐚𝐭𝐞::𝐠𝐞𝐭𝐂𝐨𝐧𝐭𝐫𝐨𝐥 𝐫𝐞𝐭𝐮𝐫𝐧𝐢𝐧𝐠 𝐍𝐔𝐋𝐋 𝐟𝐨𝐫 ( "[𝐂𝐡𝐚𝐧𝐧𝐞𝐥𝟎]" , "𝐬𝐜𝐫𝐚𝐭𝐜𝐡𝟐" ) in the console.

We should direct the attention on this in Wiki and I gonna make some edits.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

I agree this is an easy way to get tripped up, but in Mixxx we strictly maintain backwards compatibility, especially of user control scripts so we effectively can't change this even if we wanted to.

> My proposition is to make channels start from 0 and pass in all functions '[ChannelN]' not integer or other stuff

The unfortunate bit is that strings are far less efficient than integers, and you frequently want to be using an integer to index into something, so if you're passing strings around you're going to be constantly slicing them up and converting them into indices which is wasteful.

I'd suggest doing the opposite -- stop using strings in your JS except where you need to interact with the control system. Instead, use indices everywhere, and whenever you need a group, pretend you know nothing about how to construct a group manually (since that can cause you to make off-by-one errors). Always call a helper function that gives you a group for an index e.g. groupForDeck(index), groupForSampler(index), etc.

Changed in mixxx:
status: New → Won't Fix
Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/8844

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.