HomeWildfire Games

Allow setting the passability class dynamically. This is needed to improve…
AuditedrP26801

Description

Allow setting the passability class dynamically. This is needed to improve formation behavior for the release.

Accepted by: @Freagarach
Refs: Phab:D4563, Phab:D4605
Differential Revision: https://code.wildfiregames.com/Dr4599

Event Timeline

Silier added a subscriber: Silier.Apr 17 2022, 3:16 PM

Question.
What happens if there is computed path and passability gets larger

Stan added a comment.Apr 17 2022, 3:22 PM

I'm not sure, but at least it's the same for CCmpUnitMotion

What happens if there is computed path and passability gets larger

It seems it requires a proper test.

wraitii raised a concern with this commit.Apr 29 2022, 9:59 AM
wraitii added a subscriber: wraitii.

I'm somewhat certain that this leads to OOS on rejoin.

UnitMotion.h caches the unit Clearance on Init(), and changing the passability class may change that. Should be an easy fix.

What happens if there is computed path and passability gets larger

Assuming above OOS fixed, the unit will just run its path until it gets obstructed and recompute for now. We may or may not want to trigger a new path search right away.

This commit now has outstanding concerns.Apr 29 2022, 9:59 AM

Good news: this doesn't OOS because we serialise clearance.
Bad news: this is still broken, because m_Clearance does need to be updated.
Worse news: this can easily break the game, because if the Passability class becomes larger, the unit may suddenly end up in an 'impassable' region, and then can walk all over that impassable region. So depending how it's used, it can make units go into mountains, buildings, etc.

I think the only sane fix at the moment is to only allow this for formation controllers, and even then it's dodgy AF.

Stan added a comment.Apr 30 2022, 3:18 PM

Should I revert this then ? Or make a patch to fix the clearance which UnitMotionFlying doesn't seem to have ?

Can't we send a message when the passability changes to force a path recompute.

In rP26801#57670, @Stan wrote:

Should I revert this then ? Or make a patch to fix the clearance which UnitMotionFlying doesn't seem to have ?

See D4629

Can't we send a message when the passability changes to force a path recompute.

It's not about the path, it's because the unit clips the passability boundaries. We could try to recover from that, but it's non-trivial to write.

Stan requested verification of this commit.Sun, Jun 19, 12:43 AM
This commit now requires verification by auditors.Sun, Jun 19, 12:43 AM
wraitii accepted this commit.Sun, Jun 19, 2:55 PM
All concerns with this commit have now been addressed.Sun, Jun 19, 2:55 PM