Page MenuHomeWildfire Games
Differential D1120

Cleanup minimap idle worker button
ClosedPublic
Actions

Authored by temple on Dec 6 2017, 4:08 PM.

Details

Reviewers
elexis
Summary

Cleanup minimap idle worker button.

Test Plan

Could go in js instead, but seems pretty nice and compact.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3991
Build 6994: Vulcan BuildJenkins
Build 6993: arc lint + arc unit

Event Timeline

temple created this revision.Dec 6 2017, 4:08 PM
Owners added a subscriber: Restricted Owners Package.Dec 6 2017, 4:09 PM
elexis added a comment.Dec 6 2017, 4:21 PM

Nuke the duplication using let leaveFunc = ...

Vulcan added a subscriber: Vulcan.Dec 6 2017, 4:37 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Harbormaster completed remote builds in B3981: Diff 4604.Dec 6 2017, 4:37 PM
temple updated this revision to Diff 4606.Dec 6 2017, 4:50 PM

Something like this?

Vulcan added a comment.Dec 6 2017, 4:53 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Harbormaster completed remote builds in B3982: Diff 4606.Dec 6 2017, 4:53 PM
elexis accepted this revision.Dec 6 2017, 4:57 PM

No, since that is better than what I imagined.

Just a rename of the function (since that one also works for entering) and grouping the functions by consequence and it should be perfect.
I can do that when committing if you want.

Thanks!

This revision is now accepted and ready to land.Dec 6 2017, 4:57 PM
temple added a comment.Dec 6 2017, 4:59 PM
In D1120#44639, @elexis wrote:

I can do that when committing if you want.

Go ahead.

Thanks!

Thank you!

temple updated this revision to Diff 4610.Dec 6 2017, 5:41 PM

More elexis suggestions.

Vulcan added a comment.Dec 6 2017, 5:43 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Harbormaster completed remote builds in B3984: Diff 4610.Dec 6 2017, 5:43 PM
temple updated this revision to Diff 4615.Dec 6 2017, 8:26 PM

Use style instead.

Vulcan added a comment.Dec 6 2017, 8:28 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Harbormaster completed remote builds in B3988: Diff 4615.Dec 6 2017, 8:28 PM
temple updated this revision to Diff 4618.Dec 6 2017, 9:08 PM
temple retitled this revision from Move minimap idle worker button code from xml to js to Cleanup minimap idle worker button.
temple edited the summary of this revision. (Show Details)
temple edited the test plan for this revision. (Show Details)

Don't use style.

elexis accepted this revision.Dec 6 2017, 9:30 PM

I like how this patch ended up in deleting code to fix a bug!

(When the mouse hovers the button and no worker is idle, but one becomes idle on the next turn, then it had set the non-highlighted sprite despite the highlight. thats fixed now)

Vulcan added a comment.Dec 6 2017, 9:54 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Harbormaster completed remote builds in B3991: Diff 4618.Dec 6 2017, 9:54 PM
temple closed this revision.Dec 6 2017, 10:35 PM
elexis mentioned this in rP20601: Fix idle worker button mess..Dec 8 2017, 12:29 AM

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
gui/
session/
35 lines
11 lines

Diff 4618

View Options

binaries/data/mods/public/gui/session/minimap_panel.xml

Loading...
View Options

binaries/data/mods/public/gui/session/session.js

Loading...
Wildfire Games · Phabricator