add better config detection, be more generous in client detection and…

… fix some minor issues

src/config/config_manager.py

@@ -22,6 +22,24 @@ def _init(self):
         if not Path(self.config_path).is_file():
             raise RuntimeError('Could not create config.yml')
         self._config = yaml.safe_load(open(self.config_path))
+        self._checkConfig()
+
+    def _checkConfig(self) -> bool:
+        updated = False
+        if self._config["data_path"]:
+            data_path = Path(self._config["data_path"])
+            if not data_path.is_dir():
+                self._config["data_path"] = None
+                updated = True
+        if self._config["client_path"]:
+            client_path = Path(self._config["client_path"])
+            if not client_path.is_file():
+                self._config["client_path"] = None
+                updated = True
+
+        if updated:
+            self.writeConfig()
+        return updated
 
     def createConfig(self):
         client_path = StorageFinder.instance().getClientPath()
@@ -53,4 +71,5 @@ def setConfig(cls, setting, value):
         if setting not in cls.instance()._config.keys():
             raise ValueError(f"Unknown setting '{setting}'")
         cls.instance()._config[setting] = value
-        cls.instance().writeConfig()
+        if not cls.instance()._checkConfig():
+            cls.instance().writeConfig()  # make sure the config gets written

src/config/storage_finder.py

@@ -3,6 +3,9 @@
 from abc import ABC, abstractmethod
 from pathlib import Path
 from typing import List
+import logging
+
+logger = logging.getLogger(__name__)
 
 
 class StorageFinderBase(ABC):
@@ -36,70 +39,58 @@ def getDataPath(self):
 
 
 class StorageFinder(StorageFinderBase):
-    client_path = None
-    data_path = None
-
     def _init(self):
+        self.client_path = None
+        self.data_path = None
         self.findData()
 
     def findData(self):
+        linux_paths: List[str] = [
+            "/usr/share/ddnet/data",
+            "/usr/share/games/ddnet/data",
+            "/usr/local/share/ddnet/data",
+            "/usr/local/share/games/ddnet/data",
+            "/usr/pkg/share/ddnet/data",
+            "/usr/pkg/share/games/ddnet/data",
+            "/opt/ddnet/data",
+            os.path.expanduser('~/.steam/steam/steamapps/common/DDraceNetwork/ddnet/data'),
+            os.path.expanduser('~/.local/share/Steam/steamapps/common/DDraceNetwork/ddnet/data'),
+            "/usr/share/teeworlds/data",
+            "/usr/share/games/teeworlds/data",
+            "/usr/local/share/teeworlds/data",
+            "/usr/local/share/games/teeworlds/data",
+            "/usr/pkg/share/teeworlds/data",
+            "/usr/pkg/share/games/teeworlds/data",
+            "/opt/teeworlds/data",
+            os.path.expanduser('~/.steam/steam/steamapps/common/DDraceNetwork/ddnet/data'),
+            os.path.expanduser('~/.local/share/Steam/steamapps/common/Teeworlds/tw/data'),
+        ]
         match sys.platform:
             case 'linux':
-                paths: List[str] = [
-                    "/usr/share/ddnet/data",
-                    "/usr/share/games/ddnet/data",
-                    "/usr/local/share/ddnet/data",
-                    "/usr/local/share/games/ddnet/data",
-                    "/usr/pkg/share/ddnet/data",
-                    "/usr/pkg/share/games/ddnet/data",
-                    "/opt/ddnet/data",
-                    os.path.expanduser('~/.steam/steam/steamapps/common/DDraceNetwork/ddnet/data'),
-                    os.path.expanduser('~/.local/share/Steam/steamapps/common/DDraceNetwork/ddnet/data'),
-                    "/usr/share/teeworlds/data",
-                    "/usr/share/games/teeworlds/data",
-                    "/usr/local/share/teeworlds/data",
-                    "/usr/local/share/games/teeworlds/data",
-                    "/usr/pkg/share/teeworlds/data",
-                    "/usr/pkg/share/games/teeworlds/data",
-                    "/opt/teeworlds/data",
-                    os.path.expanduser('~/.steam/steam/steamapps/common/DDraceNetwork/ddnet/data'),
-                    os.path.expanduser('~/.local/share/Steam/steamapps/common/Teeworlds/tw/data'),
+                self._findLocations(linux_paths, sys.platform)
+            case 'win32' | 'cygwin':
+                paths = [
+                    os.path.expandvars(r'%APPDATA%\DDNet'),
+                    os.path.expandvars(r'%PROGRAMFILES(x86)%\Steam\steamapps\common\DDraceNetwork\ddnet\data'),
+                    os.path.expandvars(r'%APPDATA%\Teeworlds'),
+                    os.path.expandvars(r'%PROGRAMFILES(x86)%\Steam\steamapps\common\Teeworlds\tw\data'),
                 ]
-                for data_path in paths:
-                    if Path(data_path).exists() and Path(data_path).is_dir():
-                        self.data_path = data_path
-                        break
-                return
-            case 'win32':
-                self._findWindows()
-                return
-            case 'cygwin:':
-                self._findWindows()
-                return
-            case 'darwin':
-                raise RuntimeError("MacOS is currently not supported")
-
-    def _findWindows(self):
-        paths = [
-            os.path.expandvars(r'%APPDATA%\DDNet'),
-            os.path.expandvars(r'%PROGRAMFILES(x86)%\Steam\steamapps\common\DDraceNetwork\ddnet\data'),
-            os.path.expandvars(r'%APPDATA%\Teeworlds'),
-            os.path.expandvars(r'%PROGRAMFILES(x86)%\Steam\steamapps\common\Teeworlds\tw\data'),
-        ]
-        for data_path in paths:
-            if Path(data_path).exists() and Path(data_path).is_dir():
-                self.data_path = data_path
-                break
-        return
+                self._findLocations(paths, sys.platform)
+            case _:
+                logger.warning("Trying to detect client and data path on unknown OS")
+                self._findLocations(linux_paths, sys.platform)
+        if not self.data_path:
+            logger.warning("Failed to detect ddnet 'data' directory, please set it manually in the settings")
+        if not self.client_path:
+            logger.warning("Failed to detect ddnet 'client_path' directory, please set it manually in the settings")
 
     def getClientPath(self):
-        if self.data_path:
-            try:
-                for p in Path(self.data_path).parent.glob('ddnet*'):
-                    if p.is_file():
-                        return p
-            except OSError:
-                pass
+        if self.client_path:
+            return Path(self.client_path)
+        else:
+            self._findClient()
+            if self.client_path:  # don't recurse here
+                return Path(self.client_path)
         return None
 
     def getDataPath(self):
@@ -111,3 +102,26 @@ def getDataPath(self):
             except OSError:
                 pass
         return None
+
+    def _findLocations(self, paths: List[str], os_name):
+        for path in paths:
+            data_path = Path(path)
+            if data_path.exists() and data_path.is_dir():
+                if self.data_path is None:
+                    self.data_path = data_path
+                # client may be found in another data directory location
+                client_path = StorageFinder._findClient(data_path, os_name)
+                if client_path:
+                    self.client_path = client_path
+                    break
+
+        return
+
+    @staticmethod
+    def _findClient(data_path, os_name):
+        if data_path:
+            pattern = "ddnet.exe" if os_name in ["win32", "cygwin"] else "ddnet*"
+            for p in Path(data_path).parent.glob(pattern, case_sensitive=False):
+                if p.is_file():
+                    return p
+        return None