External player clean up (#914)

* minor `addSubtitles` clean up

* external player clean up

src/main/external-player.js

@@ -5,6 +5,7 @@ module.exports = {
 }
 
 const cp = require('child_process')
+const path = require('path')
 const vlcCommand = require('vlc-command')
 
 const log = require('./log')
@@ -13,15 +14,15 @@ const windows = require('./windows')
 // holds a ChildProcess while we're playing a video in an external player, null otherwise
 let proc = null
 
-function checkInstall (path, cb) {
+function checkInstall (playerPath, cb) {
   // check for VLC if external player has not been specified by the user
   // otherwise assume the player is installed
-  if (path == null) return vlcCommand((err) => cb(!err))
+  if (playerPath == null) return vlcCommand((err) => cb(!err))
   process.nextTick(() => cb(true))
 }
 
-function spawn (path, url, title) {
-  if (path != null) return spawnExternal(path, [url])
+function spawn (playerPath, url, title) {
+  if (playerPath != null) return spawnExternal(playerPath, [url])
 
   // Try to find and use VLC if external player is not specified
   vlcCommand(function (err, vlcPath) {
@@ -44,10 +45,15 @@ function kill () {
   proc = null
 }
 
-function spawnExternal (path, args) {
-  log('Running external media player:', path + ' ' + args.join(' '))
+function spawnExternal (playerPath, args) {
+  log('Running external media player:', playerPath + ' ' + args.join(' '))
 
-  proc = cp.spawn(path, args, {stdio: 'ignore'})
+  if (path.extname(playerPath) === '.app') {
+    // Mac: Use executable in packaged .app bundle
+    playerPath += '/Contents/MacOS/' + path.basename(playerPath, '.app')
+  }
+
+  proc = cp.spawn(playerPath, args, {stdio: 'ignore'})
 
   // If it works, close the modal after a second
   const closeModalTimeout = setTimeout(() =>

src/renderer/components/unsupported-media-modal.js

@@ -1,6 +1,5 @@
 const React = require('react')
 const electron = require('electron')
-const path = require('path')
 
 const ModalOKCancel = require('./modal-ok-cancel')
 const {dispatcher} = require('../lib/dispatcher')
@@ -12,15 +11,11 @@ module.exports = class UnsupportedMediaModal extends React.Component {
     const message = (err && err.getMessage)
       ? err.getMessage()
       : err
-    const playerPath = state.saved.prefs.externalPlayerPath
-    const playerName = playerPath
-      ? path.basename(playerPath).split('.')[0]
-      : 'VLC'
     const onAction = state.modal.externalPlayerInstalled
       ? dispatcher('openExternalPlayer')
       : () => this.onInstall()
     const actionText = state.modal.externalPlayerInstalled
-      ? 'PLAY IN ' + playerName.toUpperCase()
+      ? 'PLAY IN ' + state.getExternalPlayerName().toUpperCase()
       : 'INSTALL VLC'
     const errorMessage = state.modal.externalPlayerNotFound
       ? 'Couldn\'t run external player. Please make sure it\'s installed.'

src/renderer/controllers/subtitles-controller.js

@@ -33,11 +33,10 @@ module.exports = class SubtitlesController {
   }
 
   addSubtitles (files, autoSelect) {
-    const state = this.state
     // Subtitles are only supported when playing video files
-    if (state.playing.type !== 'video') return
+    if (this.state.playing.type !== 'video') return
     if (files.length === 0) return
-    const subtitles = state.playing.subtitles
+    const subtitles = this.state.playing.subtitles
 
     // Read the files concurrently, then add all resulting subtitle tracks
     const tasks = files.map((file) => (cb) => loadSubtitle(file, cb))
@@ -47,17 +46,17 @@ module.exports = class SubtitlesController {
       for (let i = 0; i < tracks.length; i++) {
         // No dupes allowed
         const track = tracks[i]
-        let trackIndex = state.playing.subtitles.tracks
-          .findIndex((t) => track.filePath === t.filePath)
+        let trackIndex = subtitles.tracks.findIndex((t) =>
+          track.filePath === t.filePath)
 
         // Add the track
         if (trackIndex === -1) {
-          trackIndex = state.playing.subtitles.tracks.push(track) - 1
+          trackIndex = subtitles.tracks.push(track) - 1
         }
 
         // If we're auto-selecting a track, try to find one in the user's language
         if (autoSelect && (i === 0 || isSystemLanguage(track.language))) {
-          state.playing.subtitles.selectedIndex = trackIndex
+          subtitles.selectedIndex = trackIndex
         }
       }
 

src/renderer/lib/state.js

@@ -68,7 +68,8 @@ function getDefaultState () {
      * Getters, for convenience
      */
     getPlayingTorrentSummary,
-    getPlayingFileSummary
+    getPlayingFileSummary,
+    getExternalPlayerName
   }
 }
 
@@ -168,6 +169,12 @@ function getPlayingFileSummary () {
   return torrentSummary.files[this.playing.fileIndex]
 }
 
+function getExternalPlayerName () {
+  const playerPath = this.saved.prefs.externalPlayerPath
+  if (!playerPath) return 'VLC'
+  return path.basename(playerPath).split('.')[0]
+}
+
 function load (cb) {
   const state = getDefaultState()
 

src/renderer/pages/player-page.js

@@ -2,7 +2,6 @@ const React = require('react')
 const Bitfield = require('bitfield')
 const prettyBytes = require('prettier-bytes')
 const zeroFill = require('zero-fill')
-const path = require('path')
 
 const TorrentSummary = require('../lib/torrent-summary')
 const Playlist = require('../lib/playlist')
@@ -289,11 +288,8 @@ function renderCastScreen (state) {
     castType = 'DLNA'
     isCast = true
   } else if (state.playing.location === 'external') {
-    // TODO: get the player name in a more reliable way
-    const playerPath = state.saved.prefs.externalPlayerPath
-    const playerName = playerPath ? path.basename(playerPath).split('.')[0] : 'VLC'
     castIcon = 'tv'
-    castType = playerName
+    castType = state.getExternalPlayerName()
     isCast = false
   } else if (state.playing.location === 'error') {
     castIcon = 'error_outline'

src/renderer/pages/preferences-page.js

@@ -1,11 +1,11 @@
-const colors = require('material-ui/styles/colors')
 const path = require('path')
 const React = require('react')
 
+const colors = require('material-ui/styles/colors')
 const Checkbox = require('material-ui/Checkbox').default
+const RaisedButton = require('material-ui/RaisedButton').default
 const Heading = require('../components/heading')
 const PathSelector = require('../components/path-selector')
-const RaisedButton = require('material-ui/RaisedButton').default
 
 const {dispatch} = require('../lib/dispatcher')
 
@@ -59,9 +59,8 @@ class PreferencesPage extends React.Component {
   }
 
   externalPlayerPathSelector () {
-    const playerName = path.basename(
-      this.props.state.unsaved.prefs.externalPlayerPath || 'VLC'
-    )
+    const playerPath = this.props.state.unsaved.prefs.externalPlayerPath
+    const playerName = this.props.state.getExternalPlayerName()
 
     const description = this.props.state.unsaved.prefs.openExternalPlayer
       ? `Torrent media files will always play in ${playerName}.`
@@ -79,16 +78,12 @@ class PreferencesPage extends React.Component {
           displayValue={playerName}
           onChange={this.handleExternalPlayerPathChange}
           title='External player'
-          value={this.props.state.unsaved.prefs.externalPlayerPath} />
+          value={playerPath ? path.dirname(playerPath) : null} />
       </Preference>
     )
   }
 
   handleExternalPlayerPathChange (filePath) {
-    if (path.extname(filePath) === '.app') {
-      // Mac: Use executable in packaged .app bundle
-      filePath += '/Contents/MacOS/' + path.basename(filePath, '.app')
-    }
     dispatch('updatePreferences', 'externalPlayerPath', filePath)
   }