-
Notifications
You must be signed in to change notification settings - Fork 51
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
Toggle ACE captive and surrender via context menu #278
Conversation
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.
Looks good, just some cleanup needed. This should also only require ace_captives
- the icons exist in ace_captives
and can copy the strings if needed.
Alrighty, removed ace_zeus dependency and made the other requested changes |
👍Looks good to me. Should merge after #286 I think, will be easier to deal with any conflicts or make small adjustments. |
I did some cleanup for the context menu changes, added an icon to the main node, added the "Captives" string (would cause an RPT error if ACE was not loaded), and moved the |
When merged this pull request will:
I decided to use the strings and icons from ace_captives and ace_zeus since it's currently dependent on the former and I doubt many would remove the latter. Other alternative I guess is copying over the strings and possibly the icons if ACE team is fine with that.