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

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

Details

Auditors

wraitii

Committed

Stan	Apr 17 2022, 1:06 PM

Differential Revision

D4599: Allow setting the passability class dynamically.

Parents

rP26800: Adds a fallback texture to skies to avoid a crash if a sky texture was invalid.

Branches

Unknown

Tags

Unknown

Build Status

Buildable 20134
Build 48361: Trigger Windows Autobuild
Build 48360: Post-Commit Build	Jenkins
Build 48359: Post-Commit Build (macOS)	Jenkins

Event Timeline

Stan committed rP26801: Allow setting the passability class dynamically. This is needed to improve….Apr 17 2022, 1:06 PM

Stan added a subscriber: Freagarach.

Harbormaster completed building B20134: rP26801: Allow setting the passability class dynamically. This is needed to improve….Apr 17 2022, 1:41 PM

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

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

In rP26801#57477, @Silier wrote:

What happens if there is computed path and passability gets larger

It seems it requires a proper test.

Freagarach added an edge: D4599: Allow setting the passability class dynamically..Apr 19 2022, 6:05 AM

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.

In rP26801#57477, @Silier wrote:

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.

wraitii mentioned this in D4629: Fix rP26801 - Only allwo changing passability of formation controllers..Apr 30 2022, 2:59 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.