refactor(transport): Consolidate transport interface #1002
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
Pass options object to P2P transport to use when instantiating the `Peer` connection.
Previously, different transport implementations received a reference to the client’s Redux store and duplicated logic to map update data from the master to an action which they dispatched to the store. This centralises this in the client, keeping the store a client-only concept and exposing a single callback to transport implementations that expects to receive the same `TransportData` emitted by the master. In this way, transports should be “thinner”, focusing on implementing the glue between a client and a master.
- `onAction` → `sendAction` - `onChatMessage` → `sendChatMessage`
…ack to base class
Rename internal `callback` property to `connectionStatusCallback` to better describe purpose.
delucis
changed the title
nicolodavis
previously approved these changes
Sep 7, 2021
|
Haha, I’m not sure if they were terrible 😄 My guess was just that the original class organically grew beyond its initial scope. I guess I should maybe also rename |
Yeah, I think that would help distinguish it from subscribing to the game state. |
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 refactors the transport interface to simplify the task of implementing new transports. (I’ve developed it out of the attempt at a peer-to-peer transport — I think eventually I’ll publish the P2P transport as a separate package.) This refactor moves various logic common to all transport implementations to the base
Transportclass and moves responsibility for interacting with the client’s Redux store to the client itself, making the transport “thinner”, primarily responsible for passing the master’s output to the client. It also renames some of the class methods to hopefully clarify their purpose.Breaking Changes
If anyone had figured out how to write a custom transport, it will definitely break. I don’t know how likely that is as we don’t export many of the things you’d ideally need access to to create a custom transport.
Checklist
main).