Fix SetGarrisoned missed in rP22753.
ClosedPublic
Actions

Authored by Freagarach on Sep 18 2019, 9:11 AM.

Details

Reviewers

Commits

rP22929: Fix SetGarrisoned() call when transforming an entity

Summary

As reported by faction02 in a forum PM conversation https://wildfiregames.com/forum/index.php?/messenger/10014/&tab=comments#comment-34445:

D2026/rP22753 Missed to transfer the garrisoned flag, leading to interesting behaviour (e.g. an unit promoted on a wall being able to jump off in-game (not visually) and perform tasks when not physically present).

Test Plan

Promote a unit atop of a wall.
Task the unit to somewhere off the wall.
1. Verify that the unit will play a walking animation but stays in place, but that the task does get executed (selection state position changes as well). (Unit can be reselected by dragging the selection box around it.)
Apply the patch.
Try above and verify that it is not possible anymore to task the unit.

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.Sep 18 2019, 9:11 AM

Freagarach mentioned this in rP22753: Make Promotion.js use the common Transform helper, add resource gatherer and….Sep 18 2019, 9:14 AM

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/helpers/Transform.js
| 210| »   »   »   »   for·(let·ent·of·cmpNewObstruction.GetEntitiesDeletedUponConstruction())
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.
Executing section cli...

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

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

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

Harbormaster completed remote builds in B9448: Diff 9838.Sep 18 2019, 9:20 AM

elexis edited the summary of this revision. (Show Details)Sep 18 2019, 12:50 PM

Pretty weird bug haha.

This revision is now accepted and ready to land.Sep 18 2019, 8:59 PM

Closed by commit rP22929: Fix SetGarrisoned() call when transforming an entity (authored by wraitii). · Explain WhySep 18 2019, 9:07 PM

This revision was automatically updated to reflect the committed changes.

wraitii added a commit: rP22929: Fix SetGarrisoned() call when transforming an entity.

But is the patch correct and complete too? I remember seeing Autogarrison, refs D885, D1146, D1228, D1403

In D2303#96116, @elexis wrote:

But is the patch correct and complete too? I remember seeing Autogarrison, refs D885, D1146, D1228, D1403

It's definitely correct in that:

it fixes the bug described
some state is not being transferred and we need to transfer that state
it was transferred before in Promotion.js

It's complete insofar as:

that's all Transform.js missed from the UnitAI transfer in Promotion.js

As for other potential cases, this doesn't seem worth checking out to me.
It seems to me this IsGarrionsed flag could/should be refactored, but that'll be work for another day.

Revision Contents
Changeset List

Path

Size

ps/

trunk/

binaries/

data/

mods/

public/

simulation/

helpers/

Transform.js

2 lines

Diff 9845

View Options

ps/trunk/binaries/data/mods/public/simulation/helpers/Transform.js

Fix SetGarrisoned missed in rP22753.ClosedPublicActions