HomeWildfire Games

Move out of world promoted, packed and upgraded entity as they are not…

Description

Move out of world promoted, packed and upgraded entity as they are not destroyed immediately. So they don't interact with the 'physical' world. Refs #4595.
Differential Revision: https://code.wildfiregames.com/D590
Reviewed By: wraitii

Details

Event Timeline

(and fix a space issue noticed by elexis)

bb raised a concern with this commit.Nov 12 2017, 10:58 PM
bb added a subscriber: bb.

This is broken: build a garrison-able upgradable wall and garrison it, then upgrade the wall to a gate: the units won't eject since the garrisonholder is out of world.

There is also a bug when the new ent has a garrisonholder but not enough space, the ents will eject, but stay unmoveable at the garrisonholders foot (probably unrelated since that is fixed by removing the autogarrison call in TransferGarrisonedUnits)

This commit now has outstanding concerns.Nov 12 2017, 10:58 PM

This is broken: build a garrison-able upgradable wall and garrison it, then upgrade the wall to a gate: the units won't eject since the garrisonholder is out of world.

You mean a garrison able upgradable unit, garrison that unit, upgrade it while garrisoned? I'm pretty sure that never worked.

bb added a comment.Nov 13 2017, 9:55 AM

y, thats the bug.
Well I tracked the code, and it gets stuck on the IsInWorld check, so my suspicion is this commit (didn't actually test if it worked before)

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

Also this comment seems like it was tested (bit ambiguous though comment)

231

(this is the call I talked about, don't know where it is good for)

leper added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/helpers/Transform.js
231

Unless my memory is failing me this piece of code is broken in either the way you found or yet another way. IIRC there's a comment in this piece of code in P46.

wraitii added inline comments.Nov 13 2017, 1:26 PM
/ps/trunk/binaries/data/mods/public/simulation/helpers/Transform.js
231

I agree, that's what I recall too.

elexis added a subscriber: elexis.Jan 20 2018, 7:41 PM

refs or fixed by rP20939

As noticed by temple, this can cause an infinite loop segfault: #5079.

(I wonder if the concern shouldn't be removed, as the pointed bug was not really related to that patch)

bb accepted this commit.Apr 16 2019, 11:16 PM

concern fixed by rP20939

All concerns with this commit have now been addressed.Apr 16 2019, 11:16 PM
bb raised a concern with this commit.May 30 2019, 5:36 PM

Causing another bug revealed by rP22313: a unit that tries to flee from a promoted unit, doesn't know anymore where to flee (since the attacker has no position anymore). This is fixable, by first passing on the fleeing call with the old entID and the promoting it. If required we can always listen to OnEntityRename, but when the attack occurs we don't know the new ent ID (we don't even now if it promotes), see the Damage.js CauseDamge.
Probably needs a test too.

This commit now has outstanding concerns.May 30 2019, 5:36 PM
Freagarach added a subscriber: Freagarach.EditedMar 25 2020, 8:28 AM

For the record, not only fleeing is affected, every movement update which bails on msg.likelyFailure.
(rP22526#38783)

This commit no longer requires audit.Jul 27 2020, 10:53 AM