Conversation
jsTron
commented
Sep 8, 2022
jsTron
commented
- Fix string conversion in GetGroupSceneItemList.
- Refactor get scene list method in test client and support top level groups.
| }; | ||
|
|
||
| var response = SendRequest(nameof(GetGroupSceneItemList), request); | ||
| return JsonConvert.DeserializeObject<List<JObject>>((string)response["sceneItems"]); |
There was a problem hiding this comment.
All other functions are in the format of (type)resonse["keyName"] is there a reason we're changing it here?
There was a problem hiding this comment.
The deserialization does not like the direct cast here, causing the request to hang. Now that you have me thinking, maybe changing List to something like a JArray could work as well.
There was a problem hiding this comment.
When I was testing with my code, it was failing on line 1747 saying it couldn't convert from Array to String but I changed it to this and got it to work:
return JsonConvert.DeserializeObject<List<SceneItemDetails>>(response["sceneItems"].ToString());
You'll notice I also changed the overall function to return as SceneItemDetails like it was before but apparently for whatever reason the (string) isn't allowed whereas .ToString() is fine.
| var node = new TreeNode(scene.Name); | ||
| var sources = new List<SceneItemDetails>(); | ||
| sources.AddRange(obs.GetSceneItemList(scene.Name)); | ||
| foreach (var item in sources) |
There was a problem hiding this comment.
Why do we need all this in the test client?
Overall groups should be avoided per the Websocket documentation:
"Using groups at all in OBS is discouraged, as they are very broken under the hood."
There was a problem hiding this comment.
Maybe we don't. If I recall correctly, the previous code received and used that info for free from the old API. I was operating under the premise that I would mimic that behavior.
* Fix string conversion in GetGroupSceneItemList. * Refactor get scene list method in test client and support top level groups.
jsTron
force-pushed
the
bugfix-getgroupsceneitemlist
branch
from
908a729 to
44c722b
Compare
3 participants
