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

Linux bugfixes #271

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

fpicot
Copy link

@fpicot fpicot commented Feb 3, 2022

Description

Multiple bugfixes for linux :

  • A windows specific keycode was sent for enter. Switched to the original robotjs lib to be able to use abstract key names
  • Keystrokes were sent to quickly, causing some not to be registered. Added 2 75ms delay between keystrokes.
  • Client.txt was not found as the binary running is wine_preloader. Added logic to retrieve the PoE directory from the wine environment. On Linux, this only compatible with steam.

How Has This Been Tested?

  • ran npm run ng:lint
  • ran npm run format
  • ran npm run ng:test
  • added unit tests
  • manually tested

@WhiteFang5 WhiteFang5 self-requested a review February 4, 2022 14:35
this.active = true
this.bounds = window.bounds()

const isLinux = process.platform !== ('win32' || 'darwin')
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest simply using process.platform === 'linux'

const text = this.clipboard.readText()
this.clipboard.writeText(command.text)
await sleep(75)
Copy link
Member

Choose a reason for hiding this comment

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

Why not use delay instead of await and making this async?

Maybe a broader question, looking at the changes, why not increase the keyboard delay (using setKeyboardDelay) itself? As it looks like Linux has trouble with keys being tapped too fast, you could perhaps consider making this platform specific too. This way the behaviour for Windows users, who have no problems with the shorter delay, won't see any behavioral changes. Especially since an additional 150ms will be noticable on Windows.


const windowPath = path.parse((window.path || '').toLowerCase())

if (POE_NAMES.includes(windowPath.base)) {
Copy link
Member

Choose a reason for hiding this comment

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

Since you're using path.parse anyway, the POE_NAMES could probably benefit from an optimization too by removing all .exe names, and using windowPath.name instead of the .base

@ktzee
Copy link

ktzee commented May 24, 2022

Any hope for this getting merged? I'm having a hard time getting this to work on Linux, I'd love to see a bit more Linux support.

@4leite 4leite mentioned this pull request Mar 5, 2024
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants