Effect chain preset menu tweaks

[ronso0]

• always show the effect slot number in the menu
• use kNoEffectString constant in DlgPrefEffects
• use kNoEffectString instead of "Empty Effect Slot" in the menu

[Swiftb0y]

Suggested change

	pEffectMenu->setTitle((effectSlotNumPrefix + pEffectSlot->getManifest()->displayName()));
	pEffectMenu->setTitle(effectSlotNumPrefix + pEffectSlot->getManifest()->displayName());

[Swiftb0y]

This entire thing could be written a bit more concisely if we choose the consistent formatting.

[ronso0]

I've moved fetching the parameter lists down and set the title in the initializer
Sqashed, rebased. Please check if that's what you had in mind.

[ronso0]

Alright now?

[Swiftb0y]

use kNoEffectString instead of "Empty Effect Slot" in the menu

I think this might unnecessarily worsen UX a little bit.

[ronso0]

use kNoEffectString instead of "Empty Effect Slot" in the menu

I think this might unnecessarily worsen UX a little bit.

Please elaborate why you think so.
I see two use cases:
1 A clean installation
The initial state is 3 empty effect slots. Users can click on effect drop-downs to load single effects, or open the effect chain menu and load a prepared chain. The label in the menu is now consistent with the empty effect selectors, so I'd say the connection menu slot > effect selector is clear, isnt it?
2 Upgrade to 2.4
Effect slots are (partially) populated and that is reflected in the chain menu

[Swiftb0y]

use kNoEffectString instead of "Empty Effect Slot" in the menu

I think this might unnecessarily worsen UX a little bit.

Please elaborate why you think so.

Sorry yes. I personally find the english sentence just nicer instead of just having the number and the placeholder string (kNoEffectString). I guess this is a discussion whether we favor consistency or (subjectively) nicer display.

Maybe UX was the wrong term since my suggestion was less about workflow and more just about the choice of string being displayed...

[ronso0]

I see, that's understandable.
My motivation is to remove the capitalized "Empty Effect Slot %1" because, at a glance, it's too similiar to capitalized chain preset names and it's not clear the slot is empty. Like in various preferences comboboxes I'd prefer a string that differentiates from the other items.

Is --- more clear if we put a heading above the slot actions Effect slots:? If not we could use (empty slot) or just empty.

[Swiftb0y]

Well non-empty effect slots should have a number and a colon at the start so you would spot the difference quite quickly. I can understand if we favor the consistency though.

On a related note, what do you think about replacing the --- with ⸺ (Unicode "Two-Em Dash": U+2E3A) for kNoEffectString?

[ronso0]

(empty)

(empty slot)

What do you think?

[JoergAtGithub]

What do you think?

I prefer the ---
It's faster to recognize, than reading a text.

[ywwg]

for what it's worth, I also prefer the --- or —, or even just "2: " (blank space)

[ywwg]

but "(empty)" is also fine.

[Swiftb0y]

I personally prefer --- or ⸺ (though the latter is nicer because --- looks like effect names were limited to ASCII chars).
Using text inplace of the effect name has the same effect as --- though its harder to see that its not a real effect.

[ronso0]

(seems some of my comments were lost after commenting on my mobile..)

Okay, so we have a majority for ---/⸺

On a related note, what do you think about replacing the --- with ⸺ ?

Hmm, --- indicates empty / missing characters, while ⸺ might be mistaken as 'decoration'?
Also, I vaguely remember I saw --- in other apps, but never ⸺.

Since --- is curently used for combobox placeholders in the preferences, and the scope of this PR is limited, I suggest to merge this now. @Swiftb0y If you want to change all placeholders at once please open a PR. Even though I prefer --- I won't stand in the way.

[ronso0]

I tried to address the other comment -- aprt from that it's okay now?

[Swiftb0y]

Thanks. LGTM