Paths

Table of Contentst

Differential D690

When an alertRaiser is captured, its units under alert should not be controllable by the enemy
AbandonedPublic
Actions

Authored by mimo on Jun 28 2017, 9:40 PM.

Details

Reviewers

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Summary

see #4656 comment 3 for a description of the problem: when the cc is captured, units which were on alert can be ungarrisoned by the enemy
the patch contains also a few other fixes (reset the productionQueue is really owned before resetting it and allow a same productionQueue to be on alert by 2 cc)

As said in #4656 the way to deal with units under alert when the cc is captured or destroyed cannot be easily fixed in the current design, so the design of the component will certainly have to be changed once A22 is released

Test Plan

check that this problem is solved

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 2441
Build 4100: Vulcan Build (Windows)	Jenkins
Build 4099: Vulcan Build	Jenkins
Build 4098: arc lint + arc unit

Event Timeline

mimo created this revision.Jun 28 2017, 9:40 PM

mimo edited the summary of this revision. (Show Details)

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/1653/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/AlertRaiser.js
|  88| »   var·level·=·this.GetLevel();
|    | [NORMAL] JSHintBear:
|    | 'level' is already defined.

binaries/data/mods/public/simulation/components/AlertRaiser.js
| 138| »   »   var·cmpUnitAI·=·Engine.QueryInterface(slot.unit,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 242| 242| 		return (cmpTechnologyManager.IsTechnologyResearched(template.top) || cmpTechnologyManager.IsInProgress(template.top)
| 243| 243| 			|| cmpTechnologyManager.IsTechnologyResearched(template.bottom) || cmpTechnologyManager.IsInProgress(template.bottom));
| 244| 244| 	}
| 245|    |-	else
| 246|    |-	{
|    | 245|+	
| 247| 246| 		return (cmpTechnologyManager.IsTechnologyResearched(tech) || cmpTechnologyManager.IsInProgress(tech));
| 248|    |-	}
|    | 247|+	
| 249| 248| };
| 250| 249| 
| 251| 250| /*
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '+'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 301| 301| 				totalCosts[res] = Math.floor(count * costs[res]);
| 302| 302| 			}
| 303| 303| 
| 304|    |-			var population = ApplyValueModificationsToTemplate("Cost/Population",  +template.Cost.Population, cmpPlayer.GetPlayerID(), template);
|    | 304|+			var population = ApplyValueModificationsToTemplate("Cost/Population", +template.Cost.Population, cmpPlayer.GetPlayerID(), template);
| 305| 305| 
| 306| 306| 			// TrySubtractResources should report error to player (they ran out of resources)
| 307| 307| 			if (!cmpPlayer.TrySubtractResources(totalCosts))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'techCostMultiplier'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 351| 351| 			}
| 352| 352| 
| 353| 353| 			let techCostMultiplier = this.GetTechCostMultiplier();
| 354|    |-			let time =  techCostMultiplier.time * template.researchTime * cmpPlayer.GetCheatTimeMultiplier();
|    | 354|+			let time = techCostMultiplier.time * template.researchTime * cmpPlayer.GetCheatTimeMultiplier();
| 355| 355| 
| 356| 356| 			let cost = {};
| 357| 357| 			for (let res in template.cost)
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'cmpAutoGarrison' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 598| 598| 		}
| 599| 599| 	}
| 600| 600| 
| 601|    |-	var cmpAutoGarrison = undefined;
|    | 601|+	var cmpAutoGarrison;
| 602| 602| 	if (cmpRallyPoint)
| 603| 603| 	{
| 604| 604| 		var data = cmpRallyPoint.GetData()[0];
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '+'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 708| 708| 			{
| 709| 709| 				// If something change population cost
| 710| 710| 				var template = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager).GetTemplate(item.unitTemplate);
| 711|    |-				item.population = ApplyValueModificationsToTemplate("Cost/Population",  +template.Cost.Population, item.player, template);
|    | 711|+				item.population = ApplyValueModificationsToTemplate("Cost/Population", +template.Cost.Population, item.player, template);
| 712| 712| 
| 713| 713| 				// Batch's training hasn't started yet.
| 714| 714| 				// Try to reserve the necessary population slots

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 354| »   »   »   let·time·=··techCostMultiplier.time·*·template.researchTime·*·cmpPlayer.GetCheatTimeMultiplier();
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'time' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 191| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 193| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 205| »   for·(var·i·=·0;·i·<·techList.length;·i++)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 207| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 214| »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(tech);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 243| »   »   »   ||·cmpTechnologyManager.IsTechnologyResearched(template.bottom)·||·cmpTechnologyManager.IsInProgress(template.bottom));
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 281| »   »   »   »   if·(requiredXp·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 339| »   »   »   var·template·=·cmpDataTemplateManager.GetTechnologyTemplate(templateName);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 368| »   »   »   if·(this.queue.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 383| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 422| »   for·(var·i·=·0;·i·<·this.queue.length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 579| »   if·(this.entityCache.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 601| »   var·cmpAutoGarrison·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'cmpAutoGarrison' to 'undefined'.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 609| »   for·(var·i·=·0;·i·<·count;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 611| »   »   var·ent·=·this.entityCache[0];
|    | [NORMAL] JSHintBear:
|    | 'ent' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 644| »   »   if·(createdEnts.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 781| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 792| »   »   »   var·cmpTechnologyManager·=·QueryOwnerInterface(this.entity,·IID_TechnologyManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTechnologyManager' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 795| »   »   »   var·template·=·cmpTechnologyManager.GetTechnologyTemplate(item.technologyTemplate);
|    | [NORMAL] JSHintBear:
|    | 'template' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 813| »   if·(this.queue.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

Harbormaster completed remote builds in B2441: Diff 2741.Jun 28 2017, 10:30 PM

• leper added a reviewer: Restricted Owners Package.Jun 29 2017, 8:42 PM

AlertRaiser should concider onEntityRenamed messages but that is out of scope here

binaries/data/mods/public/simulation/components/AlertRaiser.js
192	unneeded braces
binaries/data/mods/public/simulation/components/ProductionQueue.js
78	when storing ent ID one should also consider the onEntityRenamed messages and use them.
83	I guess typo (== ->!=) and the index is used twice so store it.

This revision now requires changes to proceed.Jul 28 2017, 4:50 PM

fatherbushido mentioned this in D403: AlertRaiser Tests and some cleanup.Aug 17 2017, 10:13 AM

temple mentioned this in D681: Town bell - remove alert status on units.Sep 29 2017, 4:01 AM

bb added inline comments.Dec 24 2020, 4:20 PM

binaries/data/mods/public/simulation/components/AlertRaiser.js
185–187	Shoudln't we listen to GlobalOwnerShip changed and update the prodBuildings accordingly. Same counts for walking and Garrisoned units. Since otherwise we could control captured units

Superseeded by D681

Revision Contents
Changeset List

Path

Size

binaries/

data/

mods/

public/

simulation/

components/

AlertRaiser.js

46 lines

ProductionQueue.js

16 lines

Diff 2741

View Options

binaries/data/mods/public/simulation/components/AlertRaiser.js