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

glfw: support multiple types from glfw.Window.hint #32

Merged
merged 2 commits into from
Oct 18, 2021
Merged

glfw: support multiple types from glfw.Window.hint #32

merged 2 commits into from
Oct 18, 2021

Conversation

Avokadoen
Copy link
Contributor

  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

This PR attempts to solve #29

slimsag
slimsag reviewed Oct 18, 2021
View reviewed changes
glfw/src/Window.zig
@@ -1368,12 +1375,51 @@ test "hint" {
try defaultHints();
}

test "hintString" {
test "hint int" {
Copy link
Member

Choose a reason for hiding this comment

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

Very cool PR! Thank you a ton for this!

Could you add a bool hint test? (unless I missed it?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor Author

@Avokadoen Avokadoen Oct 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

One fear I have is that zig is unable to type coerce strings to a zero terminated string using this meaning it is possibly UB, but it seems like this is not the case and zig is able to track the variable until it's called by the c function.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, interesting thought - I think it should be OK but if we find out it's not later on, we could always introduce a temporary buffer / allocation for purposes of passing it to C since this is an infrequent code path.

@Avokadoen
Copy link
Contributor Author

Sorry for the array of force pushes, will be using a draft next time. Didn't expect you to be that fast 😅

slimsag
slimsag approved these changes Oct 18, 2021
View reviewed changes
Copy link
Member

@slimsag slimsag left a comment

Choose a reason for hiding this comment

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

Thanks so much for this!

@Avokadoen
Copy link
Contributor Author

Thanks so much for this!

Thank you for this awesome library :)

@slimsag slimsag merged commit 7849fbf into hexops:main Oct 18, 2021
@slimsag slimsag mentioned this pull request Oct 18, 2021
Closed
@Avokadoen Avokadoen mentioned this pull request Oct 18, 2021
Closed
@slimsag slimsag added the glfw label Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slimsag slimsag slimsag approved these changes

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@Avokadoen @slimsag