Merge pull request #1309 from thecodingmachine/fix_variable_loops

Fixing loop when setting variables

back/src/Model/GameRoom.ts

@@ -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);

back/src/RoomManager.ts

@@ -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", () => {

back/src/Services/VariablesManager.ts

@@ -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)

front/src/Api/IframeListener.ts

@@ -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();

front/src/Api/iframe/state.ts

@@ -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);

front/src/Phaser/Game/SharedVariablesManager.ts

@@ -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);
         });
     }