-
-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: websocket events #2641
feat: websocket events #2641
Conversation
Haven't forgotten about this! Plan to go through it this evening |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome!!
I do have one suggestion for consolidating code / helping maintain separation of concerns. Firstly just to say, I don't believe what I am going to suggest was even available when you started on this because it was only just added with the Ranks system work, so I wouldn't have expected you to have known about this or used it. But I updated our JsonDbManager
base class to also be an event emitter and it emits some common events including created-item
, updated-item
, and deleted-item
.
With that in mind, we could create a dedicated file within /server
, perhaps called like "firebot-component-event-websocket-relay.ts" (or something to that effect), and it would loop through relevant managers to register event listeners and relay them to the websocket.
eg:
import { TypedEmitter } from "tiny-typed-emitter";
import effectQueueManager from "../backend/effects/queues/effect-queue-manager";
// etc
type ComponentEvents = {
"created-item": (item: unknown) => void;
"updated-item": (item: unknown) => void;
"deleted-item": (item: unknown) => void;
};
const FIREBOT_COMPONENT_MANAGER_MAP: Array<[string, TypedEmitter<ComponentEvents>]> = [
["effect-queue", effectQueueManager],
["preset-effect-list", presetEffectListManager],
["timer", timerManager],
["counter", counterManager]
// etc
];
const MANAGER_WEBSOCKET_EVENT_MAP: Array<[keyof ComponentEvents, string]> = [
["created-item", "created"],
["updated-item", "updated"],
["deleted-item", "deleted"]
];
export function createComponentEventListeners() {
for (const [componentName, manager] of FIREBOT_COMPONENT_MANAGER_MAP) {
for (const [managerEvent, webSocketEvent] of MANAGER_WEBSOCKET_EVENT_MAP) {
manager.on(managerEvent, (item) => {
webSocketServerManager.triggerEvent(`${componentName}:${webSocketEvent}`, item);
});
}
}
}
Benefits of this:
- Centralizes the logic of forwarding events to websocket which simplifies maintenance
- Separation of concerns from the managers themselves
- Makes it super easy to add websocket events for additional firebot components in the future, just add its manager to the list
I realize though that some of the things you are emitting events for are not extending JsonDbManager (Namely the custom role manager, custom variable manager, and command manager). For those, I'd say for now we simply make them Event Emitters and have them emit those same three event signatures as JsonDbManager.
For the custom roles manager and the command manager, since they are in typescript, I would recommend using the TypedEmitter
class from the tiny-typed-emitter
package instead of the standard EventEmitter class so you can type the events.
For the custom variable manager, since it is not a class and in js, you could just export a EventEmitter instance and type it using jsdoc like so:
/**
* @type {import("tiny-typed-emitter").TypedEmitter<{
* "created-item": (item: unknown) => void;
* "deleted-item": (item: unknown) => void;
* "updated-item": (item: unknown) => void;
* }>}
*/
exports.events = new EventEmitter();
// elsewhere
exports.events.emit("created-item", newCustomVariable);
Also, speaking of the command manager, you'll likely have to treat it separately from the other managers inside of createComponentEventListeners
since you need to differentiate between custom and system commands (unless you can think of a clean way to integrate it with the rest!)
Hey @ebiggz, thanks for the review! I wanted get a feel for a solution I thought of for the command manager. Since commands provide their type ( What do you think? |
converted to draft while investigating an overlay related issue |
Note: Because this PR updates the files for the overlay, users who have OBS open while updating need to refresh the cache on any browser sources, or restart OBS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dennisrijsdijk I think simplifying to just "command" events and letting the consumers check the type property works great! This all looks good to me.
Description of the Change
This PR updates the current websocket implementation to add events for most subjects present in the HTTP API
The websocket is adopted to an IRE (Invoke, Response, Event) Model. IRE Model Example
Clients must invoke
subscribe-events
oroverlay-connected
before being able to receive events, and in the case ofoverlay-connected
, send eventsInvoke commands:
Incoming events:
Outgoing events:
send-to-overlay
command:created
command:updated
command:deleted
counter:created
counter:updated
counter:deleted
custom-role:created
custom-role:updated
custom-role:deleted
custom-variable:created
custom-variable:updated
custom-variable:deleted
effect-queue:created
effect-queue:updated
effect-queue:length-updated
effect-queue:deleted
preset-effect-list:created
preset-effect-list:updated
preset-effect-list:deleted
timer:created
timer:updated
timer:deleted
Testing
Ensured no messages are sent to client until handshaken as either events or overlay
Ensured no Overlay messages are sent to events clients
Ensured no Events messages are sent to overlay clients
Tested individual events manually using Postman as well as client application
Ensured builtin overlay effects function as expected
Ensured a third party overlay effect (cky spin wheel) that sends an event from overlay to backend functions properly