Pioneer DDJ-400: Cycle through focused effects #4662

etileb · 2022-02-05T10:13:21Z

New FX section.

uklotzde

Please rebase on the 2.3 branch. This should also fix some pre-commit issues.

We prefer to merge fixes and improvements of mappings into the current release version and release them with the next minor version.

uklotzde · 2022-02-05T10:36:03Z

res/controllers/Pioneer-DDJ-400-script.js

-
-    engine.setValue("[EffectRack1_EffectUnit1]", "focused_effect", 3);
+    var focusfx = engine.getValue("[EffectRack1_EffectUnit1]", "focused_effect")+1;
+    if (engine.getValue("[EffectRack1_EffectUnit1]", "focused_effect")===3){


Please add a short comment what this code does. I guess cycling through the 2 focused effects 1 and 2?

Please also check the coding style, i.e. add spaces and indentation.

I commented everything now, my first contrib on GitHub, sorry.

uklotzde · 2022-02-05T10:41:55Z

As a First-time contributor please also sign our Contributor License Agreement which allows us to distribute your code in the Apple App Store (in case it is ever needed).

uklotzde · 2022-02-05T10:45:53Z

Naming your PR like "DDJ-400: Cycle through focused effects" would help to guide reviewers.

The same applies to commit messages. Describe what this commit is changing. In imperative voice like I did in the PR title example.

uklotzde · 2022-02-05T11:13:23Z

res/controllers/Pioneer-DDJ-400-script.js

-
-    engine.setValue("[EffectRack1_EffectUnit1]", "focused_effect", 3);
+    var focusfx = engine.getValue("[EffectRack1_EffectUnit1]", "focused_effect")+1;
+    if (engine.getValue("[EffectRack1_EffectUnit1]", "focused_effect")===3){


Ok, I see, the code is confusing, there are 3 effects.

engine.getValue() should only be called once and assigned to variable, e.g. focused_effect

if focused_effect >== 3 { focused_effect = 1 } else { focused_effect = focused_effect + 1 }

Be-ing · 2022-02-05T13:07:07Z

What is the motivation for this change?

uklotzde · 2022-02-05T13:43:51Z

The manual also needs to be updated if the changes in this PR should be merged:

https://manual.mixxx.org/2.3/ca/hardware/controllers/pioneer_ddj_400.html#effect-section-p-9

I agree with @Be-ing that we need to know more about the motivation for making an informed decision how to proceed. We don't own this controller and cannot try it out ourselves.

uklotzde · 2022-02-05T13:48:01Z

The proposed changes would modify the focus behavior inconsistently, i.e. effect 1 and 2 could still be focused by dedicated buttons while effect 3 could only be reached by cycling through all of them.

I actually like the cycling apporach, but then the beat < > buttons would need to be remapped.

Beat FX buttons now change the focused effect, while the FX select button cycles the focused effect.

ronso0 · 2022-02-05T14:11:37Z

res/controllers/Pioneer-DDJ-400-script.js

@@ -44,7 +44,7 @@
 //        * Keyshift mode

 var PioneerDDJ400 = {};
-
+engine.setValue("[EffectRack1_EffectUnit1]", "focused_effect", 3);


I think this slipped in by accident?
(it's in the wrong place, and also looks like a personal preference since the focused effect is stored in mixxx.cfg)
Please revert.

No, it was like that before, it makes sure that there is an effect selected on startup.

This line didn't exist, that's why it shows up in the diff. Also, it's in the wrong place.
Please revert.

etileb · 2022-02-05T14:13:33Z

I found the layout confusing with multiple different Buttons for focusing only 3 effects. I changed it so that there is one button for cycling through the effects. I also was annoyed but having to press shift+beat FX to change the focused effect, so now it works without shift.

etileb · 2022-02-05T14:21:21Z

The proposed changes would modify the focus behavior inconsistently, i.e. effect 1 and 2 could still be focused by dedicated buttons while effect 3 could only be reached by cycling through all of them.

The FX select button is the only way of changing the focused effect now,

I actually like the cycling apporach, but then the beat < > buttons would need to be remapped.

That's what I actually did.

ronso0 · 2022-02-05T17:49:17Z

Please rebase onto mixxx/2.3.
Also, please install and run the pre-commit hooks to resolve the code style issues (see https://github.com/mixxxdj/mixxx/blob/main/CONTRIBUTING.md#git-workflow, 5th bullet point). The change set is small so you can simply squash the fixes and force-push.

ywwg · 2022-02-25T17:04:32Z

@etileb , if you're unable to do the rebase, I can take over this PR and clean it up for merge. I don't have a DDJ-400 so I will need someone with that controller to verify that it works

github-actions · 2022-05-27T00:07:32Z

This PR is marked as stale because it has been open 90 days with no activity.

tiesjan · 2022-10-17T07:42:09Z

Hi there! Since I own one of these controllers myself and I found the current effects configuration confusing, I've gone ahead and picked up these changes in PR #10912. If someone has the time to have a look, that would be great, thanks! 🙂

github-actions · 2023-08-29T00:06:54Z

This PR is marked as stale because it has been open 90 days with no activity.

daschuer · 2024-06-09T15:32:01Z

I think this was replaced by the already merged #10912 and can be closed. Please file a new PR if aspects are missing.

Update Pioneer-DDJ-400-script.js

8ee4960

New FX section.

github-actions bot added the controller mappings label Feb 5, 2022

uklotzde suggested changes Feb 5, 2022

View reviewed changes

uklotzde reviewed Feb 5, 2022

View reviewed changes

Holzhaus changed the title ~~Changed Pioneer-DDJ-400-script.js~~ Pioneer DDJ-400: Cycle through focused effects Feb 5, 2022

Update Pioneer-DDJ-400-script.js

cbecdf5

Beat FX buttons now change the focused effect, while the FX select button cycles the focused effect.

etileb changed the base branch from main to 2.3 February 5, 2022 14:04

ronso0 requested changes Feb 5, 2022

View reviewed changes

Update Pioneer-DDJ-400-script.js

da3c40d

github-actions bot added build code quality library skins ui labels Feb 7, 2022

github-actions bot added the stale Stale issues that haven't been updated for a long time. label May 27, 2022

This was referenced Aug 11, 2022

BeatFX needs some love timewasternl/Pioneered#1

Closed

Upgrade to the BeatFX section to have 3 effects with level meters and one for master FX level timewasternl/Pioneered#12

Merged

tiesjan mentioned this pull request Sep 24, 2022

Pioneer DDJ-400: make Beat FX section more intuitive #10912

Merged

JoergAtGithub removed the skins label Apr 6, 2023

JoergAtGithub removed library ui build code quality stale Stale issues that haven't been updated for a long time. labels Apr 6, 2023

github-actions bot added the stale Stale issues that haven't been updated for a long time. label Aug 29, 2023

daschuer changed the base branch from 2.3 to 2.4 June 9, 2024 15:30

daschuer closed this Jun 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pioneer DDJ-400: Cycle through focused effects #4662

Pioneer DDJ-400: Cycle through focused effects #4662

etileb commented Feb 5, 2022

uklotzde left a comment

uklotzde Feb 5, 2022

etileb Feb 5, 2022

uklotzde commented Feb 5, 2022

uklotzde commented Feb 5, 2022 •

edited

Loading

uklotzde Feb 5, 2022

Be-ing commented Feb 5, 2022

uklotzde commented Feb 5, 2022

uklotzde commented Feb 5, 2022 •

edited

Loading

ronso0 Feb 5, 2022

etileb Feb 5, 2022

ronso0 Feb 5, 2022 •

edited

Loading

etileb commented Feb 5, 2022 •

edited

Loading

etileb commented Feb 5, 2022 •

edited

Loading

ronso0 commented Feb 5, 2022

ywwg commented Feb 25, 2022

github-actions bot commented May 27, 2022

tiesjan commented Oct 17, 2022

github-actions bot commented Aug 29, 2023

daschuer commented Jun 9, 2024

Pioneer DDJ-400: Cycle through focused effects #4662

Pioneer DDJ-400: Cycle through focused effects #4662

Conversation

etileb commented Feb 5, 2022

uklotzde left a comment

Choose a reason for hiding this comment

uklotzde Feb 5, 2022

Choose a reason for hiding this comment

etileb Feb 5, 2022

Choose a reason for hiding this comment

uklotzde commented Feb 5, 2022

uklotzde commented Feb 5, 2022 • edited Loading

uklotzde Feb 5, 2022

Choose a reason for hiding this comment

Be-ing commented Feb 5, 2022

uklotzde commented Feb 5, 2022

uklotzde commented Feb 5, 2022 • edited Loading

ronso0 Feb 5, 2022

Choose a reason for hiding this comment

etileb Feb 5, 2022

Choose a reason for hiding this comment

ronso0 Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

etileb commented Feb 5, 2022 • edited Loading

etileb commented Feb 5, 2022 • edited Loading

ronso0 commented Feb 5, 2022

ywwg commented Feb 25, 2022

github-actions bot commented May 27, 2022

tiesjan commented Oct 17, 2022

github-actions bot commented Aug 29, 2023

daschuer commented Jun 9, 2024

uklotzde commented Feb 5, 2022 •

edited

Loading

uklotzde commented Feb 5, 2022 •

edited

Loading

ronso0 Feb 5, 2022 •

edited

Loading

etileb commented Feb 5, 2022 •

edited

Loading

etileb commented Feb 5, 2022 •

edited

Loading