Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

Commit

Permalink
Convert all our IDs from strings to validated GUIDs (#476)
Browse files Browse the repository at this point in the history
* Start converting actor IDs to Guids

* More updates to actorId

* Convert actor network messages

* Start converting assets/containers

* Finish converting assets/containers

* Update user, client, message

* Convert mediaInstances

* Fix tests and other packages

* Remove another zeros check

* Don't throw
  • Loading branch information
stevenvergenz authored Jan 28, 2020
1 parent 3df87f2 commit 7a0ee5c
Show file tree
Hide file tree
Showing 39 changed files with 316 additions and 312 deletions.
7 changes: 4 additions & 3 deletions packages/altspacevr-extras/src/videoPlayerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import {
Context,
Guid,
User
} from '@microsoft/mixed-reality-extension-sdk';

Expand All @@ -17,7 +18,7 @@ type Video = { url: string; basisTime: number };
*/

export class VideoPlayerManager {
private videos = new Map<string, Video>();
private videos = new Map<Guid, Video>();

constructor(private context: Context) {
this.context.onUserJoined(this.userJoined);
Expand All @@ -41,7 +42,7 @@ export class VideoPlayerManager {
}
};

public play(actorId: string, url: string, startTime: number) {
public play(actorId: Guid, url: string, startTime: number) {
if (!this.videos.has(actorId) || this.videos.get(actorId).url !== url) {
const video = { url, basisTime: startTime - Date.now() / 1000.0 };
this.videos.set(actorId, video);
Expand All @@ -55,7 +56,7 @@ export class VideoPlayerManager {
}
}

public stop(actorId: string) {
public stop(actorId: Guid) {
if (this.videos.has(actorId)) {
this.context.rpc.send({
procName: 'VideoPlay'
Expand Down
6 changes: 3 additions & 3 deletions packages/functional-tests/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class App {
public assets: MRE.AssetContainer;

private firstUser: MRE.User;
private _connectedUsers: { [id: string]: MRE.User } = {};
private _connectedUsers = new Map<MRE.Guid, MRE.User>();
public testResults: { [name: string]: boolean } = {};

private activeTestName: string;
Expand Down Expand Up @@ -78,7 +78,7 @@ export class App {
}

private userJoined(user: MRE.User) {
this.connectedUsers[user.id] = user;
this.connectedUsers.set(user.id, user);
if (!this.firstUser) {
this.firstUser = user;
if (this.params.autorun === 'true' || this.params.nomenu === 'true') {
Expand All @@ -88,7 +88,7 @@ export class App {
}

private userLeft(user: MRE.User) {
delete this.connectedUsers[user.id];
this.connectedUsers.delete(user.id);
if (user === this.firstUser) {
this.firstUser = this.context.users[0] || null;
if (!this.firstUser) {
Expand Down
2 changes: 1 addition & 1 deletion packages/functional-tests/src/tests/clock-sync-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export default class ClockSyncTest extends Test {
return true;
}

public createAnimatableDigit(name: string, digits: string, parentId: string): MRE.Actor {
public createAnimatableDigit(name: string, digits: string, parentId: MRE.Guid): MRE.Actor {
return MRE.Actor.Create(this.app.context, {
actor: {
name,
Expand Down
10 changes: 5 additions & 5 deletions packages/functional-tests/src/tests/physics-sim-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default class PhysicsSimTest extends Test {
private collisionPegMat: MRE.Material;
private disabledPegMat: MRE.Material;
private ballMat: MRE.Material;
private collRefCount: { [id: string]: number } = {};
private collRefCount = new Map<MRE.Guid, number>();
private ballCount = 0;
private counterPlane: MRE.Actor;

Expand Down Expand Up @@ -116,15 +116,15 @@ export default class PhysicsSimTest extends Test {
}
});
peg.collider.onCollision('collision-enter', () => {
this.collRefCount[peg.id] = this.collRefCount[peg.id] + 1 || 1;
if (this.collRefCount[peg.id] > 0) {
this.collRefCount.set(peg.id, (this.collRefCount.get(peg.id) ?? 0) + 1);
if (this.collRefCount.get(peg.id) > 0) {
peg.appearance.material = this.collisionPegMat;
}
});

peg.collider.onCollision('collision-exit', () => {
this.collRefCount[peg.id]--;
if (this.collRefCount[peg.id] === 0) {
this.collRefCount.set(peg.id, this.collRefCount.get(peg.id) - 1);
if (this.collRefCount.get(peg.id) === 0) {
peg.appearance.material = this.defaultPegMat;
}
});
Expand Down
9 changes: 4 additions & 5 deletions packages/sdk/src/adapters/multipeer/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@

import { EventEmitter } from 'events';
import semver from 'semver';
import UUID from 'uuid/v4';
import { ClientExecution, ClientSync, MissingRule, Rules, Session } from '.';
import { Connection, Message } from '../..';
import { Connection, Guid, Message, newGuid } from '../..';
import { log } from '../../log';
import * as Protocols from '../../protocols';
import * as Payloads from '../../types/network/payloads';
Expand All @@ -30,7 +29,7 @@ export type QueuedMessage = {
export class Client extends EventEmitter {
private static orderSequence = 0;

private _id: string;
private _id: Guid;
private _session: Session;
private _protocol: Protocols.Protocol;
private _order: number;
Expand All @@ -49,14 +48,14 @@ export class Client extends EventEmitter {
public get queuedMessages() { return this._queuedMessages; }
public get userExclusiveMessages() { return this._userExclusiveMessages; }

public userId: string;
public userId: Guid;

/**
* Creates a new Client instance
*/
constructor(private _conn: Connection, private _version: semver.SemVer) {
super();
this._id = UUID();
this._id = newGuid();
this._order = Client.orderSequence++;
this._leave = this.leave.bind(this);
this._conn.on('close', this._leave);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class ClientExecution extends Protocols.Protocol implements Protocols.Mid
}

public beforeRecv = (message: Message): Message => {
if (this.promises[message.replyToId]) {
if (this.promises.has(message.replyToId)) {
// If we have a queued promise for this message, let it through
return message;
} else {
Expand Down
5 changes: 2 additions & 3 deletions packages/sdk/src/adapters/multipeer/protocols/clientSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
* Licensed under the MIT License.
*/

import UUID from 'uuid/v4';
import { Client, ClientDesyncPreprocessor, MissingRule, Rules, SyncActor } from '..';
import { Message } from '../../..';
import { Guid, Message, newGuid } from '../../..';
import { log } from '../../../log';
import * as Protocols from '../../../protocols';
import * as Payloads from '../../../types/network/payloads';
Expand Down Expand Up @@ -58,7 +57,7 @@ export class ClientSync extends Protocols.Protocol {
* Handle the outgoing message according to the synchronization rules specified for this payload.
*/
public sendMessage(message: Message, promise?: ExportedPromise, timeoutSeconds?: number) {
message.id = message.id || UUID();
message.id = message.id ?? newGuid();
const handling = this.handlingForMessage(message);
switch (handling) {
case 'allow': {
Expand Down
52 changes: 26 additions & 26 deletions packages/sdk/src/adapters/multipeer/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import deepmerge from 'deepmerge';
import { ActiveMediaInstance, Client, Session, SynchronizationStage } from '.';
import { MediaCommand, Message, WebSocket } from '../..';
import { Guid, MediaCommand, Message, WebSocket } from '../..';
import { log } from '../../log';
import * as Payloads from '../../types/network/payloads';
import { ExportedPromise } from '../../utils/exportedPromise';
Expand Down Expand Up @@ -79,7 +79,7 @@ export type Rule = {
* to stop processing of the message.
*/
beforeQueueMessageForClient: (
session: Session, client: Client, message: any, promise: ExportedPromise) => Message;
session: Session, client: Client, message: Message<any>, promise: ExportedPromise) => Message;
/**
* Called twice before a message is sent: first to determine if a message is user-dependent
* (it is queued until user-join if so), and second to determine if the joined user is the
Expand All @@ -91,7 +91,7 @@ export type Rule = {
* @returns `null` if the message does not depend on a user, `true` if it depends on the given
* user, and `false` if it depends on a different user.
*/
shouldSendToUser: (message: any, userId: string, session: Session, client: Client) => boolean | null;
shouldSendToUser: (message: Message<any>, userId: Guid, session: Session, client: Client) => boolean | null;
};

/**
Expand Down Expand Up @@ -224,7 +224,7 @@ const CreateActorRule: Rule = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.CreateActorCommon>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actor.id].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actor.id).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand Down Expand Up @@ -297,7 +297,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: Client,
message: Message<Payloads.ActorCorrection>
) => {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor && (client.authoritative || syncActor.grabbedBy === client.id)) {
const correctionPayload = message.payload;

Expand Down Expand Up @@ -373,7 +373,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
return message;
},
shouldSendToUser: (message: Message<Payloads.ActorUpdate>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actor.id].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actor.id).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -391,7 +391,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: Client,
message: Message<Payloads.ActorUpdate>
) => {
const syncActor = session.actorSet[message.payload.actor.id];
const syncActor = session.actorSet.get(message.payload.actor.id);
if (syncActor && (client.authoritative || syncActor.grabbedBy === client.id)) {
// Merge the update into the existing actor.
session.cacheActorUpdateMessage(message);
Expand Down Expand Up @@ -441,7 +441,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
},
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.AnimationUpdate>, userId: string, session: Session) => {
shouldSendToUser: (message: Message<Payloads.AnimationUpdate>, userId, session: Session) => {
// TODO: don't send animation updates when the animation targets only actors
// the client doesn't care/know about.
return true;
Expand Down Expand Up @@ -541,7 +541,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.CreateAnimation>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -554,7 +554,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
if (message.payload.animationId) {
session.cacheAnimationCreationRequest(message);
} else {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
const enabled = message.payload.initialState && !!message.payload.initialState.enabled;
syncActor.createdAnimations = syncActor.createdAnimations || [];
Expand Down Expand Up @@ -612,7 +612,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
message: Message<Payloads.DestroyActors>
) => {
for (const actorId of message.payload.actorIds) {
delete session.actorSet[actorId];
session.actorSet.delete(actorId);
}
return message;
}
Expand Down Expand Up @@ -678,7 +678,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.InterpolateActor>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -688,7 +688,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
session: Session,
message: Message<Payloads.InterpolateActor>
) => {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
syncActor.activeInterpolations = syncActor.activeInterpolations || [];
syncActor.activeInterpolations.push(deepmerge({}, message.payload));
Expand Down Expand Up @@ -732,7 +732,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
// Check that this is the authoritative client
const exclusiveUser =
message.payload.actors && message.payload.actors.length ?
session.actorSet[message.payload.actors[0].id].exclusiveToUser :
session.actorSet.get(message.payload.actors[0].id).exclusiveToUser :
undefined;
if (client.authoritative || client.userId && client.userId === exclusiveUser) {
// Create no-op creation message. Implicit sync from initialization until they're updated
Expand Down Expand Up @@ -775,7 +775,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
) => {
// Store the client id of the client that is performing the grab.
const payload = message.payload;
const syncActor = session.actorSet[payload.targetId];
const syncActor = session.actorSet.get(payload.targetId);
if (syncActor && payload.actionName.toLowerCase() === 'grab' &&
(syncActor.grabbedBy === client.id || syncActor.grabbedBy === undefined)
) {
Expand Down Expand Up @@ -843,7 +843,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.RigidBodyCommands>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
}
Expand Down Expand Up @@ -883,7 +883,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.SetAnimationState>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -895,7 +895,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
) => {
// If the app enabled or disabled the animation, update our local sync state to match.
if (message.payload.state.enabled !== undefined) {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
const animation = session.findAnimation(syncActor, message.payload.animationName);
if (animation) {
Expand All @@ -911,10 +911,10 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
message: Message<Payloads.SetAnimationState>
) => {
// Check that this is the authoritative client
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
if (client.authoritative || client.userId && client.userId === exclusiveUser) {
// Check that the actor exists.
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
// If the animation was disabled on the client, notify other clients and also
// update our local sync state.
Expand Down Expand Up @@ -962,7 +962,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.SetBehavior>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -972,7 +972,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
session: Session,
message: Message<Payloads.SetBehavior>
) => {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
syncActor.behavior = message.payload.behaviorType;
} else {
Expand All @@ -995,7 +995,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.SetMediaState>, userId, session, client) => {
const exclusiveUser = session.actorSet[message.payload.actorId].exclusiveToUser;
const exclusiveUser = session.actorSet.get(message.payload.actorId).exclusiveToUser;
return exclusiveUser ? exclusiveUser === userId : null;
}
},
Expand All @@ -1005,7 +1005,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
session: Session,
message: Message<Payloads.SetMediaState>
) => {
const syncActor = session.actorSet[message.payload.actorId];
const syncActor = session.actorSet.get(message.payload.actorId);
if (syncActor) {
syncActor.activeMediaInstances = syncActor.activeMediaInstances || [];
let activeMediaInstance: ActiveMediaInstance;
Expand Down Expand Up @@ -1077,7 +1077,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
}

// Look up asset duration from cached assets
const asset = session.assetSet[message.payload.mediaAssetId];
const asset = session.assetSet.get(message.payload.mediaAssetId);

if (activeMediaInstance.message.payload.options.looping === true ||
activeMediaInstance.message.payload.options.paused === true ||
Expand Down Expand Up @@ -1116,7 +1116,7 @@ export const Rules: { [id in Payloads.PayloadType]: Rule } = {
},
client: {
...DefaultRule.client,
shouldSendToUser: (message: Message<Payloads.ShowDialog>, userId: string) => {
shouldSendToUser: (message: Message<Payloads.ShowDialog>, userId) => {
return message.payload.userId === userId;
}
}
Expand Down
Loading

0 comments on commit 7a0ee5c

Please sign in to comment.