Currently when you upgrade a building or a unit there is no animation. This patch fixes that
Details
- Reviewers
Silier - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP23185: Allow artists to add upgrade animations.
Choose the Cart civ, build a long wall upgrade it to a gate, notice it now have scaffoldings and dust. If it had rigged animation it would also play the anim specified in the variant.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 10097 Build 17109: Vulcan Build Jenkins Build 17108: Vulcan Build (Windows) Jenkins
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/450/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/965/display/redirect
Description says, it is setting variant, but code is selecting animation.
So it should call SetVariant instead :)
One needs to keep also in mind what will happen if one cancel upgrade.
Reset the upgrade animation when the upgrade is cancelled.
As for the other point cmpVisual.SelectAnimation() calls cmpVisual.SetVariant() so no need for the later as we also want to play specific anims.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/615/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Upgrade.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Upgrade.js | 268| 268| this.upgrading = false; | 269| 269| this.CancelTimer(); | 270| 270| this.SetElapsedTime(0); | 271| |- | | 271|+ | 272| 272| let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual); | 273| 273| if (!cmpVisual) | 274| 274| return; Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1131/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/616/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /zpool0/trunk/binaries/data/mods/public/simulation/components/Upgrade.js | |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Upgrade.js | 268| 268| this.upgrading = false; | 269| 269| this.CancelTimer(); | 270| 270| this.SetElapsedTime(0); | 271| |- | | 271|+ | 272| 272| let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual); | 273| 273| if (!cmpVisual) | 274| 274| return; Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1132/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/617/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1133/display/redirect
binaries/data/mods/public/simulation/components/Upgrade.js | ||
---|---|---|
270 | choice && this.template[choice].Variant |
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1134/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/618/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/619/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/620/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1135/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1136/display/redirect
Missing animations:
brit_sentry_tower
gaul_sentry_tower
cart_s_wall_long has size of wooden palisade, so should not have scaf
rome_siege_wall_long errors out when trying to place
Add scaffolds for the two missing towers, fix wall. tweak the tower so roof doesn't get overriden.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/622/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1138/display/redirect
The feature looks good and it is nice visual update. Before accepting it I found two bigger problems and some smaller.
So this needs to be fixed:
Mace sentry and defence towers loose their windows, doors and other important props while in upgrade animations.
Rome defence tower is loosing also some wooden props which changes its overall look significantly.
Following is not required to be fixed but it would be nice to have it:
name of animation should start with lowercase letter to keep it consistant with others, mainly in atlas dropdown.
some structures have ground props like bushes, which should not disappear while in upgrade animation.
also some towers have visual prop of the path around which is too missing while in upgrade
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/629/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1145/display/redirect
Notes:
ptolemies/defense_tower: pls add <prop actor="props/structures/decals/spart_1x1.xml" attachpoint="root"/>
romans/scout_tower pls add <prop actor="props/structures/romans/rome_tower_wood.xml" attachpoint="root"/>
actually roman siege wall is about the size of palisade in height, maybe scaf should go off there too, I think it is a bit off by height and width
I think it would need to go closer a bit too to look good. Now there is a pretty big gap.
Fix ptol and rome towers. Remove scaffolds from rome siege wall. Fix missing props when upgrading. Upgrading → upgrading.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/634/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1150/display/redirect
one more thing aside inline before ok, I just noticed that cart defense tower should not use specific cart scaf because it has its own stairs and they are doubled, looks weird.
binaries/data/mods/public/art/actors/structures/romans/scout_tower.xml | ||
---|---|---|
24 | keep that things here for ivy, because attachpoint="root" is resenting root props |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/636/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1152/display/redirect
Looks good in-game. Great job.
Perhaps extend this to add a progress bar when selected (just below the health bar)? Perhaps use the same gray/white one for unpacking.