Page MenuHomeWildfire Games
Differential D648

Fix unload-all hotkey for allied buildings
ClosedPublic
Actions

Authored by Imarok on Jun 16 2017, 2:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 13, 5:01 PM
Unknown Object (File)
Wed, Sep 11, 2:58 AM
Unknown Object (File)
Wed, Sep 11, 12:55 AM
Unknown Object (File)
Mon, Sep 9, 10:33 PM
Unknown Object (File)
Mon, Sep 9, 8:15 AM
Unknown Object (File)
Tue, Sep 3, 8:19 PM
Unknown Object (File)
Fri, Aug 30, 3:21 PM
Unknown Object (File)
Thu, Aug 22, 4:54 PM
Subscribers
causative
Vulcan

Details

Reviewers
causative
Commits
rP19878: Fix unload-all hotkey for allied buildings
Summary

Unify unloadAll and unloadAllByOwner in input.js and let this function figure out, which command to send.

Test Plan

press U when selecting an own or allied building with garrisoned units.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Imarok created this revision.Jun 16 2017, 2:11 PM
Vulcan added a subscriber: Vulcan.Jun 16 2017, 3:29 PM
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'getActionInfo'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/unit_actions.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/unit_actions.js
| 676| 676| 			if (preSelectedAction != ACTION_GARRISON)
| 677| 677| 				return false;
| 678| 678| 
| 679|    |-			let actionInfo =  getActionInfo("garrison", target);
|    | 679|+			let actionInfo = getActionInfo("garrison", target);
| 680| 680| 			if (!actionInfo.possible)
| 681| 681| 				return {
| 682| 682| 					"type": "none",

binaries/data/mods/public/gui/session/unit_actions.js
| 574| »   »   »   switch·(tradingDetails.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/unit_actions.js
|1049| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.unload")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1067| »   »   »   »   »   »   colorizeHotkey("%(hotkey)s"·+·"·",·"session.kill")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1106| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.stop")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1125| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.garrison")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1166| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.repair")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1185| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"camera.rallypointfocus")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1220| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.backtowork")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1239| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.guard")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1292| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.patrol")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
|1367| »   »   »   »   "tooltip":·colorizeHotkey("%(hotkey)s"·+·"·",·"session.unload")·+
|    | [NORMAL] ESLintBear (no-useless-concat):
|    | Unexpected string concatenation of literals.

binaries/data/mods/public/gui/session/unit_actions.js
| 598| »   »   »   »   if·(tradingDetails.gain.traderGain·==·0)···//·markets·too·close
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/gui/session/unit_actions.js
|1410| »   »   if·(player·==·"Gaia"·&&·targetState.player·==·0·||
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'target' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
| 271| 271| 	if (!g_DevSettings.controlAll && !allOwnedByPlayer)
| 272| 272| 		return undefined;
| 273| 273| 
| 274|    |-	var target = undefined;
|    | 274|+	var target;
| 275| 275| 	if (!fromMinimap)
| 276| 276| 	{
| 277| 277| 		var ent = Engine.PickEntityAtPoint(x, y);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'actionInfo' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/gui/session/input.js
| 285| 285| 	var actions = Object.keys(unitActions).slice();
| 286| 286| 	actions.sort((a, b) => unitActions[a].specificness - unitActions[b].specificness);
| 287| 287| 
| 288|    |-	var actionInfo = undefined;
|    | 288|+	var actionInfo;
| 289| 289| 	if (preSelectedAction != ACTION_NONE)
| 290| 290| 	{
| 291| 291| 		for (var action of actions)

binaries/data/mods/public/gui/session/input.js
| 267| »   »   var·entState·=·GetEntityState(ent);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'entState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/input.js
| 492| »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 519| »   switch·(inputState)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 523| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 578| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 585| »   »   »   var·maxDragDelta·=·16;
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxDragDelta' is already declared in the upper scope.

binaries/data/mods/public/gui/session/input.js
| 628| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 657| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 726| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 843| »   switch·(inputState)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 846| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 948| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1040| »   »   switch·(ev.type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1109| »   »   »   switch·(ev.hotkey)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
|1526| »   switch·(action)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/input.js
| 232| »   »   var·entState·=·GetExtendedEntityState(entityID);
|    | [NORMAL] JSHintBear:
|    | 'entState' is already defined.

binaries/data/mods/public/gui/session/input.js
| 274| »   var·target·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'target' to 'undefined'.

binaries/data/mods/public/gui/session/input.js
| 288| »   var·actionInfo·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'actionInfo'

http://jw:8080/job/phabricator_lint/202/ for more details.

Vulcan added a comment.Jun 16 2017, 5:06 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1570/ for more details.

Harbormaster completed remote builds in B2308: Diff 2582.Jun 16 2017, 5:06 PM
Imarok edited the test plan for this revision. (Show Details)Jul 6 2017, 7:42 PM
causative requested changes to this revision.Jul 6 2017, 8:33 PM
causative added a subscriber: causative.
causative added inline comments.
binaries/data/mods/public/gui/session/input.js
1753 ↗(On Diff #2582)

going by garrisonHolders[0] doesn't work correctly if they selected a mix of own and ally buildings. Why not just always issue an unload-all-by-owner command, and just remove the "unload-all" command entirely? What is the downside?

This revision now requires changes to proceed.Jul 6 2017, 8:33 PM
causative accepted this revision.Jul 6 2017, 9:12 PM

On second thought it seems you can't select a mix of own and ally buildings. There is also a justification for keeping unload-all-by-owner and unload-all as separate commands because petra uses unload-all-by-owner.

This revision is now accepted and ready to land.Jul 6 2017, 9:12 PM
Closed by commit rP19878: Fix unload-all hotkey for allied buildings (authored by Imarok). · Explain WhyJul 6 2017, 10:51 PM
This revision was automatically updated to reflect the committed changes.
bb mentioned this in D736: Missing check in input.UnloadAll.Jul 10 2017, 11:36 PM

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
gui/
session/
19 lines
2 lines

Diff 2835

View Options

ps/trunk/binaries/data/mods/public/gui/session/input.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/session/unit_actions.js

Loading...
Wildfire Games · Phabricator