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

Keys namespace #349

Closed
iamajoe opened this issue Jun 4, 2016 · 18 comments · Fixed by #517
Closed

Keys namespace #349

iamajoe opened this issue Jun 4, 2016 · 18 comments · Fixed by #517

Comments

@iamajoe
Copy link

iamajoe commented Jun 4, 2016

As of this moment, each key is under the same general namespace. So if you want the w key, you would get engo.W.

My proposition:
Namespace so we could get it with engo.Keys.W. This way we wouldn't "dirty" the general namespace and it would be easier with an IDE for example to understand what that engo.W means.

@EtienneBruines
Copy link
Member

SGTM. @paked @otraore ?

@paked
Copy link
Member

paked commented Jun 4, 2016

SGTM.

@paked paked added the proposal label Jun 4, 2016
@otraore
Copy link
Member

otraore commented Jun 4, 2016

SGTM. I'll submit a PR soon if no-one does it by the time I'm free.

@iamajoe
Copy link
Author

iamajoe commented Jun 4, 2016

Cool! I would do it but i'm on the tutorial PR ehehe

@Luukdegram
Copy link

Luukdegram commented Jun 4, 2016

Doing this would require to either move the keys to a new package called keys or create a struct called Keys. Personally, I'm for placing them in a new package. This way we can make them a const so users can't edit them. One thing to note is that it would become keys.W instead of engo.Keys.W. I wouldn't mind making a PR myself unless @otraore has started on it.

@EtienneBruines
Copy link
Member

engo.KeyW is also a possibility. That way we won't have it namespaced, but it'll be clear what it is?

This way we can make them a const so users can't edit them.

I would think this is something we'd want to achieve, yes.

@Luukdegram
Copy link

Luukdegram commented Jun 4, 2016

engo.KeyW is also a possibility. That way we won't have it namespaced, but it'll be clear what it is?

I like this. Also makes it more 'part' of Engo, if that makes any sense. The only thing we would have to keep in mind is that it could become too congested.

@iamajoe
Copy link
Author

iamajoe commented Jun 4, 2016

I still would namespace. When you engo. it should only show the modules, if you have there KeyA, KeyB... it will show a thousand properties. Still... It is better than not having any namespace in my opinion.

@otraore
Copy link
Member

otraore commented Jun 4, 2016

I think like @Sendoushi it shouldn't polute the global engo package but now that I think about it, If you type in engo.Key all the keys should pop up. This is done in the stdlib for example here. Instead of doing something like status.OK they used http.StatusOK. I'm personally fine with all possibilities but just giving what I think about each.

@otraore
Copy link
Member

otraore commented Jun 12, 2016

Bringing this issue up again, thoughts?

@iamajoe
Copy link
Author

iamajoe commented Jun 13, 2016

I'm with my proposal engo.Key.W.

@otraore
Copy link
Member

otraore commented Jun 13, 2016

I'm going to assume you meant to write engo.Keys?

@iamajoe
Copy link
Author

iamajoe commented Jun 13, 2016

Yes. I did.

@ghost
Copy link

ghost commented Jun 13, 2016

As mentioned before, these values need to be const to avoid problems and to allow the compiler to inline them when needed.

Golang does not provide a way to namespace const values in to engo.Keys (as far as I know), so for me the only valid options are Keys.W or engo.KeyW. Both have there ups and downs and I'm neutral towards ether of those, I'm opposed to any solution that makes (or keeps) these values as variables.

@otraore
Copy link
Member

otraore commented Jun 15, 2016

For reasons mentioned by @Nitya-dev I think keys.W or engo.KeyW are the best options here. Thoughts on either one? I'm leaning more towards engo.KeyW.

@EtienneBruines
Copy link
Member

And as an added bonus, they would all be constant where possible. Instead of set within an init method as done in engo_glfw.go - we should have platform-specific input_data.go files IMHO.

@EtienneBruines EtienneBruines added this to the 1.0 milestone Jun 16, 2016
@otraore
Copy link
Member

otraore commented Jul 3, 2016

I created a poll here so we can vote on either one http://goo.gl/DHniyG. Also @Sendoushi if we do use engo.Key.W that means that 'Key' would be a struct with no support for constants.

@otraore otraore self-assigned this Jul 3, 2016
@otraore
Copy link
Member

otraore commented Aug 21, 2016

engo.KeyW it is, anyone is welcome to work on this. I will if it's open by the time I'm free. @Luukdegram you still want to take this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment