Fixes issue #1783 #1790
Fixes issue #1783 #1790
Conversation
…and not all connections. It goes to the home screen after there are no remaining connections.
Pull reviewers statsStats of the last 30 days for popstellar:
|
| @@ -139,7 +139,10 @@ class NetworkManager { | |||
|
|
|||
| if (alwaysPersistConnection) { | |||
| // always push the connection to the array so that we can re-connect later | |||
| this.connections.push(connection); | |||
| // if it doesn't already exist to avoid duplicates | |||
There was a problem hiding this comment.
Great improvement! Do you think it makes sense to use a set instead of an array?
There was a problem hiding this comment.
Arrays in Typescript have useful functions that Sets do not.
I see that sets can be useful because of the uniqueness of their elements, but due to the above point, I'd say we keep it as an array. I don't think that checking if a connection already exists in the array is going to introduce much overhead considering that the array is probably not going to be that big.
What do you think?
There was a problem hiding this comment.
I was just wondering since I would have assumed that we only need a small set of operations on that but if not we‘ll keep it, sure :)
| @@ -162,14 +165,16 @@ class NetworkManager { | |||
| } | |||
| } | |||
|
|
|||
| public disconnectFrom(address: string, intentional = true): void { | |||
| public disconnectFrom(address: string, intentional = true): Number { | |||
There was a problem hiding this comment.
Maybe add a tsdoc describing the return value? Also did you intentionally chose to return Number and not number? (https://stackoverflow.com/a/67155149)
There was a problem hiding this comment.
The tsdoc can be useful to have.
No, it was not intentional and I just learned about the difference between them from the link you shared. Thank you. Will be switching to number instead.
|
|
|
|
|
In case a connection dies, it now only disconnects from that connection and not all connections (considering that there are multiple connections). When there are no remaining connections, it navigates the to the home screen.