Should global color still trigger custom effects?

[revision29]

Since the global color has been moved into device settings, it makes less sense for effect manager to create a custom fire effect or even spectrum analyzer when a global color is being set. If I call EffectManager::ApplyGlobalColor I do not expect it to draw a special fire effect. I expect it to set the color and let whatever effects are active read the color and act accordingly. It doesn't even make sense anymore to trigger a spectrum analyzer based on the global color. It does make sense if a spectrum analyzer effect is active for it to read the global color and adjust the color scheme to match.

[rbergen]

This question is basically answered in #485: no it shouldn't, but before we can stop EffectManager doing that, more effects need to adopt the global colors when they are set.

[revision29]

Thanks for the quick reply. I didn't even notice the global color code had even reworked until last night. When that PR was adopted, I was on vacation and was away from the internet most of the time. I had been working on my own implantation and started to port my changes to a fresh pull of the codebase. But, I quickly found the improvements.

Back to the question: I don't even think it is necessary for change any more effects before removing the creation of custom effects by effects manager. Besides changing the palette for matrices, only the spectrum analyzer and a custom MusicalPaletteFire were impacted. A fallback to a generic PaletteFlameEffect, that does not even work, is triggered for non-audio systems. What I am getting at is two effects were ever involved. Since only two effects are really impacted it does not make as much sense to hold back progress. I am keeping this discussion disconnected form my most recent global solid fill color pull request.

[rbergen]

I don't even think it is necessary for change any more effects before removing the creation of custom effects by effects manager.

That's actually a fair point.

I am keeping this discussion disconnected form my most recent global solid fill color pull request.

That's fine. As you'll see, I did make the link to this discussion from the PR in question. :)

[revision29]

So, do you mind if I go ahead and make a separate pull to eliminate the custom effects in effects manager once the new, new color fill pull request is accepted? I am trying to get back to the active codebase and this is the final hurdle.

Also, I need advice for keeping forks in sync with the main branch while also keeping some personal patches in place. I want to keep everything else in sync but maintain my own custom IR remote handling code. I want to push those changes to my GitHub repo but not have the changes override when I merge changes from main. This also applies to effects.cpp and globals.h. For the latter two files, I want to include any changes made upstream, but maintain my own entries.

[rbergen]

No, I don't mind. You made a valid point. If someone wants something to respond to the global color(s) they can change the effect(s) they want to do so to pull it/them in.

It sounds to me like you're looking for file-level integration between the main tree and your changes. For effects.cpp, one can create a "custom_effects.h" file with the entries you want to use. That will then always be the set that is used as long as that file exists. For globals.h a mechanism like that is not yet in place, but could be added to globals.h in the main tree to pull in a "custom_globals.h" (if present) to replace whatever project config (DEMO, SPECTRUM, MESMERIZER, ...) would otherwise be used.

How to hang on to your own IR remote code while the code in the main tree might evolve I wouldn't know. I know @robertlipe is working on quite a few branches in parallel that also include local customizations, maybe he can provide some advice on this.