Skip to content

Add keyboard shortcuts to LoadAll, UnloadAll - #10

Merged
mcpiroman merged 4 commits into
mcpiroman:masterfrom
rogerbarton:shortcuts
Mar 16, 2020
Merged

Add keyboard shortcuts to LoadAll, UnloadAll#10
mcpiroman merged 4 commits into
mcpiroman:masterfrom
rogerbarton:shortcuts

Conversation

@rogerbarton

Copy link
Copy Markdown
Contributor

Works shortcut manager in Unity 2019.1+
Older versions work with MenuItemAttribute

Works with 2019.1 shortcut manager
Older versions work with MenuItemAttribute
@mcpiroman

Copy link
Copy Markdown
Owner

Well, sure. Is [Shortcut] attribute somehow superior to [MenuItem]? Because if it's the same thing but newer, I think it'd be better to just stick with the letter, not to double things unnecessarily. And as a nit, I'd prefer if these attributes were in the DllManipulatorScript, because is's meant to be the 'front end' for user. They may just simply forward calls to DllManipulator.

@rogerbarton

Copy link
Copy Markdown
Contributor Author

Yes [Shortcut] exists since 2019.1 allows you to use the shortcut manager UI to customize the shortcut etc. These settings are stored between projects (as an EditorPref I believe) so you only set them once.

I will move them to the DllManipulatorScript ...

@mcpiroman

Copy link
Copy Markdown
Owner

Well, since you also use UnityEditor namespace here (which I didn't notice previously, my bad) this should also go somewhere inside Editor folder. I think you can just simply copy this to DllManipulatorEditor file and it'd be fine.

@rogerbarton

Copy link
Copy Markdown
Contributor Author

That's a good point, I've moved it now.

@mcpiroman

Copy link
Copy Markdown
Owner

Oh and you should also add the relevant namespace for [MenuItem] too, because they aren't in UnityEditor.ShortcutManagement

@rogerbarton

Copy link
Copy Markdown
Contributor Author

That's a valid point, however its in UnityEditor. But I realized I forgot the macro in the using part, which I've now added.

@mcpiroman

Copy link
Copy Markdown
Owner

Looks fine, thanks!

@mcpiroman
mcpiroman merged commit bc9f1fa into mcpiroman:master Mar 16, 2020
@rogerbarton

Copy link
Copy Markdown
Contributor Author

Thanks for creating this tool!

@mcpiroman

Copy link
Copy Markdown
Owner

Well, I just pulled your branch to see the changes (forgetting that I already merged this PR) , and I see some additional repainting stuff going on. Is that something that could matter?

@rogerbarton

Copy link
Copy Markdown
Contributor Author

That commit "Repaint editor GUIs on shortcut" has not been merged yet.

Basically I wanted to repaint when the shortcut is triggered. However, I believe that there is still a better implementation (with actions/events) that catches all the points when a repaint is needed. So its still work in progress.

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.

2 participants