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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetBSD support proposal. #1723

Merged
merged 1 commit into from Dec 7, 2020
Merged

NetBSD support proposal. #1723

merged 1 commit into from Dec 7, 2020

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Dec 5, 2020

Thanks a lot for making a contribution to SFML! 馃檪

Before you create the pull request, we ask you to check the follow boxes. (For small changes not everything needs to ticked, but the more the better!)

  • Has this change been discussed on the forum or in an issue before?
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?
  • If you have additional steps which need to be performed list them as tasks!

Copy link
Member

@eXpl0it3r eXpl0it3r left a comment

Choose a reason for hiding this comment

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

I commented on some small stylistic changes.

As someone with no knowledge or experience with NetBSD, how would I go about testing this?

src/SFML/Window/CMakeLists.txt Outdated Show resolved Hide resolved
src/SFML/Window/NetBSD/JoystickImpl.cpp Outdated Show resolved Hide resolved
src/SFML/Window/NetBSD/JoystickImpl.cpp Outdated Show resolved Hide resolved
src/SFML/Window/NetBSD/JoystickImpl.cpp Outdated Show resolved Hide resolved
src/SFML/Window/NetBSD/JoystickImpl.cpp Outdated Show resolved Hide resolved
cmake/Config.cmake Outdated Show resolved Hide resolved
@Bromeon
Copy link
Member

Bromeon commented Dec 5, 2020

Is there any difference in behavior regarding the different BSD distributions?
If not, we might think about collapsing the config and #define constants.

@devnexen
Copy link
Contributor Author

devnexen commented Dec 5, 2020

Mainly in term of joystick support in this case. Couple of os specifics too.

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Dec 5, 2020
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Dec 5, 2020
@Bromeon
Copy link
Member

Bromeon commented Dec 6, 2020

Basically, your PR is very similar to the existing FreeBSD joystick implementation, in small differences e.g. how to get joystick ID. Meanwhile, the OpenBSD implementation is an empty stub.

But in this case I'm not sure if it makes things more complicated to unify common code rather than duplicating it. The only thing I could imagine is if there is some Window/BSD folder with shared code -- not sure if it's worth it.

By the way, do you happen to know if the same code can be used on OpenBSD?

@devnexen
Copy link
Contributor Author

devnexen commented Dec 6, 2020

By the way, do you happen to know if the same code can be used on OpenBSD?

It might ... maybe the only difference is the ioctl lookup id, just a glance in their code tree tough.

@eXpl0it3r
Copy link
Member

eXpl0it3r commented Dec 6, 2020

I my opinion, code duplication is okay, if it's for the sake of isolating independent contexts/modules/etc.
Maybe there's a way to share more code between the different BSD versions, but I think that should then maybe be done as a separate PR.

@eXpl0it3r eXpl0it3r merged commit 4cbb34d into SFML:master Dec 7, 2020
SFML 2.6.0 automation moved this from Discussion to Done Dec 7, 2020
@eXpl0it3r
Copy link
Member

Thank you for the contribution! 馃檪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants