Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mobile Blocking Refactor #9105

Merged
merged 4 commits into from Aug 28, 2015
Merged

Conversation

RoosterDragon
Copy link
Member

This is a refactoring in Mobile.cs of the logic used to determine if actors block movement.

This centralises some duplicated logic in CanMoveFreelyInto and into GetAvailableSubCell into a new method and makes some minor changes to squeeze out a bit more performance as the pathfinder calls CanMoveFreelyInto frequently.

This is intended to have no impact on the output of these functions. This means for testing you could record a game on bleed and replaying it with this PR should not desync. That's how I tested it.

…he start.

When transient actors checks are not needed, all control flows in the method return true. Therefore, we can return true directly in this case. Checking this condition is cheaper than checking for a free sub-cell, so this allows us a faster exit when we don't need to check for transient actors.
return false;

// If we cannot crush the other actor in our way, we are blocked.
if (self == null || Crushes == null || Crushes.Length == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

The thing that irks me about this line is that we are checking this multiple times in CanMoveFreelyInto even though it doesn't ever change... I guess it is negligible but it still irks me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Whilst it's a touch irksome, it's certainly not a performance issue compared to grovelling through the various dictionary lookups.

@Mailaender
Copy link
Member

Nudging and crushing seems to work as it did before. ✅

@chrisforbes
Copy link
Member

👍 good to go.

penev92 added a commit that referenced this pull request Aug 28, 2015
@penev92 penev92 merged commit d1a3bf9 into OpenRA:bleed Aug 28, 2015
@penev92
Copy link
Member

penev92 commented Aug 28, 2015

Changelog

@RoosterDragon RoosterDragon deleted the cmfi-refactor-perf branch August 28, 2015 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants