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

Missing windowMode StateVar #198

Open
ekmett opened this issue Jun 11, 2019 · 9 comments
Open

Missing windowMode StateVar #198

ekmett opened this issue Jun 11, 2019 · 9 comments

Comments

@ekmett
Copy link
Contributor

ekmett commented Jun 11, 2019

You have all the machinery to make a StateVar for WindowMode, but don't take the last step.

Right now to get at it you have to grab the whole WindowConfig and peel it out.

windowMode :: SDL.Window -> StateVar WindowMode
windowMode w@(Window rw) = StateVar getWindowMode (setWindowMode w) where
  getWindowMode = fromNumber <$> SDL.Raw.getWindowFlags rw

This could replace setWindowMode. (Deprecate it first?).

@ocharles
Copy link
Member

Yep, probably just missed this in the initial StateVar work

@ocharles
Copy link
Member

windowMode is going to clash with the windowMode field of WindowConfig. Any votes for other names?

@ekmett
Copy link
Contributor Author

ekmett commented Jun 11, 2019

windowMode' ?

@ekmett
Copy link
Contributor Author

ekmett commented Jun 11, 2019

or windowState or something?

@ocharles
Copy link
Member

Possibly currentWindowMode. I don't like randomly sticking a ' for something completely different.

@ekmett
Copy link
Contributor Author

ekmett commented Jun 11, 2019

I like currentWindowMode

@ocharles
Copy link
Member

I'll have a think about this. It is a bit of a shame to have the API inconsistency of windowSize :: Window -> StateVar (V2 CInt) and currentWindowMode :: Window -> StateVar WindowMode. I'm half tempted to break the API - https://codesearch.aelve.com/haskell/search?query=windowMode&filter=import%20SDL&insensitive=off&space=off&precise=on&sources=on shows no uses other than in this library, but I'm not sure exactly how much code that searches.

@ekmett
Copy link
Contributor Author

ekmett commented Jun 11, 2019

You're thinking currentWindowSize, etc?

@ocharles
Copy link
Member

I think I'd prefer to

a. Rename all of WindowConfig's fields to have a windowConfig prefix (instead of window)
b. Move WindowConfig to a separate module, maybe dropping the field prefix entirely and prefer a qualified import.

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

No branches or pull requests

2 participants