Delegate connection creation to ChatScreen.

Author: retrixe

src/screens/ServerScreen.tsx

@@ -1,4 +1,4 @@
-import React, { useContext, useEffect, useRef, useState } from 'react'
+import React, { useContext, useEffect, useState } from 'react'
 import {
   StyleSheet,
   View,
@@ -27,17 +27,13 @@ import TextField from '../components/TextField'
 import ElevatedView from '../components/ElevatedView'
 import useDarkMode from '../context/useDarkMode'
 import ServersContext from '../context/serversContext'
-import useSessionStore from '../context/sessionStore'
-import SettingsContext from '../context/settingsContext'
-import AccountsContext from '../context/accountsContext'
 import ConnectionContext from '../context/connectionContext'
 import { parseIp, protocolMap } from '../minecraft/utils'
 import {
   ChatToJsx,
   lightColorMap,
   mojangColorMap
 } from '../minecraft/chatToJsx'
-import { createConnection } from './chat/sessionBuilder'
 
 interface RootStackParamList {
   [index: string]: any
@@ -48,13 +44,8 @@ type Props = NativeStackScreenProps<RootStackParamList, 'Chat'>
 
 const ServerScreen = (props: Props) => {
   const darkMode = useDarkMode()
-  const { settings } = useContext(SettingsContext)
   const { servers, setServers } = useContext(ServersContext)
-  const { accounts, setAccounts } = useContext(AccountsContext)
-  const { connection, setConnection, setDisconnectReason } =
-    useContext(ConnectionContext)
-  const { sessions, setSession } = useSessionStore()
-  const initiatingConnection = useRef(false)
+  const { setDisconnectReason } = useContext(ConnectionContext)
 
   const [ipAddr, setIpAddr] = useState('')
   const [ipAddrRed, setIpAddrRed] = useState(false)
@@ -147,46 +138,23 @@ const ServerScreen = (props: Props) => {
     cancelAddServer()
   }
 
-  const connectToServer = async (server: string) => {
-    if (initiatingConnection.current) return
-    if (!connection) {
-      initiatingConnection.current = true
-      let version = protocolMap[servers[server].version]
-      if (version === -1) {
-        const ping = pingResponses[servers[server].address]
-        // Try the latest.
-        if (!ping) version = protocolMap['1.19.1']
-        else if (typeof ping.version === 'object') {
-          version = ping.version.protocol
-        } else version = (ping as LegacyPing).protocol
-      }
-      if (version < 754) {
-        initiatingConnection.current = false
-        return setDisconnectReason({
-          server,
-          reason: 'EnderChat only supports 1.16.4 and newer (for now).'
-        })
-      }
-      // LOW-TODO: Creating a session would be better with a loading screen, since Microsoft Login is slow.
-      // Maybe setConnection(null) to bring up ChatScreen while still being in a logged out state?
-      // Or delegate this task to ChatScreen? Would make initiatingConnectionRef unnecessary.
-      try {
-        await createConnection(
-          server,
-          version,
-          servers,
-          settings,
-          accounts,
-          sessions,
-          setSession,
-          setAccounts,
-          setConnection,
-          setDisconnectReason,
-          () => {}
-        )
-      } catch (e) {}
-      initiatingConnection.current = false
+  const connectToServer = async (serverName: string) => {
+    let version = protocolMap[servers[serverName].version]
+    if (version === -1) {
+      const ping = pingResponses[servers[serverName].address]
+      // Try the latest.
+      if (!ping) version = protocolMap['1.19.1']
+      else if (typeof ping.version === 'object') {
+        version = ping.version.protocol
+      } else version = (ping as LegacyPing).protocol
     }
+    if (version < 754) {
+      return setDisconnectReason({
+        server: serverName,
+        reason: 'EnderChat only supports 1.16.4 and newer (for now).'
+      })
+    }
+    props.navigation.push('Chat', { serverName, version: version }) // getId prevents duplicate navigation.
   }
 
   const modalButtonCancelText = darkMode

src/screens/chat/ChatScreen.tsx

@@ -16,10 +16,14 @@ import {
   enderChatPrefix,
   sendMessageError
 } from './packetHandler'
+import { createConnection } from './sessionBuilder'
 import globalStyle from '../../globalStyle'
 import useDarkMode from '../../context/useDarkMode'
+import AccountsContext from '../../context/accountsContext'
+import ServersContext from '../../context/serversContext'
+import useSessionStore from '../../context/sessionStore'
 import SettingsContext from '../../context/settingsContext'
-import ConnectionContext from '../../context/connectionContext'
+import ConnectionContext, { Connection } from '../../context/connectionContext'
 import {
   ChatToJsx,
   mojangColorMap,
@@ -41,6 +45,8 @@ interface RootStackParamList {
 }
 type Props = NativeStackScreenProps<RootStackParamList, 'Chat'>
 
+export type Status = 'OPENING' | 'CONNECTING' | 'CONNECTED' | 'CLOSED'
+
 interface Message {
   key: number
   text: MinecraftChat
@@ -84,17 +90,26 @@ const handleError =
     addMessage(enderChatPrefix + translated)
   }
 
+const isConnection = (connection: any): connection is Connection =>
+  !!(connection as Connection).connection
+
 // TODO: Ability to copy text.
 const ChatScreen = ({ navigation, route }: Props) => {
   const darkMode = useDarkMode()
   const { settings } = useContext(SettingsContext)
-  const { connection, setConnection } = useContext(ConnectionContext)
+  const { servers } = useContext(ServersContext)
+  const { accounts, setAccounts } = useContext(AccountsContext)
+  const { connection, setConnection, setDisconnectReason } =
+    useContext(ConnectionContext)
+  const { sessions, setSession } = useSessionStore()
   // TODO: Show command history.
   const [, setCommandHistory] = useState<string[]>([])
   const [settingsOpen, setSettingsOpen] = useState(false)
   const [messages, setMessages] = useState<Message[]>([])
   const [loggedIn, setLoggedIn] = useState(false)
   const [message, setMessage] = useState('')
+  const healthRef = useRef<number | null>(null)
+  const statusRef = useRef<Status>(connection ? 'CONNECTING' : 'OPENING')
   const idRef = useRef(0)
 
   const charLimit =
@@ -106,17 +121,67 @@ const ChatScreen = ({ navigation, route }: Props) => {
       const trunc = m.length > 500 ? m.slice(0, 499) : m
       return [{ key: idRef.current++, text }].concat(trunc)
     })
+  const closeChatScreen = () => {
+    if (navigation.canGoBack() && statusRef.current !== 'CLOSED') {
+      navigation.goBack()
+    }
+  }
+
+  // Screen cleanup function.
+  useEffect(() =>
+    navigation.addListener('beforeRemove', () => {
+      statusRef.current = 'CLOSED'
+      // Gracefully disconnect on unmount, destroy will be called in 20s automatically if needed.
+      if (connection) {
+        connection.connection.close()
+        setConnection(undefined)
+      }
+    })
+  )
+
+  // Create connection useEffect.
+  useEffect(() => {
+    if (statusRef.current === 'OPENING') {
+      statusRef.current = 'CONNECTING'
+      createConnection(
+        route.params.serverName,
+        route.params.version,
+        servers,
+        settings,
+        accounts,
+        sessions,
+        setSession,
+        setAccounts,
+        setConnection,
+        setDisconnectReason,
+        closeChatScreen
+      )
+        .then(conn => {
+          console.log(statusRef.current)
+          console.log(isConnection(conn))
+          if (statusRef.current !== 'CLOSED') {
+            if (isConnection(conn)) setConnection(conn)
+            else setDisconnectReason(conn)
+          } else if (isConnection(conn)) conn.connection.close() // No memory leaky
+        })
+        .catch(() => {
+          closeChatScreen()
+          setDisconnectReason({
+            server: route.params.serverName,
+            reason: 'An error occurred resolving the server hostname!'
+          })
+        })
+    }
+  })
 
   // Packet handler useEffect.
-  const loggedInRef = useRef(false)
-  const healthRef = useRef<number | null>(null)
   useEffect(() => {
     if (!connection) return
     connection.connection.on(
       'packet',
       packetHandler(
         healthRef,
-        loggedInRef,
+        statusRef,
         setLoggedIn,
         connection.connection,
         addMessage,
@@ -138,19 +203,6 @@ const ChatScreen = ({ navigation, route }: Props) => {
     settings.sendSpawnCommand
   ])
 
-  // Cleanup useEffect on unload.
-  useEffect(() => {
-    if (!connection) {
-      navigation.goBack() // Safety net.
-      return
-    }
-    return () => {
-      // Gracefully disconnect, destroy will be called in 20s automatically if needed.
-      connection.connection.close()
-      setConnection(undefined)
-    }
-  }, [connection, setConnection, navigation])
-
   const sendMessage = (msg: string, saveHistory: boolean) => {
     if (!connection || !msg) return
     setMessage('')
@@ -191,7 +243,6 @@ const ChatScreen = ({ navigation, route }: Props) => {
     }
   }
 
-  if (!connection) return <></> // This should never be hit hopefully.
   const title =
     route.params.serverName.length > 12
       ? route.params.serverName.substring(0, 9) + '...'
@@ -203,7 +254,7 @@ const ChatScreen = ({ navigation, route }: Props) => {
         iconStyle={styles.backButtonIcon}
         backgroundColor='#363636'
         onPress={() => {
-          settingsOpen ? setSettingsOpen(false) : navigation.goBack()
+          settingsOpen ? setSettingsOpen(false) : closeChatScreen()
         }}
       />
     </View>
@@ -223,7 +274,7 @@ const ChatScreen = ({ navigation, route }: Props) => {
           onPress={() => setSettingsOpen(true)}
         />
       </View>
-      {!loggedIn && (
+      {(!loggedIn || !connection) && (
         <View style={styles.loadingScreen}>
           <ActivityIndicator
             color='#00aaff'
@@ -235,7 +286,7 @@ const ChatScreen = ({ navigation, route }: Props) => {
           <Text style={styles.loadingScreenText}>Connecting...</Text>
         </View>
       )}
-      {loggedIn && (
+      {loggedIn && connection && (
         <>
           <ChatMessageListMemo
             messages={messages}

src/screens/chat/packetHandler.ts

@@ -1,4 +1,5 @@
 import React from 'react'
+import { Status } from './ChatScreen'
 import { MinecraftChat, parseValidJson } from '../../minecraft/chatToJsx'
 import { ServerConnection } from '../../minecraft/connection'
 import { concatPacketData, Packet } from '../../minecraft/packet'
@@ -47,7 +48,7 @@ const handleSystemMessage = (
 export const packetHandler =
   (
     healthRef: React.MutableRefObject<number | null>,
-    loggedInRef: React.MutableRefObject<boolean>,
+    statusRef: React.MutableRefObject<Status>,
     setLoggedIn: React.Dispatch<React.SetStateAction<boolean>>,
     connection: ServerConnection,
     addMessage: (text: MinecraftChat) => any,
@@ -58,9 +59,9 @@ export const packetHandler =
     charLimit: number
   ) =>
   (packet: Packet) => {
-    if (!loggedInRef.current && connection.loggedIn) {
+    if (statusRef.current === 'CONNECTING' && connection.loggedIn) {
       setLoggedIn(true)
-      loggedInRef.current = true
+      statusRef.current = 'CONNECTED'
       if (sendJoinMessage) {
         connection
           .writePacket(

src/screens/chat/sessionBuilder.ts

@@ -28,24 +28,25 @@ export const createConnection = async (
   setConnection: (conn?: Connection) => void,
   setDisconnectReason: (reason: DisconnectReason) => void,
   closeChatScreen: () => void
-) => {
+): Promise<Connection | DisconnectReason> => {
   const [hostname, portNumber] = parseIp(servers[server].address)
   const [host, port] = await resolveHostname(hostname, portNumber)
   const activeAccount = Object.keys(accounts).find(e => accounts[e].active)
   if (!activeAccount) {
     closeChatScreen()
-    return setDisconnectReason({
+    return {
       server,
       reason:
         'No active account selected! Open the Accounts tab and add an account.'
-    })
+    }
   }
   const uuid = accounts[activeAccount].type ? activeAccount : undefined
   // Create an updated "session" containing access tokens and certificates.
   let session = sessions[activeAccount]
   const is119 = version >= protocolMap[1.19]
   if (uuid && (!session || (!session.certificate && is119))) {
     // LOW-TODO: Certificates and access tokens should be updated regularly.
+    // We should probably lock access to them via a semaphore.
     try {
       // Create a session with the latest access token.
       const account = accounts[activeAccount]
@@ -88,8 +89,7 @@ export const createConnection = async (
       closeChatScreen()
       const reason =
         'Failed to create session! You may need to re-login with your Microsoft Account in the Accounts tab.'
-      setDisconnectReason({ server, reason })
-      return
+      return { server, reason }
     }
   }
 
@@ -114,9 +114,9 @@ export const createConnection = async (
     }
     newConn.on('close', onCloseOrError)
     newConn.on('error', onCloseOrError)
-    setConnection({ serverName: server, connection: newConn })
+    return { serverName: server, connection: newConn }
   } catch (e) {
     closeChatScreen()
-    setDisconnectReason({ server, reason: 'Failed to connect to server!' })
+    return { server, reason: 'Failed to connect to server!' }
   }
 }