-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
Feat/modelspinnermousebutton #5981
Draft
mwerle
wants to merge
7
commits into
pioneerspacesim:master
Choose a base branch
from
mwerle:feat/modelspinnermousebutton
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b104ca3
Use middle mouse button to spin models
zonkmachine 33027e6
IsMiddleMouseButton -> EmulateMiddleMouseButton
zonkmachine efc26d8
fixup
zonkmachine 180cef0
ui(spin): unify mouse rotation in map views
mwerle a89f280
ui(spin): add same spin handling to ModelSpinner
mwerle 15366b5
ui(spin): fix deselecting objects in SystemView
mwerle 2652656
ui(spin): fix WorldView spinning and weapon discharge
mwerle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am hesitant to accept this. I do not think (philosophically or practically) that it is the role of the
Input::Manager
to define the conditions for rotating the view across the entire application. I seeInput::Manager
as being significantly lower-level and this being the responsibility of a higher layer of the application.Additionally, note that implementing it at this level provides no ability for the user to rebind which modifier key is used when emulating the middle mouse button (perhaps they'd like to use Alt or the meta keys?)
I wish I had an easy solution I could suggest to accomplish this. The mouse rotation code is one of those things that is common to a number of views, but specific enough in scope it's not really a good fit to throw in
Pi
,Input::Manager
, orView
. It's game-specific code, but there's a little bit of overlap with the editor as the editor also uses "game" widgets.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, just seemed like a natural/easy place for this functionality. It can be placed somewhere else instead. I'm very loathe to re-implement it in every place where it's needed though. I very much dislike code duplication and it already chafes me that I have to duplicate between these views and the Ship Spinner..
Why not? I haven't gotten around to adding options for rebinding yet but I wasn't aware of there being a technical issue with adding such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same. I do think it should be centralized somewhere (perhaps a static method on some sort of helper namespace?) but just not in the low-level input subsystem.
The "no ability" referred to the as-implemented state. In terms of potential implementations, it's not so much that there is a technical problem with it, but rather that the API becomes significantly messier as the
Input::Manager
now has to export a specific action binding that all code which wants to use the function needs to ensure is present in the InputFrame stack before calling the method.