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.

Sign up for GitHub

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

Jump to bottom

Implement Linux support #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement Linux support #216

wants to merge 1 commit into from

Conversation

Jan200101
Copy link
Contributor

@Jan200101 Jan200101 commented Nov 12, 2021 •
edited
Loading

late followup to #55
closes #43

  • Windows specific logic has been gated behind an if case
  • APPDATA has been globalized in many cases to reduce calls to os.getenv as well as to use $HOME/.config on non NT systems
  • a dependency on a posix compatibility layer for winreg has been added for non Windows systems
    • no need to rewrite huge chunks of the code
    • has been written specificially for this purpose

Things to implement before this can be merged:

  • game detection
  • Steam Username detection
    • there seems to be no reliable way to detect the current steam username so we just use the users name so its still unique. Most people won't have more than 1 account either way
  • building on Linux
    • building is a huge mess and its best to not do this on Linux at all since distros all have ship different libraries and deal with them differently

@codecov
Copy link

codecov bot commented Nov 12, 2021 •
edited
Loading

Codecov Report

Merging #216 (5eaa80a) into master (b31010d) will decrease coverage by 0.33%.
The diff coverage is 50.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #216      +/-   ##
==========================================
- Coverage   77.43%   77.10%   -0.34%     
==========================================
  Files          17       17              
  Lines        3173     3197      +24     
==========================================
+ Hits         2457     2465       +8     
- Misses        716      732      +16
Impacted Files Coverage Δ
TF2 Rich Presence/console_log.py 88.05% <ø> (ø)
TF2 Rich Presence/custom.py 0.00% <ø> (ø)
TF2 Rich Presence/game_state.py 80.18% <ø> (ø)
TF2 Rich Presence/gamemodes.py 88.88% <ø> (ø)
TF2 Rich Presence/gui.py 75.68% <0.00%> (-0.45%) ⬇️
TF2 Rich Presence/server.py 76.36% <ø> (-0.91%) ⬇️
TF2 Rich Presence/settings_gui.py 79.87% <ø> (ø)
TF2 Rich Presence/tests.py 95.24% <ø> (ø)
TF2 Rich Presence/configs.py 78.00% <40.00%> (-2.42%) ⬇️
TF2 Rich Presence/logger.py 71.89% <40.00%> (+0.37%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2010dc7...5eaa80a. Read the comment docs.

Jan200101
Jan200101 commented Nov 12, 2021
View reviewed changes
TF2 Rich Presence/build.py Outdated Show resolved Hide resolved
requirements.txt Outdated
vdf==3.4
unixreg; sys_platform != 'win32'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If having another external dependency is undesirable its possible to include the code in this repo

Copy link
Owner

Choose a reason for hiding this comment

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

It's fine if it's not big

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference
the source code of unixreg is available on PyPi and Github

https://pypi.org/project/unixreg/
https://github.com/Jan200101/unixreg

TF2 Rich Presence/utils.py Show resolved Hide resolved
TF2 Rich Presence/settings.py Outdated Show resolved Hide resolved
TF2 Rich Presence/localization.py Show resolved Hide resolved
@Kataiser
Copy link
Owner

Yo this is amazing! I'd love to help see this through. I know absolutely nothing about releasing on Linux so you'd have to help me a bit with that, but I'd maintain a Linux build for sure

@Jan200101 Jan200101 force-pushed the master branch 2 times, most recently from bacf482 to e98af8b Compare November 15, 2021 10:32
@Jan200101
Implement Linux support
5eaa80a 
- disable building on Linux (TODO)
- add winreg compatibility layer to allow reusing existing code without having to write an new system
- replace ctypes usage available python replacement
- gate windows logic behind os.name == "nt" checks (TODO compare how this behaves on cygwin and msys)
- update hl2 linux executable name to use the script instead
- replace hardcoded usage of "*.exe" with indexing of the executables list
@Jan200101
Copy link
Contributor Author

image
its now in a working state again

@Jan200101 Jan200101 marked this pull request as ready for review January 22, 2022 14:45
@Kataiser
Copy link
Owner

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kataiser Kataiser Kataiser left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Any chance of Linux support?
2 participants
@Jan200101 @Kataiser