Skip to content

Fix movement on ramps when moving to a subcell#21491

Merged
PunkPun merged 1 commit into
OpenRA:bleedfrom
towelcoded:FixRampMoveWithSubcell
Jul 22, 2024
Merged

Fix movement on ramps when moving to a subcell#21491
PunkPun merged 1 commit into
OpenRA:bleedfrom
towelcoded:FixRampMoveWithSubcell

Conversation

@towelcoded
Copy link
Copy Markdown
Contributor

When infantry moves on ramps their position's Z value got reset to the cell center's height despite them moving to a subcell that is off-center. Also when ending their movement there was some adjusting to the X and Y position that left the Z value unchanged. As a result infantry on a ramp would always end up with a non-zero distance above ground.

This fix keeps ground units at the correct height for the ramp they are on even if they don't end their movement at the cell's center.

Fixes #21375

@towelcoded towelcoded force-pushed the FixRampMoveWithSubcell branch from 1df1eed to cb1a4f1 Compare July 20, 2024 15:53
Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm this works. Add the following to the TS rules to allow infantry to be crushed by friendlies.

Crushable:
	WarnProbability: 0
	CrushedByFriendlies: True

On bleed, they will crush on the flat, but crushing on slopes fails. With this PR, the crushing on slopes is resolved.

Comment thread OpenRA.Mods.Common/Traits/Mobile.cs
Comment thread OpenRA.Mods.Common/Activities/Move/Move.cs
@RoosterDragon
Copy link
Copy Markdown
Member

This raises some extra questions for me, such as:

  • Do other callers of mobile.SetCenterPosition need to do a DistanceAboveTerrain adjustment as well?
  • Move notes that "HACK: DistanceAboveTerrain works only with ground layer" and guards it - should other usages of DistanceAboveTerrain also be checking for the ground layer (cell layer == 0) too?

No need to block the PR on answering those - probably something for a separate issue - just mulling really.

Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash the fixup commit.

Fix movement on ramps when moving to subcell
@towelcoded towelcoded force-pushed the FixRampMoveWithSubcell branch from 5399840 to 35127b9 Compare July 21, 2024 13:04
@towelcoded towelcoded closed this Jul 21, 2024
@teinarss teinarss reopened this Jul 21, 2024
@towelcoded
Copy link
Copy Markdown
Contributor Author

Please squash the fixup commit.

done.

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PunkPun PunkPun merged commit 81bcff0 into OpenRA:bleed Jul 22, 2024
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Jul 22, 2024

changelog

@towelcoded towelcoded deleted the FixRampMoveWithSubcell branch July 22, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crushing do not work properly on slopes.

4 participants