Let splash radius be affected by "ApplyValueModificationsToEntity".
ClosedPublic
Actions

Authored by Freagarach on Oct 13 2019, 9:01 AM.

Details

Reviewers

Silier

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP23378: Let splash radius be affected by modifications and rename GetSplashDamage to…

Summary

Let splash ranges be able to be affected by techs.
Rename "getsplashDamage" to "getSplashData".

Test Plan

Verify that techs are applied by creating a test tech and applying that.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Freagarach created this revision.Oct 13 2019, 9:01 AM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/449/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/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
| 821| 821| 		updateEntityColor(data.showAllStatusBars && (i == player || player == -1) ?
| 822| 822| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer, IID_StatusBars] :
| 823| 823| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer],
| 824|    |-			cmpRangeManager.GetEntitiesByPlayer(i));
|    | 824|+		cmpRangeManager.GetEntitiesByPlayer(i));
| 825| 825| 	}
| 826| 826| 	updateEntityColor([IID_Selectable, IID_StatusBars], data.selected);
| 827| 827| 	Engine.QueryInterface(SYSTEM_ENTITY, IID_TerritoryManager).UpdateColors();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1654|1654| 			{
|1655|1655| 				minDist2 = dist2;
|1656|1656| 				minDistEntitySnapData = {
|1657|    |-						"x": pos.x,
|    |1657|+					"x": pos.x,
|1658|1658| 						"z": pos.z,
|1659|1659| 						"angle": cmpPosition.GetRotation().y,
|1660|1660| 						"ent": ent
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1655|1655| 				minDist2 = dist2;
|1656|1656| 				minDistEntitySnapData = {
|1657|1657| 						"x": pos.x,
|1658|    |-						"z": pos.z,
|    |1658|+					"z": pos.z,
|1659|1659| 						"angle": cmpPosition.GetRotation().y,
|1660|1660| 						"ent": ent
|1661|1661| 				};
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1656|1656| 				minDistEntitySnapData = {
|1657|1657| 						"x": pos.x,
|1658|1658| 						"z": pos.z,
|1659|    |-						"angle": cmpPosition.GetRotation().y,
|    |1659|+					"angle": cmpPosition.GetRotation().y,
|1660|1660| 						"ent": ent
|1661|1661| 				};
|1662|1662| 			}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1657|1657| 						"x": pos.x,
|1658|1658| 						"z": pos.z,
|1659|1659| 						"angle": cmpPosition.GetRotation().y,
|1660|    |-						"ent": ent
|    |1660|+					"ent": ent
|1661|1661| 				};
|1662|1662| 			}
|1663|1663| 		}
Executing section cli...

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

Harbormaster completed remote builds in B9806: Diff 10145.Oct 13 2019, 9:17 AM

Freagarach edited the test plan for this revision. (Show Details)Oct 19 2019, 6:06 PM

Stan added a subscriber: Stan.Dec 16 2019, 9:54 AM

Stan added inline comments.

binaries/data/mods/public/simulation/components/Attack.js
418 ↗	(On Diff #10145)	Isnt it weird for it to return false?

Return undefined instead of false.
Don't check for this.template[type].Splash when launching a projectile since that is checked in the GetSplashData.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/801/display/redirect

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

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/Attack.js
| 420| »   return·{
|    | [NORMAL] ESLintBear (consistent-return):
|    | Function expected no return value.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
| 823| 823| 		updateEntityColor(data.showAllStatusBars && (i == player || player == -1) ?
| 824| 824| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer, IID_StatusBars] :
| 825| 825| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer],
| 826|    |-			cmpRangeManager.GetEntitiesByPlayer(i));
|    | 826|+		cmpRangeManager.GetEntitiesByPlayer(i));
| 827| 827| 	}
| 828| 828| 	updateEntityColor([IID_Selectable, IID_StatusBars], data.selected);
| 829| 829| 	Engine.QueryInterface(SYSTEM_ENTITY, IID_TerritoryManager).UpdateColors();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1656|1656| 			{
|1657|1657| 				minDist2 = dist2;
|1658|1658| 				minDistEntitySnapData = {
|1659|    |-						"x": pos.x,
|    |1659|+					"x": pos.x,
|1660|1660| 						"z": pos.z,
|1661|1661| 						"angle": cmpPosition.GetRotation().y,
|1662|1662| 						"ent": ent
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1657|1657| 				minDist2 = dist2;
|1658|1658| 				minDistEntitySnapData = {
|1659|1659| 						"x": pos.x,
|1660|    |-						"z": pos.z,
|    |1660|+					"z": pos.z,
|1661|1661| 						"angle": cmpPosition.GetRotation().y,
|1662|1662| 						"ent": ent
|1663|1663| 				};
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1658|1658| 				minDistEntitySnapData = {
|1659|1659| 						"x": pos.x,
|1660|1660| 						"z": pos.z,
|1661|    |-						"angle": cmpPosition.GetRotation().y,
|    |1661|+					"angle": cmpPosition.GetRotation().y,
|1662|1662| 						"ent": ent
|1663|1663| 				};
|1664|1664| 			}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1659|1659| 						"x": pos.x,
|1660|1660| 						"z": pos.z,
|1661|1661| 						"angle": cmpPosition.GetRotation().y,
|1662|    |-						"ent": ent
|    |1662|+					"ent": ent
|1663|1663| 				};
|1664|1664| 			}
|1665|1665| 		}
Executing section cli...

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

Harbormaster completed remote builds in B10380: Diff 10655.Dec 19 2019, 9:14 PM

Silier added a reviewer: Silier.Jan 8 2020, 8:39 AM

Patch looks good, it is removing one code duplicity, Damage -> Data is correct since return type is object with more information than damage values.
Idea itself adds option for techs and auras in mods to change radius of splash damage what is generally good.

This revision is now accepted and ready to land.Jan 9 2020, 11:24 PM

It would be good to add splash radius into tooltips, @Freagarach ping me if you want to do it here or in another diff.

It would indeed be good to add that to the tooltips, I have a patch locally waiting for my diff list to shrink xD However, the GUI/tooltips are getting clumsy, certainly when Status Effects are added someday. Hence I think it would be good to do that in a different patch.

Closed by commit rP23378: Let splash radius be affected by modifications and rename GetSplashDamage to… (authored by Silier). · Explain WhyJan 12 2020, 2:37 PM

This revision was automatically updated to reflect the committed changes.

Silier added a commit: rP23378: Let splash radius be affected by modifications and rename GetSplashDamage to….

Thanks for the review and commit @Angen :)