Toggle cursor #3909

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@fluffyfreak
Contributor

Toggle visibility of the mouse cursor depending on the Pi::doingMouseGrab state.

This means that it is hidden and the position fixed (centered) when the player is flying the ship with the mouse.

@fluffyfreak
Contributor

@ecraven there's an issue with this in that a "debug" window appears when I disable the mouse. Do you know what's going on?

@fluffyfreak fluffyfreak added the WIP label Jan 16, 2017
@fluffyfreak
Contributor

@ecraven ok that was just me being stupid. This is ready for review now.

@impaktor
Member

If you didn't have a shit day today, I'd comment that the last commit aught to have been a git commit --amend as it fixes a bug in this PR (if I read it right).

@fluffyfreak
Contributor

Like this SO answer?

Man that looks horrible.

@impaktor
Member

Well, depends on the question you're asking. For "fixing" this PR to "ideal" then, yes, a rebase would do it, but I can do that at merge time, as soon as @ecraven gives 👍. For how to do an --amend (for future reference) it's far simpler, and very useful (I use it a lot).

Normally:

git add <changed_file>
git commit 
git push

Ops! Now we realize something was wrong in this last commit. Fix the problem, then to update/change the last commit:

git add <changed_file>
git commit --amend
git push -f

Will push the same (but fixed) commit once more.

(Actually, if you want to fix this specific PR yourself, you need only rewind the commit history two steps: git reset --soft HEAD~2 and re-commit the changes (and force push). The --soft keeps the actual changes in the files, only the commit history is reset. If not, I'll fix it at merge time).

I've written down how I do this (and how to rebase) on pioneer wiki: 8 Fixing/Updating your pull request. (should be of interest for @mike-f1 as well)

(Sorry to rain on your parade by nitpicking, but clean commits helps understanding and tracking changes for whoever will work on the code in the future, plus saves us tracking mistakes made in PRs in master.)

@ecraven
Contributor
ecraven commented Jan 18, 2017

lgtm, +1

@impaktor
Member

rebased and merged, thus manual close.

@impaktor impaktor closed this Jan 18, 2017
@fluffyfreak fluffyfreak deleted the fluffyfreak:toggle-cursor branch Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment