Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: fix.py / linting code files / added Types #48

Merged
merged 5 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions checks.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
""" Run some tests and generate warnings for proton configuration issues
"""

import shutil
import os
import subprocess
from .logger import log


def esync_file_limits():
def esync_file_limits() -> bool:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if we're adding types to non-gamefixes, type annotations should probably be enforced by the linter, Pylint, and caught in the CI system to avoid having to add the missing type in the future. This can be addressed in a separate PR though

See https://pypi.org/project/flake8-annotations/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also enforce types for the gamefixes.
This should be trivial, as most of them only consist of the main() function.

Copy link
Member

@R1kaB3rN R1kaB3rN Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could and it'll be keep things consistent. Though, I don't think it'll add much though so you don't need to.

For the game fixes, what we really want to catch from the linter are the typos, logical errors and some bad practices. There were just a ton of them before I added the linter in this project, and for the subset of fixes that return a non-None type for some reason I don't think it'll affect the behavior of the program.

I won't stop you from doing it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I think it is just easier to maintain one linter configuration than multiple. Alternatively there might be an option to allow functions that return None to have no type. Not sure about that though.

"""
Check esync file limits using /proc/sys/fs/file-max
https://www.reddit.com/r/SteamPlay/comments/9kqisk/tip_for_those_using_proton_no_esync1/
Expand All @@ -19,14 +16,15 @@ def esync_file_limits():
https://github.com/zfigura/wine/blob/esync/README.esync
'''

with open('/proc/sys/fs/file-max') as fsmax:
with open('/proc/sys/fs/file-max', encoding='ascii') as fsmax:
max_files = fsmax.readline()
if int(max_files) < 8192:
log.warn(warning)
return False
return True

def run_checks():

def run_checks() -> None:
""" Run checks to notify of any potential issues
"""

Expand Down
2 changes: 1 addition & 1 deletion debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

os.environ['DEBUG'] = '1'

def show_debug_info():
def show_debug_info() -> None:
""" Show various debug info """

check_args = [
Expand Down
26 changes: 14 additions & 12 deletions download.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,37 @@
HASH_BLOCK_SIZE = 65536


def get_filename(headers):
def get_filename(headers: list) -> str:
""" Retrieve a filename from a request headers via Content-Disposition
"""
content_disp = [x for x in headers if x[0] == 'Content-Disposition'][0][1]
raw_filename = [x for x in content_disp.split(';') if x.startswith('filename=')][0]
return raw_filename.replace('filename=', '').replace('"', '')


def gdrive_download(gdrive_id, path):
def gdrive_download(gdrive_id: str, path: str) -> None:
Copy link
Member

@R1kaB3rN R1kaB3rN Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functionality was inherited from upstream and it's not being used anywhere in this project.

Personally, I haven't verified if it actually works either, and if possible, I'd like to remove this functionality all together. But since users might be using it in their local fixes, I guess we can't really do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have come across a number of functions that I am convinced are not being used. I left them in and updated them anyway, as it wasn't in the scope of this PR.

Some stuff could be broken up, like certain functions of the utils.py.. but then again some users may need to update their local fixes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this specifically, if it can be proved that the google drive functionality does not work, then that would warrant removing it. Of course, it doesn't have to be addressed in this PR.

In a future separate PR, we can definitely evaluate removing some of them too. Or like, wait until the upcoming new year for the cleaning 😄

""" Download a file from gdrive given the fileid and a path to save
"""
url = GDRIVE_URL.format(gdrive_id)
cjar = http.cookiejar.CookieJar()
opener = urllib.request.build_opener(urllib.request.HTTPCookieProcessor(cjar))
urllib.request.install_opener(opener)

req = urllib.request.Request(url)
resp = urllib.request.urlopen(req)
confirm_cookie = [x for x in resp.getheaders() if
(x[0] == 'Set-Cookie'
and x[1].startswith('download_warning'))][0][1]
with urllib.request.urlopen(req, timeout=10) as resp:
confirm_cookie = [x for x in resp.getheaders() if
(x[0] == 'Set-Cookie'
and x[1].startswith('download_warning'))][0][1]
confirm = confirm_cookie.split(';')[0].split('=')[1]
req2 = urllib.request.Request(url + '&confirm={}'.format(confirm))
resp2 = urllib.request.urlopen(req2)
filename = get_filename(resp2.getheaders())
with open(os.path.join(path, filename), 'wb') as save_file:
save_file.write(resp2.read())

req = urllib.request.Request(f'{url}&confirm={confirm}')
with urllib.request.urlopen(req, timeout=10) as resp:
filename = get_filename(resp.getheaders())
with open(os.path.join(path, filename), 'wb') as save_file:
save_file.write(resp.read())


def sha1sum(filename):
def sha1sum(filename: str) -> str:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with this function. It's sorta useless and it makes more sense if the winetricks executable uses it. But unfortunately it's a shell script, and for the same above reasons, we'll probably just have to leave it

""" Computes the sha1sum of the specified file
"""
if not os.path.isfile(filename):
Expand Down
73 changes: 44 additions & 29 deletions engine.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
""" Game engine API
"""

import io
import os
import sys
from .logger import log
Expand All @@ -10,7 +9,7 @@ class Engine():
""" Game engines
"""

def __init__(self):
def __init__(self) -> None:
self.engine_name = None
self.supported = {
'Dunia 2': 'https://pcgamingwiki.com/wiki/Engine:Dunia_2',
Expand Down Expand Up @@ -40,14 +39,14 @@ def __init__(self):
log.info('Engine: ' + self.supported[self.engine_name])


def _add_argument(self, args=''):
def _add_argument(self, args: str = '') -> None:
""" Set command line arguments
"""

sys.argv += args.split(' ')


def _is_unity(self):
def _is_unity(self) -> bool:
""" Detect Unity engine
"""

Expand All @@ -62,7 +61,7 @@ def _is_unity(self):
return False


def _is_dunia2(self):
def _is_dunia2(self) -> bool:
""" Detect Dunia 2 engine (Far Cry >= 3)
"""

Expand All @@ -76,7 +75,8 @@ def _is_dunia2(self):

return False

def _is_rage(self):

def _is_rage(self) -> bool:
""" Detect RAGE engine (GTA IV/V)
"""

Expand All @@ -91,63 +91,66 @@ def _is_rage(self):

return False

def _is_ue3(self):

def _is_ue3(self) -> bool:
""" Detect Unreal Engine 3
"""

return False


def _is_ue4(self):
def _is_ue4(self) -> bool:
""" Detect Unreal Engine 4
"""

return False


def _log(self, ctx, msg, warn=False):
def _log(self, ctx: str, msg: str, warn: bool = False) -> None:
""" Log wrapper
"""

if self.engine_name is None:
log.warn(ctx + ': Engine not defined')
return False
return

elif warn is not False:
log.warn(self.engine_name + ': ' + ctx + ': ' + msg)
else:
log.info(self.engine_name + ': ' + ctx + ': ' + msg)
log_func = log.warn if warn else log.info
log_func(f'{self.engine_name}: {ctx}: {msg}')


def name(self):
def name(self) -> str:
""" Report Engine name
"""
return self.engine_name


def set(self, engine=None):
def set(self, _engine: str = None) -> bool:
""" Force engine
"""

if engine in self.supported:
self.engine_name = engine
if _engine in self.supported:
self.engine_name = _engine
self._log('set', 'forced')
else:
log.warn('Engine not supported (' + engine + ')')
log.warn(f'Engine not supported ({_engine})')
return False
return True


def nosplash(self):
def nosplash(self) -> bool:
""" Disable splash screen
"""

if self.engine_name == 'UE3':
self._add_argument('-nosplash')
self._log('nosplash', 'splash screen disabled' + res)
self._log('nosplash', 'splash screen disabled')
else:
self._log('nosplash', 'not supported', True)
return False
return True


def info(self):
def info(self) -> bool:
""" Show some information about engine
"""

Expand All @@ -156,9 +159,11 @@ def info(self):
self._log('info', 'command line arguments')
else:
self._log('info', 'not supported', True)
return False
return True


def nointro(self):
def nointro(self) -> bool:
""" Skip intro videos
"""

Expand All @@ -170,9 +175,11 @@ def nointro(self):
self._log('nointro', 'intro videos disabled')
else:
self._log('nointro', 'not supported', True)
return False
return True


def launcher(self, show=True):
def launcher(self) -> bool:
""" Force launcher
"""

Expand All @@ -181,8 +188,11 @@ def launcher(self, show=True):
self._log('launcher', 'forced')
else:
self._log('launcher', 'not supported', True)
return False
return True

def windowed(self):

def windowed(self) -> bool:
""" Force windowed mode
"""

Expand All @@ -194,17 +204,20 @@ def windowed(self):
self._log('windowed', 'window')
else:
self._log('windowed', 'not supported', True)
return False
return True


def resolution(self, res=None):
def resolution(self, res: str = None) -> bool:
""" Force screen resolution
"""

if res is not None:
res_wh = res.split('x')
else:
if not isinstance(res, str):
self._log('resolution', 'not provided')
return False

res_wh = res.split('x')

if self.engine_name == 'Unity':
self._add_argument('-screen-width ' + res_wh[0] + ' -screen-height ' + res_wh[1])
self._log('resolution', res)
Expand All @@ -213,6 +226,8 @@ def resolution(self, res=None):
self._log('resolution', res)
else:
self._log('resolution', 'not supported', True)
return False
return True


engine = Engine() #pylint: disable=C0103
Loading