Allow creation of effects without setting them

[WolfspiritM]

While Colore is great cause of the many Helper Methods (like SetCustom...) I think the base Methods of the SDK should be public and similar to the C++ implementation to not confuse people. This allows for easier porting of existing C++ or C# code to the other language.

It also allows more advantage use of storing and retriving frames with CreateEffect and SetEffect(guid).

That means:

• Create(Device)Effect should be made available to the public as a method under each Device and have a way to pass NULL as ref which seems to directly apply the effect after creation as seen in the Chroma Sample SDK App.
• SetEffect should accept a GUID instead of an Effect(Type).
• ... more(?)

Your thoughts about this?

Possible implementation 1. Guid passed as out as in the Chroma SDK.:

# Get GUID and Apply
var custom = Custom.Create();
custom.Set(Color.Red);
Guid guid;
Chroma.Instance.Keyboard.CreateEffect(Corale.Colore.Razer.Keyboard.Effects.Effect.Custom, custom, out guid);
if (guid != Guid.Empty) Chroma.Instance.Keyboard.SetEffect(guid);

# Apply directly
var custom2 = Custom.Create();
custom2.Set(Color.Blue);
Chroma.Instance.Keyboard.CreateEffect(Corale.Colore.Razer.Keyboard.Effects.Effect.Custom, custom2);

Possible implementation 2. CreateEffect with additional parameter for applying instead of returning Guid:

# Get GUID and Apply
var custom = Custom.Create();
custom.Set(Color.Red);
var guid = Chroma.Instance.Keyboard.CreateEffect(Corale.Colore.Razer.Keyboard.Effects.Effect.Custom, custom);
if (guid != Guid.Empty) Chroma.Instance.Keyboard.SetEffect(guid);

# Apply directly
var custom2 = Custom.Create();
custom2.Set(Color.Blue);
Chroma.Instance.Keyboard.CreateEffect(Corale.Colore.Razer.Keyboard.Effects.Effect.Custom, custom2, true);

[Sharparam]

For setting a GUID, we already have the method SetGuid for that (IDevice). Maybe it's worth adding an overload to SetEffect that takes a Guid to mimic the SDK endpoints.

For directly applying the effect, we already have the normal methods which do just that (and the GUID of the created effect can be retrieved with the CurrentEffectId property of IDevice).

As for how to do effect creation while not actually setting the effect, we'll need some discussion on how to best implement it.

The most sensible might be to add Create* methods alongside the existing Set* methods that will create the effect and return its GUID, but not actually pass them to the internal SetEffect SDK endpoint.

The only worry is that we'd be effectively doubling the amount of methods for every device class, making them quite large. Maybe it's possible to put the creation logic in the effect struct themselves, meaning it would be something like: new Static(Color.Blue).Id.

[WolfspiritM]

If we want to mimic the SDK Endpoints then having a CreateEffect for each Device like that should be enough to just forward everything to the Native Method:

        public Guid CreateEffect<T>(Effect effect, T @struct) where T : struct
        {
            return NativeWrapper.CreateKeyboardEffect(effect, @struct);
        }

The current Set* methods all retrieve the GUID, delete the old Effect and apply the new one.
By just setting the Effect without a GUID this reduces the amounts of SDK calls needed.
In nearly all cases the GUID isn't actually needed.
We might just need to ask Razer if an Effect set like that still needs cleanup (I don't think it needs) as their Sample SDK doesn't do any cleanup, or if the Device (or SDK) handles the cleanup itself. If no cleanup is needed we can save two SDK calls (SetEffect, DeleteEffect) by changing the way the current effects are applied.

[Sharparam]

Reworking the Create methods to be able to pass NULL for the Guid parameter would be quite a bit of work if they also need to be able to return a Guid for other methods, so unless there is a significant performance gain to be had from doing that, I don't think we will pursue that path right now.

Last time we contacted Razer about some issue related to endpoints they said that DeleteEffect must always be called before a new one is set. But it's not present anywhere in the sample app as you say, so maybe this has changed recently.

[Sharparam]

Possibly related to #31.

[njbmartin]

As much as I hate seeing people write this:

+1

[Sharparam]

GitHub added reactions for just that purpose you know.

On Tue, 5 Jul 2016, 10:40 Nico, notifications@github.com wrote:

As much as I hate seeing people write this:

+1

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#183 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAhott6_EpTlnXgzY1mJGBXDHTMCUDDcks5qShhogaJpZM4JEjqj
.

[Sharparam]

Moving this to v6.1 milestone. We'll probably put in the Create* methods on each device for obtaining effect GUIDs without setting them, at least to begin with.