-
Notifications
You must be signed in to change notification settings - Fork 34
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
Upgrade to TypeScript #39
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… from .ts files, more modules are converted, more modules use esm rather than cjs, started adding unit tests for roomserver, rollup working (tentatively)
…ples and added status module
… them. Made sure Unit Tests passed after.
…or Linter. Updated all the samples (and removed old ones). Fixed Rollup config. Updated bundle and made sure browser sample works.
From the in-person discussion just now:
|
@bnco-dev the above has been done now, and the negotiation works with the browser still! |
Additional jobs before this is done:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR completes #37 - upgrading the server to TypeScript.
It seems Github does not support changing the source branch of a PR, so we need to make a new one. The initial changes were contributed by TheBv, whose branch https://github.com/UCL-VR/ubiq/tree/serverUpgrade1 is based off.
While performing the TypeScript upgrade we have taken the opportunity to make a number of improvements, and start new features to make the server more extensible.
Some features didn't get as far as I'd have liked, but at least the foundations are there (e.g. HTTP admin API). For others, the half completed support has been pulled completely leaving the code cleaner until it can be scoped properly (e.g. subclassing rooms for the scalability project).
In full, we have:
Over a couple of blog posts I'll detail the changes in more detail, but to overview the things that are important to get running:
ts-node
The project has been configured to use ts-node. That is, there is no additional build step or artefacts. But node must be provided the arguments
--loader ts-node/esm app.ts
, either on the command line or viaNODE_OPTIONS
.This is the vscode launch config, for example,
And the
package.json
has astart
command added to it which does the same thing. (i.e.npm start
)Compatability
There are no changes to the actual server API, therefore once we have merged, the version on nexus should upgrade in place with the same port numbers transparently.
HTTPS/WSS
Just a reminder, since Chrome seems to be even more stringent nowadays, to use WebSockets, you need a local certificate and private key.
Create these with, e.g.
openssl req -nodes -new -x509 -keyout key.pem -out cert.pem
You will also likely need to visit the URI as an HTTP page so Chrome can show a warning about a mismatched hostname and you can accept it, before wss will work.
Status Module
The status module is in place but it only has a few endpoints at the moment. Hopefully now though it will be easier to upgrade. The status module runs its own HTTP server and a config entry has been added for it. It defaults to 8011. The API is intended to be consumed programmatically. We will need to add a small static page to display its results nicely, but being Json/JavaScript based its easy enough to read still.
rtcpeerconnection sample
This has been removed since wrtc seems out of development and has installation issues. Further our browser samples demonstrate using WebRTC in JavaScript now.
Browser Samples
The browser samples have already had this branch merged. (As that branch is actually an entire copy of the project, it doesn't need to be perfectly in sync at all times with master).
It now has just two (upstream/Node) changes, which we should merge back in here, right at the end:
NetworkContext
holds its ownnetworkId
member, instead of assuming the object has itLogCollector
has alog
member so it can be used in client-mode.PeerConnectionImpl
in Unity exposes the Unity nativePeerConnection
objectFuture Work
Things that ideally would be done in this PR but we didn't manage and its taking too long:
Larger future projects:
Make the Room and RoomServer strucutre more extensible so that we can add back the observer and KNN code from the scalability samples, without needing to make any upstream changes. This will need both the RoomPeer and Room types to be subclassed (the mistake before was only doing this for the Room type). Now with TypeScript perhaps we can make RoomServer generic in some way?
Make it possible to add 'services' at the same level as the RoomServer. For example, the Get/SetBlob APIs aren't great in their current form. This would be better moved out into a dedicated module, which would be straightforward if we can basically attach NetworkedComponents to a RoomPeer, as if they were attached to a NetworkScene, but before a Room was joined.
Outstanding things