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

Create a trait that grants a condition on actor creation #14216

Merged
merged 1 commit into from Mar 8, 2018

Conversation

Projects
None yet
7 participants
@penev92
Copy link
Member

penev92 commented Oct 19, 2017

This grants the actor a random condition for a list in order to enable having different "variants" of an actor (like the Technicals in Generals).

@penev92 penev92 changed the title Create a trait that crants a condition on actor creation Create a trait that grants a condition on actor creation Oct 19, 2017

@abcdefg30
Copy link
Member

abcdefg30 left a comment

Please change the spaces to tabs.


public void Created(Actor self)
{
if (!info.Conditions.Any())

This comment has been minimized.

@abcdefg30

abcdefg30 Oct 19, 2017

Member

Does this even make sense if we have [FieldLoader.Require] on that field?

This comment has been minimized.

@penev92

penev92 Oct 19, 2017

Author Member

No, I tested that. If you specify in the YAML
Conditions: and leave it blank the Required check passes, but there is still no value.

return;

var condition = info.Conditions.Random(self.World.SharedRandom);
var conditionManager = self.TraitOrDefault<ConditionManager>();

This comment has been minimized.

@abcdefg30

abcdefg30 Oct 19, 2017

Member

Either use .Trait or add a null check for conditionManager.

@penev92 penev92 force-pushed the penev92:randomCondition branch 3 times, most recently from d0276c6 to 3b6c652 Oct 19, 2017

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 19, 2017

I think GrantRandomConditionOnCreation or at least GrantConditionOnCreation sounds better as a name for trait.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Oct 19, 2017

This is absolutely NOT modder friendly however. I'm aware @pchote propagated this for a good while now, but this is just another tunnel vision reimplementation with limited modding options compared to the original, akin to how the TS EMP Cannon turned out.

Sure, conditions might be the future and all that, but due to the sheer amount of visual offsets defined in yaml trait codes (turret offsets, firing offsets), actor variations cannot be done effectively via random conditions - this limits their use and requires really finicky yaml code to reach the level of complexity Generals could do.

Take note that Generals could also spawn entirely different units as variations - a good example is the Rise of the Reds Propaganda Airship's Trollship variant, which won't even get selected with the "select all of this unit's type" button, because it isn't defined as a mere variation.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Oct 19, 2017

Footnote: this was the feature which pushed me to start my soft-fork - as I said back at #12554 (comment) - , because I realized AttacqueSuperior#18 wouldn't be "compliant" and favorable over this solution even though it would actually be able to do everything Generals could and allow YAML deduplication via creating an unit template for inheritance and individual variants would only need to define offsets/variant codes.

Code changed

@penev92 penev92 force-pushed the penev92:randomCondition branch 2 times, most recently from c1ec27a to eee4ce7 Oct 22, 2017

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Oct 22, 2017

OpenRA.Utility(1,1): Error: Missing Type: GrantConditionOnCreateInfo

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Oct 25, 2017

How does a mapper preplace a specific unit variant in this system?

@penev92 penev92 force-pushed the penev92:randomCondition branch from eee4ce7 to 4aab187 Oct 26, 2017

@penev92

This comment has been minimized.

Copy link
Member Author

penev92 commented Oct 26, 2017

@GraionDilach ok, so this approach is somewhat harder to use for really complicated cases where you want really different units (and simpler to use for simple cases). Does that mean it's bad to have, though?

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Oct 26, 2017

My real issue with it is that it works "like in Generals" yet it can't be used to spawn completely different actors or allow mappers to choose a variant statically either - which Generals could do. These are just arbitrary limits introduced for no reason - and actually hurts usability more than a default magic value on a trait.

I don't believe it is simpler to use for simple cases either - it is simpler to use if you don't rely on trait conditions a lot and don't have as complicated gameplay mechanics as Generals had (I might be doing it wrong, but I am already at the level where I'm creating shim conditions to simplify my trait setups, because no way I'd write rank-level || empdisable || weaponjammer || temporal || paradrop for all weapons defined and just shim emp-temporal-weaponjammers-paradrop to a weapondisable onto a template).

Feel free taking that other option suggested on the IRC anyway. That's a clearly working solution to the issue I present. It's working for good months now afterall.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Oct 26, 2017

I'm creating shim conditions to simplify my trait setups, because no way I'd write rank-level || empdisable || weaponjammer || temporal || paradrop for all weapons defined and just shim emp-temporal-weaponjammers-paradrop to a weapondisable onto a template

This was how I intended people to handle complicated situations, so 👍

@Arular101

This comment has been minimized.

Copy link
Contributor

Arular101 commented Jan 13, 2018

Please don't forget to update the copyright notice year.

@reaperrr reaperrr force-pushed the penev92:randomCondition branch from 4aab187 to fa4b400 Mar 8, 2018

@reaperrr reaperrr merged commit b620e81 into OpenRA:bleed Mar 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.