Page MenuHomeWildfire Games

Extend the visible garrisoning schema to allow specific units on specific turrets.
ClosedPublic

Authored by Stan on Sep 20 2019, 1:03 PM.

Details

Reviewers
Freagarach
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#3488
Summary

See #3488

Test Plan

Test that it only change gameplay while activated and does not require anything else.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after computed key 'entity'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 343| 343| 
| 344| 344| 	// Needs to be called before the visible garrison points are cleared.
| 345| 345| 	let visible = {
| 346|    |-		[entity] : this.IsVisiblyGarrisoned(entity)
|    | 346|+		[entity]: this.IsVisiblyGarrisoned(entity)
| 347| 347| 	};
| 348| 348| 
| 349| 349| 	for (let vgp of this.visibleGarrisonPoints)
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1540/display/redirect

Freagarach added inline comments.Mar 8 2020, 12:32 PM
binaries/data/mods/public/simulation/components/GarrisonHolder.js
201

If there is no visibleGarrisonPoint given, it will try to match to "", with will return false always, IIRC.
So one could do an early return I guess?

214

You've got a - between the type and description above.

Stan updated this revision to Diff 11469.Mar 8 2020, 12:49 PM

Fix inlines.

Stan marked 2 inline comments as done.Mar 8 2020, 12:50 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1842/display/redirect

Freagarach accepted this revision.EditedMar 8 2020, 1:26 PM
  • Code looks good, gets unit-tested extensively (segfault unrelated).
  • Addresses the ticket, although there are some comments there that are not implemented, about user control, that can be ticketed and handled independently, IMHO.
  • I added the option to a wall template and the behaviour is as expected.
  • It is a very nice addition, since it allows for visibly garrisoning a catapult on a ship, as suggested by (the tips).
This revision is now accepted and ready to land.Mar 8 2020, 1:26 PM
Stan added a comment.Mar 8 2020, 2:45 PM

@Angen any objections?

Angen added a comment.EditedMar 9 2020, 1:26 PM

nothing breaking i can find just by reading the code

binaries/data/mods/public/simulation/components/GarrisonHolder.js
49

help text should be added and state that if not present, default x+y will be used

226

vgpEntity -> visibleGarrisonPoint

!visibleGarrisonPoint -> visibleGarrisonPoint = undefined this is not needed to do
this.AllowedToVisibleGarrisoning checks for visibleGarrisonPoint to be falsy so its probably better
!!visibleGarrisonPoint && !this.AllowedToVisibleGarrisoning

350

maybe Needs to be set ...

Stan updated this revision to Diff 11473.Mar 9 2020, 8:17 PM
Stan marked 3 inline comments as done.

Fix comments.

Vulcan added a comment.Mar 9 2020, 8:35 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after computed key 'entity'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 343| 343| 
| 344| 344| 	// Needs to be set before the visible garrison points are cleared.
| 345| 345| 	let visible = {
| 346|    |-		[entity] : this.IsVisiblyGarrisoned(entity)
|    | 346|+		[entity]: this.IsVisiblyGarrisoned(entity)
| 347| 347| 	};
| 348| 348| 
| 349| 349| 	for (let vgp of this.visibleGarrisonPoints)
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1845/display/redirect

Angen added inline comments.Mar 9 2020, 8:36 PM
binaries/data/mods/public/simulation/components/GarrisonHolder.js
49

will they ?
you default to "Infantry+Ranged" at L90

Stan updated this revision to Diff 11474.Mar 9 2020, 8:44 PM

Fix lying comment.

Vulcan added a comment.Mar 9 2020, 8:59 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after computed key 'entity'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 343| 343| 
| 344| 344| 	// Needs to be set before the visible garrison points are cleared.
| 345| 345| 	let visible = {
| 346|    |-		[entity] : this.IsVisiblyGarrisoned(entity)
|    | 346|+		[entity]: this.IsVisiblyGarrisoned(entity)
| 347| 347| 	};
| 348| 348| 
| 349| 349| 	for (let vgp of this.visibleGarrisonPoints)
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1846/display/redirect

Stan updated this revision to Diff 11475.Mar 9 2020, 9:01 PM

Fallback to what is allowed to garrison, fix comment, and tests.

elexis added a subscriber: elexis.Mar 9 2020, 9:03 PM
elexis added inline comments.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
90

The componetn should be agnostic of identity classes (Infantry+Ranged), make it a error if the template doesn't specify one and manifest it in the schema

Stan marked an inline comment as done.Mar 9 2020, 9:04 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
90

Agreed. I think it makes more sense to fallback to everything allowed to garrison by default. Hence the last change just right before your comment :)

elexis added a comment.Mar 9 2020, 9:05 PM

Else the classes specified in the List tag will be used.

That works too

Stan marked 2 inline comments as done.Mar 9 2020, 9:06 PM
Vulcan added a comment.Mar 9 2020, 9:10 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after computed key 'entity'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 343| 343| 
| 344| 344| 	// Needs to be set before the visible garrison points are cleared.
| 345| 345| 	let visible = {
| 346|    |-		[entity] : this.IsVisiblyGarrisoned(entity)
|    | 346|+		[entity]: this.IsVisiblyGarrisoned(entity)
| 347| 347| 	};
| 348| 348| 
| 349| 349| 	for (let vgp of this.visibleGarrisonPoints)
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1847/display/redirect

Stan updated this revision to Diff 11476.Mar 9 2020, 9:19 PM

Fix whitespace noticed by linter.

Vulcan added a comment.Mar 9 2020, 9:25 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1848/display/redirect

elexis added a comment.Mar 9 2020, 9:34 PM

Is the fallback redundant and thus useless (same as always allowed if not specified)?

What happens if someone specifies a class in the AllowedClasses that is not part of List, should that description be the following?

If specified, only entities matching these classes and the List classes...

Stan updated this revision to Diff 11477.EditedMar 9 2020, 10:56 PM

One cannot visibly garrison a unit that cannot be garrisoned non visibly. (Added a test for that)

The fallback is needed for things like walls, because it reduces the changes needed, and doesn't require everything to be specified.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1849/display/redirect

One cannot visibly garrison a unit that cannot be garrisoned non visibly
The fallback is needed for things like walls

Why is the check needed for walls if thats the case?

because it reduces the changes needed, and doesn't require everything to be specified.

Is that (only) the difference between || template.List and if (!foo) return true;? Because in that case it would be less code executed and more obvious to the reader to not evaluate a function that will always return true.

Stan updated this revision to Diff 11479.Mar 10 2020, 8:27 AM

Simplify code by returning true when there are no allowed classes specified for a VGP. The check is handled by perfom garrison.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1850/display/redirect

(I didn't check whether the message moving makes sense, or test it in any way, or anything else)
IsVisiblyGarrisoned does loop for each entity checked, perhaps that can be optimized, I didn't check (code only handled for garrisoned entities, so its not as grave as it could be, but there also was some 10s freeze when a ship was destroyed some releases ago, and there could be cases where a player deletes/renames/ownershipchanges 100+ entities with 10+ entities garrisoned each). (Also didn't check whether sending visible entities in the messages instead of having the message handlers call into the function is more performant.)

binaries/data/mods/public/simulation/components/GarrisonHolder.js
7

-inside?

49

From https://code.wildfiregames.com/D2308#111472

should that description be the following?

If specified, only entities matching these classes and the List classes...

(also will be used is ambiguous, used for what?)

"<element name='AllowedClasses' a:help='If specified, only entities matching these classes and the List classes are able to use this visible garrison point.'>" +

192

its not an object if it's falsy

201

It's not a fallback, since template.List is checked also in the case allowedClasses are specified.
Perhaps skip Identity test if allowedClasses is not given, since that will not be a rare condition. (return cmpIdentity && MatchesClassList... to avoid making it more lines of code, though then I guess it doesnt always return a boolean but perhaps undefined, I didnt check.)

226

!!vgpEntity -> vgpEntity

226

!! unneeded

Freagarach resigned from this revision.Mar 10 2020, 6:19 PM
Freagarach removed a reviewer: Freagarach.

