Additional effects for Razer Kraken Kitty Edition #438
Conversation
ineffyble
changed the title
|
Nice work! I will have a look through tomorrow |
|
Worth noting that based on this we should probably be removing Spectrum from the Headphone defaults and adding it to the Kraken V2 specifically (or adding it as "missing" from the other headphones). |
| static createFeatureFrom(featureConfig) { | ||
| const featureIdentifier = Object.keys(featureConfig)[0]; | ||
| const configuration = featureConfig[featureIdentifier]; | ||
| static createFeatureFrom(featureIdentifier) { |
There was a problem hiding this comment.
Hopefully this change doesn't affect other devices? I'll test it with my keyboard tonight.
There was a problem hiding this comment.
@1kc It doesn't because no other devices use this. Every other device JSON has "features": null.
There was a problem hiding this comment.
Worth noting that based on this we should probably be removing Spectrum from the Headphone defaults and adding it to the Kraken V2 specifically (or adding it as "missing" from the other headphones).
Should we do that as a result of this change? Do you mind having a look through in that case then
No problems, will do.
There was a problem hiding this comment.
@1kc It doesn't because no other devices use this. Every other device JSON has
"features": null.
It's more about the property "featuresConfig" here which allows to configure the features. If I see it correctly, your change would remove this.
There was a problem hiding this comment.
@dhobi "featuresConfig" is applied via a separate function and doesn't use createFeatureFrom - it overrides the config on existing features: https://github.com/1kc/razer-macos/pull/438/files#diff-725951effb09a2201fc24d3dbc718b13771f12d775260c3fc14668d2ca873014R138
There was a lot of duplication between this and that when that one was created.
Effectively the way it works right now:
- Features are instantiated, either with no config if "features" is null and the default for main type is used, or the configuration from the relevant key under "features"
- If "featuresMissing" is defined, any features in that array are removed
- If "featuresConfig" is defined, we find the matching instantiated feature and override the config
This PR separates out instantiation from config, rather than "features" and "featuresConfig" both being responsible for defining feature configuration.
There was a problem hiding this comment.
Ahh yes, I see now that the configs are overridden later if needed. Thanks for pointing this out.
We might just get rid of the config parameter in the constructor in this case? We can easily do this in a separate step as well. What do you think?
There was a problem hiding this comment.
Makes sense to me, don't think it is used anywhere.
Should we do that as a result of this change? Do you mind having a look through in that case then |
|
@ineffyble, can this be merged? |
|
Hello! I wanted to open a ticket for this bug, I am really late but do you think you will merge this one? Regards. |
Add "Starlight", "Wave", and "Spectrum" effects for the Razer Kraken Kitty Edition.
Reliant on 1kc/librazermacos#10.
I took the liberty of refactoring the per-device
featureswhich has been unused since the introduction offeatureConfigand was a more verbose version of same.Fixes #411.