Merge lp://staging/~widelands-dev/widelands/ctrl-priorities into lp://staging/widelands

Proposed by Notabilis
Status: Merged
Merged at revision: 8535
Proposed branch: lp://staging/~widelands-dev/widelands/ctrl-priorities
Merge into: lp://staging/widelands
Diff against target: 91 lines (+47/-2)
3 files modified
data/txts/tips/general_game.lua (+2/-2)
src/wui/inputqueuedisplay.cc (+43/-0)
src/wui/inputqueuedisplay.h (+2/-0)
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/ctrl-priorities
Reviewer Review Type Date Requested Status
GunChleoc Approve
Review via email: mp+335276@code.staging.launchpad.net

Commit message

Updating all ware priorities of a building when CTRL is pressed while clicking.

Description of the change

For me it is a relatively frequent case that I want to change all ware priorities of a building at once, e.g., a low/high priority construction site. Currently this means clicking on multiple little red/green dots. This branch adds the possibility to hold the CTRL key while pressing one of the priority buttons to set all of them at once.

CTRL since the action more or less similar to the "dismantle the building without asking" function, kind of a strong request of some functionality.

Known bug: When a priority is CTRL-clicked for a ware that already has the priority set, the other wares are not updated. This happens since the Radiogroup does not relay the linked method when the value is already set. Could be changed but I didn't wanted to do so without asking since it will also affect other classes.

Feel free to reject this change. :-)

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 2961. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/317141860.
Appveyor build 2770. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ctrl_priorities-2770.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I think this is a great feature :)

By all means, edit the Radiogroup to your needs. It's not used in many places, so we can test the changes easily.

You should also change the tooltips to make the player aware of the feature ;)

Revision history for this message
Notabilis (notabilis27) wrote :

Thanks for the review! Enjoy the new latest version with:
- Handling of already set priorities. Turned out the radiogroup already supported it but I didn't noticed before.
- Documentation of the new feature in the tips on the loading screens. The Ctrl-click for dismantle and destroy is also documented there, so I think that is the appropriate place.
- No more recursion. Did no harm but I didn't liked it.

Revision history for this message
GunChleoc (gunchleoc) wrote :

LGTM :)

@bunnybot merge

review: Approve
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 2973. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/317638701.
Appveyor build 2782. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ctrl_priorities-2782.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 2973. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/317638701.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Transient failure on Travis

@bunnybot merge force

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: