Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mixxxbot opened this issue Aug 23, 2022 · 2 comments
Closed

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

mixxxbot opened this issue Aug 23, 2022 · 2 comments
Labels

Comments

@mixxxbot
Copy link
Collaborator

Reported by: wihola
Date: 2017-04-15T11:06:25Z
Status: Won't Fix
Importance: Undecided
Launchpad Issue: lp1683021


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.

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2017-04-15T18:08:52Z


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.

@mixxxbot
Copy link
Collaborator Author

Issue closed with status Won't Fix.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant