OpenFunscripter / OFS

A tool to create funscripts
GNU General Public License v3.0
90 stars 38 forks source link

Websocket API #59

Open OpenFunscripter opened 1 year ago

OpenFunscripter commented 1 year ago

A websocket based API for clients to connect to and receive events from OFS. All messages are going to be json.

Events should include:

Support for commands that can be sent to OFS should also be added for future additions.

OpenFunscripter commented 1 year ago

TODO

Yoooi0 commented 1 year ago

I think the additional id field might over complicate things as the script name itself can be used as the identifier. On name changes you could do funscript_removed with old name then funscript_changed with new name.

I also saw b8ae2aae8c4db299acd5af74ba63f2cb47b66a44, the ev property might be a little confusing, maybe name would be better? If you still want to tweak the json then maybe something like this?:

{
  "type": "event",
  "name": "play_change",
  "data": {
    "playing": true
  }
}

or just:

{
  "type": "event",
  "name": "play_change",
  "data": true
}

data would be common for each message, and could also be an json object depending on message type. I find the mpv ipc pretty nice to work with, but the whole property subscribing is probably overkill for OFS.

No further suggestions for now ๐Ÿ‘

OpenFunscripter commented 1 year ago

On name changes you could do funscript_removed with old name then funscript_changed with new name.

I'm fine with that solution. I don't have this unique id yet so not having to add makes it easier. ๐Ÿ˜… I think it's nice to have a small amount events that way clients don't have to handle too much. So it would be 8 events to handle.

Any further additions can be on a subscription basis but these 8 are kind of core.

I'll also change the json to the top one, better now than later.

OpenFunscripter commented 1 year ago

@Yoooi0 Another thing. For the funscript_change events I'm omitting the metadata completely I think it's just an empty object. I think that will be a later addition something like a metadata_change event.

Yoooi0 commented 1 year ago

Hmm, I think most players would expect the full funscript file with metadata. That might require internal changes to all players since they all load the whole funscript and then parse actions/metadata at the same time.

It will also complicate things due to event order, the player will have to store name -> actions map and name -> metadata map on funscript_change and metadata_change events, and then use the other map to create the combined "full" funscript. The player would have to load the funscript on each funscript_change and metadata_change since it would not be able to know if there will be another event coming later. And funscript_removed would also have to clear the metadata map.

My suggestion would be to always send the full combined funscript on each actions/metadata etc. change, guarantees matching state between OFS and players. It will be a lot more data to send but very simple. But I've seen 8MB funscripts so no idea how will that behave.

If we want to super optimize then the events would have to be more complicated, like:

{
  "type": "event",
  "name": "property_change",
  "data": {
    "path": "$.actions[1089].at", //json path
    "value": 12345
  }
}
{
  "type": "event",
  "name": "property_change",
  "data": [
    {
      "path": "$.actions[1089]", //json path
      "value": { "at": 12345, "pos": 100 }
    },
    {
      "path": "$.actions[1090]", //json path
      "value": { "at": 23456, "pos": 100 }
    },
    {
      "path": "$.actions[1091]", //json path
      "value": { "at": 34567, "pos": 100 }
    }
  ]
}
{
  "type": "event",
  "name": "property_change",
  "data": {
    "path": "$.metadata.author", //json path
    "value": "Bob"
  }
}

funscript_change would still send the full funscript for initial load. Players would store their own json object and update it on each property_change event.

OpenFunscripter commented 1 year ago

The way it currently works in OFS is that there's only one set of metadata per project. Meaning that the metadata is the same for all scripts but I can't guarantee that that doesn't change in the future.

It's a micro-optimization however with the addition of bookmarks and chapters the metadata could become significantly larger than it is right now. ๐Ÿค”

I don't have support for json paths but I do have support for json pointers. I didn't know either existed before this. ๐Ÿ™ƒ

Apparently my json library supports the ability to compute a json patch. This would be very advanced and not every client would have support for this on their end.

{
  "type": "event",
  "name": "funscript_patch",
  "data": {
    "patch": [
      { "op":" replace", "path": "/metadata/author", "value": "Bob" },
      { "op": "remove","path": "/actions/123" },
      { "op": "add", "path": "/actions/-", "value": { "at": 1234, "pos": 64 } }
   ]
}

I don't know if this is a valid patch I just typed this by hand.

Again very advanced not every library will have support for this but this is a standard https://www.rfc-editor.org/rfc/rfc6902. So you can just initialize using the entire script first and after that send these patches. This would still require some optimization on my end but the client should have an easy time assuming they have support for this.

But here's what I will do. For now I'll just put the metadata back into the funscript_change event. And I'll explore this json patch thing later as an optional flag which can be enabled but is turned off by default.

Yoooi0 commented 1 year ago

The way it currently works in OFS is that there's only one set of metadata per project. Meaning that the metadata is the same for all scripts but I can't guarantee that that doesn't change in the future.

I think thats fine, not sure what would require different metadata per script.

Apparently my json library supports the ability to compute a json patch. This would be very advanced and not every client would have support for this on their end.

Seems like mine doesnt support json patch, there is a addon library for that tho. So yea, I think sending the full script each time would be the simplest solution, no addition dependencies on player/ofs side.

If needed we could also use something like brotli compression, I compressed 8237KB script to 502KB, but not sure if the compression would take less time than sending it uncompressed, since the player and ofs will 99% of the time be on the same machine anyways.

OpenFunscripter commented 1 year ago

Seems like mine doesnt support json patch, there is a addon library for that tho. So yea, I think sending the full script each time would be the simplest solution, no addition dependencies on player/ofs side.

That's a shame would've been cool application for this I will also be consuming this api using the godot game engine and it doesn't have support for this either. It's just mostly used on the web I guess.

Like you said for now I'll stick with sending the entire script.

OpenFunscripter commented 1 year ago

Still a couple for todos but no more API changes:

Yoooi0 commented 1 year ago

So I guess we should also figure out the command api, not sure if you want to include it in this issue or leave it for the future.

I think we should just keep it similar to the events, just change the type to command:

{
  "type": "command",
  "name": "change_play",
  "data": {
    "playing": true
  }
}

And maybe change the names to:

change_media
change_play
change_time
change_playbackspeed

And I'm assuming these would be not implemented:

change_duration
change_funscript
remove_funscript
change_project
OpenFunscripter commented 1 year ago

I don't know what the behaviour of change_media would be. Because that could either change the media for the currently open project or it could try to load an entire new project. ๐Ÿคจ I'd rather not implement that tbh.

The rest is fine though. I also want some simple add_actions remove_actions commands as well as a change_funscript command. Questions is if these funscript modifying commands should still trigger funscript_changed events. Edit: They definitely should trigger funscript_changed events because there can be multiple websocket clients and I'd rather make things to complicated for myself.

I think there should be a response sent back when a command is processed. So there should be the ability to set a tag on commands which is then included in the response. But we could also say that all commands are processed in the order they are received so that responses are sent in the same order and not do any annoying message tagging. ๐Ÿค”

Yoooi0 commented 1 year ago

I don't know what the behaviour of change_media would be. Because that could either change the media for the currently open project or it could try to load an entire new project. ๐Ÿคจ I'd rather not implement that tbh.

Oh right, true.

I also want some simple add_actions remove_actions commands as well as a change_funscript command.

Then I guess remove_funscript could also be added. As for add_actions/remove_actions I think they could just be handled with change_funscript, it would work the same as the events from OFS where they always push the full funscript.

They definitely should trigger funscript_changed events because there can be multiple websocket clients and I'd rather make things to complicated for myself.

Yes they 100% should, and same with the other change_ commands:

client -> change_play -> OFS -> play_changed -> client`

For example this is how MFP expects the video players to behave, where MFP sends a command to play/pause media, but it actually doesnt do that internally until the player responds with an play/pause event. And same thing with change_time.

I think there should be a response sent back when a command is processed. So there should be the ability to set a tag on commands which is then included in the response.

I'm not sure how would it be used here, it would only be needed if you send multiple commands of the same type at the same time, or maybe with multiple clients. The commands dont really need a response, the response would be the event that would be sent by OFS after the state changes on its side, like in the example above.

Tho it maybe would help prevent circular loops like client updates script -> command -> ofs -> event -> client script gets updated -> command -> ofs ..., so the client would be able to tell if the script changed from ofs or it was a response to a command.

OpenFunscripter commented 1 year ago

At the very least there needs to be an error response a success response is unnecessary.

Yoooi0 commented 1 year ago

If you want to respond to a command on error then I think might as well respond on success.

Assuming that the commands will be async/fire and forget, then the tag/commandID you mentioned would be needed. So for example client sends change_play command which is non blocking:

{
  "type": "command",
  "name": "change_play",
  "data": {
    "playing": true
  },
  "commandID": 123
}

And after a while ofs responds with something like:

{
  "type": "command_executed" // needs better name
  "result": "error", // or "success"
  "data" : {
    // error data/message?
  },
  "commandID": 123
}

Or alternatively, client sends command and gets blocked until ofs executes it, then receives error/success result. This would not require the tag/commandID field and the "type" field.

OpenFunscripter commented 1 year ago

Really all I want is way to tell the API user that they are doing something wrong. How about a simple message event.

{
  "type": "event",
  "name": "message",
  "data": {
    "level": "warning",
    "msg": "Playback speed was clamped between 0.05 and 3.0",    
  }
}

When attempting to set the playback speed higher/lower than what is allowed in OFS.

Yoooi0 commented 1 year ago

TBH something like that should be specified in the api docs, that for example change_playbackspeed command accepts values between 0.05 and 3.0 and everything else will be clamped. It's not something that will be useful as a response to a command for the client implementing the api.

But this could be a separate thing, have "message"/"log" events that are dispatched during command execution, so that the client can pass OFS logs to its own logs. Tho then there is the issue of how can the client specify a log level so its not spammed with debug messages, but I guess you could just limit the logs to "info"/"warning"/"error".

I think easiest would be to either have no response or just simple "success" that the command has been received. So client sends a command, receives "success" response, then if the command fully succeeds executing there will be a _changed event, if it doesnt, there will be a message event with actual error.

OpenFunscripter commented 1 year ago

I added three commands change_play, change_time & change_playbackspeed. In the same order they expect a playing boolean, time in seconds float and speed as a float always inside of the data element.

Looks exactly like this

{
  "type": "command",
  "name": "change_play",
  "data": {
    "playing": true
  }
}

I'm going to leave it like this for know and defer any additions. Documenting this is also something I will defer for now. Because I'd like to get a new release out this year. ๐Ÿ˜…

Yoooi0 commented 1 year ago

The funscript_change event should also be raised when saving metadata and adding or removing bookmarks/chapters. Currently you have to update script actions for that stuff to refresh on the client.

OpenFunscripter commented 1 year ago

I knew there was something I forgot. It's a bit weird technically it needs to raise a funscript_change event for all loaded scripts. Because all loaded scripts contain the same metadata and the same chapters/bookmarks.

Yoooi0 commented 1 year ago

Yea that works, I don't think that will be an issue. I stress tested it with a 8mb funscript and there were no issues.