obsproject / obs-browser

CEF-based OBS Studio browser plugin
GNU General Public License v2.0
788 stars 226 forks source link

Inconsistancy with JS hooks #72

Closed SReject closed 4 years ago

SReject commented 7 years ago

1 - Passed Data Typings

With the obsSceneChanged event, <event>.detail is a JS object, similar to:

{height:1080,name:"brb",width:1920}

But with window.obsstudio.getCurrentScene() when the callback is called, the data argument is a non-parsed JSON string, resulting in data having to be passed through JSON.parse() before it is usable:

// data as String: {"height":1080,"name":"brb","width":1920}
window.obsstudio.getCurrentScene((data) => {
    console.log(data); // results in a string
    console.log(JSON.parse(data)); // results in a js object
});

I feel the JSON data of the latter should be parsed into a JS object as it is with the former.

2 - Different design patterns for similar functionality

The interface is all over the place on design for similar functionality sets. For example .onVisibilityChange is a property that gets set with a handler function while other events use the window.addEventListener() methodology.

3 - Incompleteness

Scenes have a getCurrentScene method and an onSceneChange event, but onVisibilityChange doesn't have an accompanying getter function; nor do the obsStreaming* and obsRecording* events.

Win10 - Home Edition
OBS Studio: 18.0.1 BrowserSource: v1.29.0

SReject commented 7 years ago

I've taken a bit of time, from a ES/JS developer standpoint, to write a short draft of what the interface should look like; it can be found HERE

styler commented 7 years ago

Any updates regarding this?

OsirisNL commented 6 years ago
  1. should be solved now, the code now does JSON.parse on the json string before sending it back, so you get a JS object instead of just a string.
  2. This is probably due to the fact that visibility is not an event fired from the obs frontend api. Not sure since I didn't write that part.
  3. Afaik it is not possible for the source to know if it is visible or not, since there can be multiple instances of the same source in the scene collection and they can all have different visibility states.
WizardCM commented 4 years ago

Alrighty, please review & test #186 which should cover requests 2 and 3. Let me know if I've missed anything you need!