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

Made BlocksProjectiles upgradable #9270

Merged
merged 2 commits into from Sep 30, 2015

Conversation

Projects
None yet
7 participants
@reaperrr
Contributor

reaperrr commented Sep 5, 2015

Needed for TS firestorm defense and laser fences.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 5, 2015

Contributor

Updated.

Contributor

reaperrr commented Sep 5, 2015

Updated.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Sep 20, 2015

Member

Needs a rebase, sorry.

Member

pchote commented Sep 20, 2015

Needs a rebase, sorry.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 20, 2015

Contributor

Rebased.

@atlimit8 Is there or will there be something to replace

.Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(b => !b.IsTraitDisabled))

with an Info equivalent?

Contributor

reaperrr commented Sep 20, 2015

Rebased.

@atlimit8 Is there or will there be something to replace

.Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(b => !b.IsTraitDisabled))

with an Info equivalent?

@atlimit8

This comment has been minimized.

Show comment
Hide comment
@atlimit8

atlimit8 Sep 20, 2015

Member

If you mean check if the actor type has a BlocksProjectiles trait that is enabled by default, then yes; however, that (being enabled/disabled) is part of the traits state (may change).

In an upcoming PR I plan on allowing something like:

.WhereType(projectileBlockerTypes).Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(Exts.IsTraitEnabled))

where projectileBlockerTypes is a cached HashSet<uint> of ActorTypeIDs :

projectileBlockerTypes = self.World.Map.Rules.ActorsWhereTraitInfo<BlocksProjectilesInfo>();

That would allow for filtering out all non-blockers with an amortized cost of O(N) where N is the number of actors so the cost (O(N * log T) where N is the number of possible blockers and T is the number of trait pairs in the dictionary) of the

.Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(Exts.IsTraitEnabled))

will only be applied for actors that might block (usually 0, otherwise generally 1).

Basically, that should change the cost from O(N * log T) to an amortized O(N + B * log T) where N is the number of actors, B is the number of possible blockers and T is the number of trait pairs in the dictionary.

I am working on a branch that will add WhereOwner* filters to replace common delegates and then basing the aforementioned branch on that using comment feedback to iron out naming style.

Member

atlimit8 commented Sep 20, 2015

If you mean check if the actor type has a BlocksProjectiles trait that is enabled by default, then yes; however, that (being enabled/disabled) is part of the traits state (may change).

In an upcoming PR I plan on allowing something like:

.WhereType(projectileBlockerTypes).Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(Exts.IsTraitEnabled))

where projectileBlockerTypes is a cached HashSet<uint> of ActorTypeIDs :

projectileBlockerTypes = self.World.Map.Rules.ActorsWhereTraitInfo<BlocksProjectilesInfo>();

That would allow for filtering out all non-blockers with an amortized cost of O(N) where N is the number of actors so the cost (O(N * log T) where N is the number of possible blockers and T is the number of trait pairs in the dictionary) of the

.Any(a => a.TraitsImplementing<BlocksProjectiles>().Any(Exts.IsTraitEnabled))

will only be applied for actors that might block (usually 0, otherwise generally 1).

Basically, that should change the cost from O(N * log T) to an amortized O(N + B * log T) where N is the number of actors, B is the number of possible blockers and T is the number of trait pairs in the dictionary.

I am working on a branch that will add WhereOwner* filters to replace common delegates and then basing the aforementioned branch on that using comment feedback to iron out naming style.

@atlimit8

This comment has been minimized.

Show comment
Hide comment
@atlimit8

atlimit8 Sep 20, 2015

Member

In case you missed it in my previous post:
You should replace b => !b.IsTraitDisabled with Exts.IsTraitEnabled

Member

atlimit8 commented Sep 20, 2015

In case you missed it in my previous post:
You should replace b => !b.IsTraitDisabled with Exts.IsTraitEnabled

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 22, 2015

Contributor

You should replace b => !b.IsTraitDisabled with Exts.IsTraitEnabled

Done.

Contributor

reaperrr commented Sep 22, 2015

You should replace b => !b.IsTraitDisabled with Exts.IsTraitEnabled

Done.

@RoosterDragon

This comment has been minimized.

Show comment
Hide comment
@RoosterDragon

RoosterDragon Sep 26, 2015

Member

Is the duplicated BlockedByActor method something we can move into one place, maybe on BlocksProjectiles?

Member

RoosterDragon commented Sep 26, 2015

Is the duplicated BlockedByActor method something we can move into one place, maybe on BlocksProjectiles?

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Sep 28, 2015

Member

Looks good to me. 👍

Member

abcdefg30 commented Sep 28, 2015

Looks good to me. 👍

@penev92

This comment has been minimized.

Show comment
Hide comment
@penev92

penev92 Sep 30, 2015

Member

Maybe have the if (both of them) as

if (ticks++ >= length || (info.Blockable && BlocksProjectiles.AnyBlockersAt(world, pos)))

and move BlockedByActor() to BlocksProjectiles as
public static bool AnyBlockersAt(World world, WPos pos) ?

Member

penev92 commented Sep 30, 2015

Maybe have the if (both of them) as

if (ticks++ >= length || (info.Blockable && BlocksProjectiles.AnyBlockersAt(world, pos)))

and move BlockedByActor() to BlocksProjectiles as
public static bool AnyBlockersAt(World world, WPos pos) ?

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 30, 2015

Contributor

Rebased and updated.

Contributor

reaperrr commented Sep 30, 2015

Rebased and updated.

@penev92

This comment has been minimized.

Show comment
Hide comment
@penev92

penev92 Sep 30, 2015

Member

Works 👍

Member

penev92 commented Sep 30, 2015

Works 👍

penev92 added a commit that referenced this pull request Sep 30, 2015

Merge pull request #9270 from reaperrr/upgradable-blockproj
Made BlocksProjectiles upgradable

@penev92 penev92 merged commit ce52c62 into OpenRA:bleed Sep 30, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@penev92

This comment has been minimized.

Show comment
Hide comment
@penev92
Member

penev92 commented Sep 30, 2015

@reaperrr reaperrr deleted the reaperrr:upgradable-blockproj branch Oct 21, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment