Fixing loop when setting variables

Setting a variable would makes the application enter in an infinite loop of events (between all the scripts and the back)
This fix makes sure a variable does not emit any event if it is changed to a value it already has.
This commit is contained in:
David Négrier 2021-07-23 11:50:03 +02:00
parent 84df25f863
commit 2aba6b1c27
6 changed files with 61 additions and 23 deletions

View file

@ -327,6 +327,11 @@ export class GameRoom {
const readableBy = variableManager.setVariable(name, value, user);
// If the variable was not changed, let's not dispatch anything.
if (readableBy === false) {
return;
}
// TODO: should we batch those every 100ms?
const variableMessage = new VariableWithTagMessage();
variableMessage.setName(name);

View file

@ -77,7 +77,11 @@ const roomManager: IRoomManagerServer = {
} else if (message.hasSilentmessage()) {
socketManager.handleSilentMessage(room, user, message.getSilentmessage() as SilentMessage);
} else if (message.hasItemeventmessage()) {
socketManager.handleItemEvent(room, user, message.getItemeventmessage() as ItemEventMessage);
socketManager.handleItemEvent(
room,
user,
message.getItemeventmessage() as ItemEventMessage
);
} else if (message.hasVariablemessage()) {
await socketManager.handleVariableEvent(
room,
@ -97,7 +101,10 @@ const roomManager: IRoomManagerServer = {
message.getWebrtcscreensharingsignaltoservermessage() as WebRtcSignalToServerMessage
);
} else if (message.hasPlayglobalmessage()) {
socketManager.emitPlayGlobalMessage(room, message.getPlayglobalmessage() as PlayGlobalMessage);
socketManager.emitPlayGlobalMessage(
room,
message.getPlayglobalmessage() as PlayGlobalMessage
);
} else if (message.hasQueryjitsijwtmessage()) {
socketManager.handleQueryJitsiJwtMessage(
user,
@ -128,7 +135,7 @@ const roomManager: IRoomManagerServer = {
emitError(call, e);
call.end();
}
})().catch(e => console.error(e));
})().catch((e) => console.error(e));
});
call.on("end", () => {

View file

@ -134,7 +134,17 @@ export class VariablesManager {
return variable;
}
setVariable(name: string, value: string, user: User): string | undefined {
/**
* Sets the variable.
*
* Returns who is allowed to read the variable (the readableby property) or "undefined" if anyone can read it.
* Also, returns "false" if the variable was not modified (because we set it to the value it already has)
*
* @param name
* @param value
* @param user
*/
setVariable(name: string, value: string, user: User): string | undefined | false {
let readableBy: string | undefined;
if (this.variableObjects) {
const variableObject = this.variableObjects.get(name);
@ -159,6 +169,11 @@ export class VariablesManager {
readableBy = variableObject.readableBy;
}
// If the value is not modified, return false
if (this._variables.get(name) === value) {
return false;
}
this._variables.set(name, value);
variablesRepository
.saveVariable(this.roomUrl, name, value)

View file

@ -32,9 +32,9 @@ import type { HasPlayerMovedEvent } from "./Events/HasPlayerMovedEvent";
import { isLoadPageEvent } from "./Events/LoadPageEvent";
import { handleMenuItemRegistrationEvent, isMenuItemRegisterIframeEvent } from "./Events/ui/MenuItemRegisterEvent";
import { SetTilesEvent, isSetTilesEvent } from "./Events/SetTilesEvent";
import { isSetVariableIframeEvent, SetVariableEvent } from "./Events/SetVariableEvent";
import type { SetVariableEvent } from "./Events/SetVariableEvent";
type AnswererCallback<T extends keyof IframeQueryMap> = (query: IframeQueryMap[T]['query']) => IframeQueryMap[T]['answer']|PromiseLike<IframeQueryMap[T]['answer']>;
type AnswererCallback<T extends keyof IframeQueryMap> = (query: IframeQueryMap[T]["query"], source: MessageEventSource | null) => IframeQueryMap[T]["answer"] | PromiseLike<IframeQueryMap[T]["answer"]>;
/**
* Listens to messages from iframes and turn those messages into easy to use observables.
@ -187,7 +187,7 @@ class IframeListener {
};
try {
Promise.resolve(answerer(query.data)).then((value) => {
Promise.resolve(answerer(query.data, message.source)).then((value) => {
iframe?.contentWindow?.postMessage({
id: queryId,
type: query.type,
@ -197,18 +197,6 @@ class IframeListener {
} catch (reason) {
errorHandler(reason);
}
if (isSetVariableIframeEvent(payload.query)) {
// Let's dispatch the message to the other iframes
for (iframe of this.iframes) {
if (iframe.contentWindow !== message.source) {
iframe.contentWindow?.postMessage({
'type': 'setVariable',
'data': payload.query.data
}, '*');
}
}
}
} else if (isIframeEventWrapper(payload)) {
if (payload.type === "showLayer" && isLayerEvent(payload.data)) {
this._showLayerStream.next(payload.data);
@ -439,6 +427,21 @@ class IframeListener {
public unregisterAnswerer(key: keyof IframeQueryMap): void {
delete this.answerers[key];
}
dispatchVariableToOtherIframes(key: string, value: unknown, source: MessageEventSource | null) {
// Let's dispatch the message to the other iframes
for (const iframe of this.iframes) {
if (iframe.contentWindow !== source) {
iframe.contentWindow?.postMessage({
'type': 'setVariable',
'data': {
key,
value,
}
}, '*');
}
}
}
}
export const iframeListener = new IframeListener();

View file

@ -23,11 +23,11 @@ export const initVariables = (_variables: Map<string, unknown>): void => {
setVariableResolvers.subscribe((event) => {
const oldValue = variables.get(event.key);
// If we are setting the same value, no need to do anything.
if (oldValue === event.value) {
// No need to do this check since it is already performed in SharedVariablesManager
/*if (JSON.stringify(oldValue) === JSON.stringify(event.value)) {
return;
}
}*/
variables.set(event.key, event.value);
const subject = variableSubscribers.get(event.key);

View file

@ -54,7 +54,7 @@ export class SharedVariablesManager {
});
// When a variable is modified from an iFrame
iframeListener.registerAnswerer("setVariable", (event) => {
iframeListener.registerAnswerer("setVariable", (event, source) => {
const key = event.key;
const object = this.variableObjects.get(key);
@ -82,10 +82,18 @@ export class SharedVariablesManager {
throw new Error(errMsg);
}
// Let's stop any propagation of the value we set is the same as the existing value.
if (JSON.stringify(event.value) === JSON.stringify(this._variables.get(key))) {
return;
}
this._variables.set(key, event.value);
// Dispatch to the room connection.
this.roomConnection.emitSetVariableEvent(key, event.value);
// Dispatch to other iframes
iframeListener.dispatchVariableToOtherIframes(key, event.value, source);
});
}