diogotr7 / OpenRGB.NET

C# client for the OpenRGB SDK
MIT License
40 stars 16 forks source link

Store ID numbers in Devices and Zones #13

Closed squeakyneb closed 2 years ago

squeakyneb commented 2 years ago

Gives the zone objects some useful value for patterns like:

IEnumerable<Zone> zones = devices.SelectMany(dev => dev.Zones);
Zone longZone = zones.OrderByDescending(_ => _.LedCount).First();
client.UpdateZone(longZone.DeviceID, longZone.ID, someColors);

or

IEnumerable<Zone> singleZones = zones.Where(z => z.Type == ZoneType.Single);
foreach (Zone z in singleZones)
{
    client.UpdateZone(z.DeviceID, z.ID, new Color [] { Color.FromHsv(startTime / 36 + z.ID * 60, 1.0, 1.0) } );
}

I found I'm otherwise having to constantly work downwards from the Device[] returned by GetAllControllerData() and it's all for loops and indexing and manually managing ID numbers, which just feels very clumsy.

I intend to extend this out to something like zone.Update(Color[] colors) (just storing a reference to the client automatically passed down like the ID numbers and calling back to client.UpdateZone, I suppose?). This allows for much more intuitive and less-manually-managed interaction.

Reaching out for some feedback. I'm not much of C# guy, but it's a nice intersection of some of my needs at the moment.

Cheers, Benny

diogotr7 commented 2 years ago

Hey, the inclusion of Ids in each class seems harmless and useful enough so if it's all the same to you I can merge it. As for stuff like device.Update() and zone.Update(), I'm not sure how to proceed. The client was written to be as thin as possible, no magic. I wrote it for Aurora and Artemis, and the current implementation serves that purpose well.

I haven't decided if I want to add such functionality to the client, or perhaps even add a separate set of classes for that purpose. I was already considering an async version of the client, as well as taking advantage of better memory management features of recent .net versions, so it's probably a good idea to think of all of these ideas together for a major version bump.

I'm not sure if people are using the library with .net framework but ideally i would drop support for it altogether.

squeakyneb commented 2 years ago

Ah, okay. I was wondering if there might be a difference of goals in the thick vs thin regard, which is why I made this pull request in the very early stages.

Perhaps if I extend past here, I make my own "thick" fork entirely? Is it possible to achieve both of our goals in the same library? I feel like dropping .NET would be a fairly solid split. Like I said, C# isn't my home turf, so I'm not sure of what the pros/cons of such a thing would be.

I'll un-draft this pull request, but I think I'd like to discuss further.