0.4.1: finish the grouping, type the protocol's numbers, and chain client options - #20
Merged
Merged
Conversation
GetSourceScreenshot returns "data:image/png;base64,..." rather than bare Base64, so Convert.FromBase64String threw and the in-memory helper returned null for every call. The file variant was unaffected because it never touched the payload.
The suite covered the generated requests but not the hand written conveniences, which is how the screenshot decode bug survived.
Every example still showed the flat extension methods, which are gone.
The 60 stream accessors were the last flat extensions on the client, so
half the surface was grouped and half was not. The protocol documents
requests and events under one set of category headings, and the groups
now hold both.
Renames the group types from {Category}RequestGroup to {Category}Group,
since they no longer hold only requests. Call sites go from
client.SceneCreatedStream() to client.Scenes.SceneCreatedStream().
Both helper files were left as hollow banner comments when their methods moved onto the category groups, and WaitForEventAsync had overloads split across two static classes. The lambda CallBatchAsync overload also still returned the untyped list while the builder overload returned BatchResults, so which overload you picked silently decided whether you got typed results.
The tool was pinned in dotnet-tools.json but nothing ever ran it, so 69 of 99 files had drifted. Some of that drift was misleading rather than cosmetic: the group structs kept the indentation of the extension blocks their methods were lifted out of. CI now checks it.
Six helpers decided "not found" by substring matching the exception message, so rewording a message would have turned a null return into a throw. ObsWebSocketRequestException now exposes the reported status as the protocol enum, and the helpers match on that. SetInputMutesAsync still built its batch out of raw BatchRequestItem and returned nothing, leaving per input failures visible only in the log. It uses the typed builder now and returns the results. FindSceneItemIdAsync returns int? so the Int32 suffixed duplicate is no longer needed, and the Try prefixed forwarder announced for removal in v0.4 is gone.
The protocol has one numeric type because JSON has one, so every Number became a double and callers read scene item ids, frame counts and byte counts as floating point. Which fields are integral is not recoverable from the definition: sceneItemId and inputVolumeMul are both Number with a >= 0 restriction. The classification is therefore an explicit table, not a rule over field names, because guessing wrong on a volume field truncates it silently while an unlisted field only stays double. 32 field names become int or long, 10 stay double. A refresh that adds an unlisted Number field reports OBSWSGEN012 rather than drifting in. Verified against OBS 32.2.2 on both transports: MessagePack decodes the retyped fields, which was the risk.
WithAutoConnect hung off IServiceCollection, so it read as applying to nothing and could not target one of two clients. AddObsWebSocketClient now returns a builder, the shape AddHttpClient uses, and WithAutoConnect, WithHealthCheck and WithReconnectPipeline chain off it. A named client gets its own connection service, its own options instance and a health check name that does not collide with the other client's. Also fixes a test that drove a FakeTimeProvider with a real Task.Delay, which cannot finish inside its own timeout when the suite runs in parallel. It waits on a signal now.
The protocol's RequestStatus enum shared a name with the RequestStatus record carried on every response, which is the pair a caller has to disambiguate to write a status filter. The enum is RequestStatusCode now. SwitchSceneAsync took a switchToProgram flag and sat next to the named shorthands, so the group appeared to offer five ways to switch a scene. The flag versions are private, reached through SwitchProgramSceneAsync and SwitchPreviewSceneAsync, and the missing preview-and-wait shorthand now exists. Two more connection tests drove real delays rather than the fake clock, and a container resolution test carried a one second wall clock limit with nothing in it that could hang. All three failed only under load.
The example built a typed batch and then dropped it to Raw, and registered the health check separately from the client. The README's status filter still named the enum by its old name, and neither the numeric typing nor the per client chain were documented.
26 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #14. Everything here was found by auditing the built assembly rather than the source, which turned up surface that #14 already claimed was done.
Grouping, finished
categoryfield inprotocol.jsonthat the requests use.{Category}RequestGroupbecomes{Category}Group.WaitForEventAsynchad overloads split across two static classes, and the twoCallBatchAsyncoverloads returned different types, so picking the lambda one silently gave untyped results.The protocol's numbers
JSON has one numeric type, so every
Numberbecamedoubleand callers read scene item ids, frame counts and byte counts as floating point. Which fields are integral is not recoverable from the definition:sceneItemIdandinputVolumeMulare bothNumberwith a>= 0restriction. The classification is an explicit table, not a rule over field names, because guessing wrong on a volume field truncates it silently while an unlisted field only staysdouble. 32 field names becomeintorlong, 10 staydouble, and an unlisted field reports OBSWSGEN012.Client options
AddObsWebSocketClientreturns a builder, soWithAutoConnect,WithHealthCheckandWithReconnectPipelinechain off it.WithAutoConnectpreviously hung offIServiceCollectionand could not target one of two named clients.Fixes
GetSourceScreenshotreturns a data URI, so the in-memory helper threw and returned null for every call. The file variant was unaffected, which is why it went unnoticed.ObsWebSocketRequestException.StatusCodeexposes the reported status as the protocol enum and they match on that.RequestStatusenum shared a name with theRequestStatusrecord on every response, which is the pair a caller has to disambiguate to write a status filter. It isRequestStatusCode.dotnet-tools.jsonand never run; 69 of 99 files had drifted. Some of that drift was misleading rather than cosmetic. CI checks it now.FakeTimeProviderand then waited on realTask.Delay, and one put a 1s wall clock limit on a test that only builds a container.Verification
Validated live against OBS 32.2.2 on both transports, 29 checks each with value assertions. Writing an int over MessagePack is asserted explicitly, including
0, which also covers the falsy-field fix from #11.Closes #14