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

Expose control over cursor visibility and icon of winit::Window #111

Closed
oguzkocer opened this issue Oct 20, 2019 · 7 comments
Closed

Expose control over cursor visibility and icon of winit::Window #111

oguzkocer opened this issue Oct 20, 2019 · 7 comments
Labels
feature New feature or request
Milestone

Comments

@oguzkocer
Copy link
Contributor

It'd be good if we could get access to set_cursor_visible and set_cursor_icon of winit::Window. This should be fairly simple to do once #62 is merged in.

@hecrj I'd be happy to work on this if you are OK with this addition. I was thinking we could just add these two methods in graphics::Window, which would just call the winit::Window methods. For the CursorIcon enum, I am guessing we'd want to have an enum in coffee that maps one to one with the winit::window::CursorIcon since it doesn't look like winit is exposed to the consumers of this crate at all. If you have other ideas, I am happy to follow them as well, just let me know if you're interested.

For context, the reason I am interested in these methods is to be able to show a different icon in specific cases. Although set_cursor_icon would be enough to show the standard cursor icons, I believe set_cursor_visible is necessary to completely hide it to show a custom icon. I don't know which one I'll go with yet, but I think someone else might also find use for one or the other. Furthermore, a lot of games do hide the mouse icon completely, so the visibility control would be necessary for that as well.

@hecrj hecrj added the feature New feature or request label Oct 20, 2019
@hecrj hecrj added this to the 0.4.0 milestone Oct 20, 2019
@hecrj
Copy link
Owner

hecrj commented Oct 20, 2019

I was thinking we could just add these two methods in graphics::Window, which would just call the winit::Window methods.

An interesting alternative, I think, would be to add a new Game::cursor_icon(&self) -> CursorIcon method and let the game loop call this every frame and handle when to change the cursor, centralizing mutations. This would allow us to control what happens when a UserInterface also needs to change the icon (I think it should have priority).

For the CursorIcon enum, I am guessing we'd want to have an enum in coffee that maps one to one with the winit::window::CursorIcon since it doesn't look like winit is exposed to the consumers of this crate at all.

Yes, that sounds great! The enum could have a Hidden variant or similar. There is already a MouseCursor enum in the ui::core module, but it has a slightly different connotation, so I would wait a bit before we try to unify both (maybe after #88 gets merged).

I'd be happy to work on this if you are OK with this addition.

Go ahead! Let me know if you need anything or have any questions!

@oguzkocer
Copy link
Contributor Author

Thanks a lot for the quick turnaround @hecrj, very much appreciated! 🙇

Game::cursor_icon(&self) -> CursorIcon sounds great to me 👍

The enum could have a Hidden variant or similar. There is already a MouseCursor enum in the ui::core module, but it has a slightly different connotation, so I would wait a bit before we try to unify both (maybe after #88 gets merged).

👍

Go ahead! Let me know if you need anything or have any questions!

I think it might be worth waiting for #62 to be merged before tackling this. I looked into branching from improvement/new-winit-event-loop, but that branch doesn't build for me due to small changes in winit or something, not sure 😬. I also checked if I could just fix the minor build issue, but if I am understanding it correctly you have a patch for winit that you are referring to, so I didn't want to get too deep into it without knowing the context behind it.

On that same subject, excuse me if this issue is the wrong place to ask this, but is there a specific thing holding off the merge of #62? Is there anything I can help with? I am pretty new to Rust, but I can try my best to help get that moving along. Feel free to reply to this in #62 for visibility.

@hecrj
Copy link
Owner

hecrj commented Oct 21, 2019

@oguzkocer You are right, the new winit release has been in alpha for a while and the API is still changing slightly.

On that same subject, excuse me if this issue is the wrong place to ask this, but is there a specific thing holding off the merge of #62? Is there anything I can help with? I am pretty new to Rust, but I can try my best to help get that moving along.

This winit issue is the main blocker: rust-windowing/winit#1082.

Just tonight, I have been able to make macOS work with the revamped redraw request API here: hecrj/winit@1c081d3. However, I still have to test it properly.

If you want to help with iOS and Linux Wayland support, I am sure the winit maintainers (and myself) will really appreciate it!

In any case, I have pushed a commit to #62 which should fix the build issues on macOS for the time being and also fixed the conflicts with master.

@oguzkocer
Copy link
Contributor Author

This winit issue is the main blocker: rust-windowing/winit#1082.

Thanks a lot for the clarification!

If you want to help with iOS and Linux Wayland support, I am sure the winit maintainers (and myself) will really appreciate it!

As much as I'd like to help with that, after checking out the conversation and some of the PRs around it, it looks like it could be a bit more than I could handle at this point. I am sorry about that. Hopefully I'll be a bit more comfortable with Rust and the ecosystem to get involved with more crates in the future.

In any case, I have pushed a commit to #62 which should fix the build issues on macOS for the time being and also fixed the conflicts with master.

Thank you for this as well! I intended to start working on the cursor changes tonight, but ended up debugging why the examples weren't working. I left a comment about it on #62, I hope it's helpful 🤞

@hecrj
Copy link
Owner

hecrj commented Oct 21, 2019

As much as I'd like to help with that, after checking out the conversation and some of the PRs around it, it looks like it could be a bit more than I could handle at this point. I am sorry about that.

There's no need to be sorry! 😄 I was completely clueless just some months ago!

I intended to start working on the cursor changes tonight, but ended up debugging why the examples weren't working.

My bad. I need to start testing everything on multiple platforms... 😅

I left a comment about it on #62, I hope it's helpful 🤞

Thank you for the catch!

@oguzkocer
Copy link
Contributor Author

There's no need to be sorry! 😄 I was completely clueless just some months ago!

🙇

My bad. I need to start testing everything on multiple platforms... 😅

No worries, I liked the debugging session. Let me know if you ever need one more person to test on Windows or Mac OS. I am trying my best to spend an hour or two every day with ☕️ so should be able to help out, at least for now.

Thank you for the catch!

Glad it helped!

@hecrj
Copy link
Owner

hecrj commented May 10, 2020

This was implemented in #112 🎉

@hecrj hecrj closed this as completed May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants