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

Handle Mouse button differentiation, handle button release & only send one event #146

Merged
merged 9 commits into from Mar 7, 2016

Conversation

faide
Copy link
Contributor

@faide faide commented Mar 1, 2016

No description provided.

@faide faide mentioned this pull request Mar 1, 2016
1 task
@paked
Copy link
Member

paked commented Mar 3, 2016

@faide, my laptop just died. I might take a bit to review this.

@EtienneBruines
Copy link
Member

Nice PR 😄. I have reviewed all commits, and commented on the code.

Feel free to ask any questions you may have, and I will do my best to answer them.

@@ -16,6 +16,16 @@ type SpaceComponent struct {
Height float32
}

// Center positions the space component according to its center instead of its
// top-left point (this avoids doing the same math each time in your systems)
func (sc *SpaceComponent) Center(p Point) {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure this is the best way to approach this problem.

For example:

Sprite A:

  • Position: 0, 0
  • System: DoesNotCallCenter
  • Width: 20

Sprite B:

  • Position: 0, 0
  • System: CallsCenter
  • Width: 20

If this code was run, Sprite B would end up at position (10, 10), while Sprite A kept the same position - Even if the system made no other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it the name that you don't like? Maybe we should call it PositionCenter
The whole point of this method is to be able to move an entity by placing "it's center" to a desired point, but the name does not fully reflect that. An since this is just a helper I can place it anywhere in my own code but I felt like it could be interesting to reuse.

@paked
Copy link
Member

paked commented Mar 5, 2016

@faide I've given your code a review and left some small comments. Once we have those resolved, I'll be happy to merge it in.

Thanks for your persistence!

@faide
Copy link
Contributor Author

faide commented Mar 6, 2016

Paked I agree we could move the value affectation into each file (js & glfw) but this would create many duplicates because only a small subset is redefined in glfw... Would this be acceptable?

@paked
Copy link
Member

paked commented Mar 6, 2016

@faide Ah ha. Now I see the problem. OK, I'd leave it as it is then.

Make any final changes, then I'm happy to merge this in.

@faide
Copy link
Contributor Author

faide commented Mar 6, 2016

I think I finished what I could in this PR, let's see what will be the next one :)

paked added a commit that referenced this pull request Mar 7, 2016
Handle Mouse button differentiation, handle button release & only send one event
@paked paked merged commit 2aeba01 into EngoEngine:master Mar 7, 2016
@paked
Copy link
Member

paked commented Mar 7, 2016

@faide Thanks for the PR. It is awesome to see other people contributing!

https://49.media.tumblr.com/ce2992e272e03264d883ffc80f6f9561/tumblr_mowjer5HDD1s9frcro1_500.gif

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

Successfully merging this pull request may close these issues.

None yet

3 participants