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

cleaning up the linux part: #14

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

S-Marais
Copy link

@S-Marais S-Marais commented Sep 7, 2021

_ removing the need to use Xlib
_adressing a few tweaks here and there
_temporary fix for "variable length array used" that caused an error on compile

_ removing the need to use Xlib
_adressing a few tweaks here and there
reverting changes for vla error as it it irrelevant for this pr.
implementing the minimize and raise event and suspending app.
tweaking keycode keysym to use hopefully more accurate keys.
Copy link
Owner

@travisvroman travisvroman left a comment

Choose a reason for hiding this comment

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

Hi! Thank you for submitting this, I appreciate the effort! Apologies it took me so long to get to it.

Unfortunately, these changes violate the pattern I am trying to enforce of keeping all platform-specific code (that is, windows/linux/etc) in it's respective platform c file. The key codes should be generic and engine-specific, and the fact that they align with the Windows platform is not much more than a coincidence. They should be translated in the platform layer from the platform-specific codes to the ones the engine uses, so that all other application code always uses the same keycodes regardless of what platform the engine is being run on.

I would be interested in the other changes though to only use xcb, so if you'd be willing to revisit this and make these changes I'd love to try it out.

You'll also want to update the branch, as I see there are conflicts.

I'll keep a closer eye out for updates to this. Thank you!

@S-Marais
Copy link
Author

S-Marais commented Jun 1, 2022

Sorry been a bit busy. I believe the changes should only take care of the issue this PR primarily aimed at resolving.
I've also removed the additional events as they would only be implemented on Linux.

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.

2 participants