So far the value of my acceptance ^^

This revision now requires review to proceed.Mar 10 2020, 6:19 PM

I suppose the value is cumulative

Stan updated this revision to Diff 11480.Mar 10 2020, 7:42 PM
Stan marked 6 inline comments as done.

Fix inlines.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1851/display/redirect

Freagarach added inline comments.Mar 24 2020, 9:12 AM
binaries/data/mods/public/simulation/components/GarrisonHolder.js
738

Whether the entity is visible on the garrison holder. Or something alike, but now the entities don't match ;)

binaries/data/mods/public/simulation/components/tests/test_GarrisonHolder.js
228

-n.

Stan updated this revision to Diff 11531.Mar 24 2020, 9:20 AM

Fix comments

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1888/display/redirect

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1894/display/redirect

Freagarach added inline comments.Apr 9 2020, 4:20 PM
binaries/data/mods/public/simulation/components/GarrisonHolder.js
204–207

return cmpIdentity && ...?

225–227

Perhaps:

let visibleGarrisonPoint;
if (vgpEntity && this.AllowedToVisibleGarrisoning(entity, vgpEntity))
  visibleGarrisonPoint = vgpEntity;

Seems more logical than first setting, then resetting.

Stan updated this revision to Diff 11656.Apr 9 2020, 5:07 PM
Stan marked 4 inline comments as done.

Fix inlines

Vulcan added a comment.Apr 9 2020, 5:15 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1987/display/redirect

Vulcan added a comment.Apr 9 2020, 5:49 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/567/display/redirect

Freagarach accepted this revision.Apr 9 2020, 8:23 PM

Take two ;)

  • Code looks good better, gets unit-tested extensively (MacOS build failure unrelated).
  • Addresses the ticket, although there are some comments there that are not implemented, about user control, that can be ticketed and handled independently, IMHO.
  • I added the option to a wall and tower templates and the behaviour is as expected.
  • It is a very nice addition, since it allows for visibly garrisoning a catapult on a ship, as suggested by the tips.
binaries/data/mods/public/simulation/components/GarrisonHolder.js
192

+.

262

One could make this (whether it was garrisoned visibly) a variable (around L230) and save a function call here, but is not necessary I guess.

This revision is now accepted and ready to land.Apr 9 2020, 8:23 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/573/display/redirect

Stan updated this revision to Diff 11661.Apr 12 2020, 1:10 AM
Stan marked 2 inline comments as done.

Add missing ".". Save a function call

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 138| 138| 
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 141|+		this.entity, range.min, range.max, range.elevationBonus,
| 142| 142| 			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 139| 139| 	var range = cmpAttack.GetRange(attackType);
| 140| 140| 	this.enemyUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 141| 141| 			this.entity, range.min, range.max, range.elevationBonus,
| 142|    |-			enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 142|+		enemies, IID_Resistance, cmpRangeManager.GetEntityFlagMask("normal"));
| 143| 143| 
| 144| 144| 	cmpRangeManager.EnableActiveQuery(this.enemyUnitsQuery);
| 145| 145| };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 167| 167| 
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170|    |-			this.entity, range.min, range.max, range.elevationBonus,
|    | 170|+		this.entity, range.min, range.max, range.elevationBonus,
| 171| 171| 			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/BuildingAI.js
| 168| 168| 	// This query is only interested in Gaia entities that can attack.
| 169| 169| 	this.gaiaUnitsQuery = cmpRangeManager.CreateActiveParabolicQuery(
| 170| 170| 			this.entity, range.min, range.max, range.elevationBonus,
| 171|    |-			[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
|    | 171|+		[0], IID_Attack, cmpRangeManager.GetEntityFlagMask("normal"));
| 172| 172| 
| 173| 173| 	cmpRangeManager.EnableActiveQuery(this.gaiaUnitsQuery);
| 174| 174| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1992/display/redirect

Freagarach accepted this revision.Apr 12 2020, 3:51 PM
Stan closed this revision.May 7 2020, 5:05 PM

Fixed in rP23630