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

Add PlaceAlternateBuilding trait #16510

Merged
merged 2 commits into from Jun 10, 2019

Conversation

@pchote
Copy link
Member

commented May 6, 2019

This PR allows Buildings to define an alternate version that can be selected and placed by holding Shift while placing the actor. This adds another option to the toolbox for the next time discussion about north-south imbalance flares up, and i'm sure modders will be able to find some more creative usecases for it.

This uses separate actors because we can't (yet) modify the building previews or footprint using conditions. We can consider changing this to use conditions in the future once these have been fixed.

I've added this to the TS gates, and added a temporary testcase that flips the TS refinery - demonstrating "silent" alternates that aren't buildable on their own. The testcase isn't useful as a real feature because the x-flip casts the shadow in the wrong direction and doesn't significantly help with the balance issue.

@pchote pchote added this to the Next Release milestone May 6, 2019

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I'm a bit unsure about the holding shift part. Could be more useful in terms of modding if you could switch between 2+ actors by clicking shift. Ctrl (or CTRL+SHIFT) could be used for going thru the list other way around.

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

We already use shift to modify the behaviour for walls, so I figured it made sense to apply that here too.

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Hmm, i wasn't aware of that. But still allowing multipile sounds better than allowing just 2 to me.

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

Shift and Ctrl are modifier keys - it would be bad UX and bad code (they can't be detected via normal button presses, so would need to be bodged by observing modifier state changes from Tick) to implement them for state changes. Using the mousewheel was raised as an option in Discord, but this is also bad UX (scrolling is a continuous action, and should not be used to toggle discrete states) and makes the feature difficult to access for players using trackpads or other input methods without a dedicated scrollwheel.

The only option that IMO would be reasonable to toggle discrete states would be the tab key, at the expense of disabling queue toggling while the OG is active.

In any case, changing this from a modifier-based alternative to toggling between N discrete states would require a complete rewrite of the code here, so if there is a consensus that this is the way forward I will close this and leave the reimplementation to somebody else.

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 17, 2019

This now needs to be rewritten on top of #16553.

@pchote pchote force-pushed the pchote:place-alternate-building branch from d28398d to d725467 May 23, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 23, 2019

Updated to be based on #16553, but the changes to support more than one variant are not yet completed.

@pchote pchote force-pushed the pchote:place-alternate-building branch from d725467 to e4962db May 23, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 23, 2019

Updated to support toggling between an arbitrary number of variants using a mod-definable hotkey.

It doesn't make sense to expose a dedicated hotkey just for rotating gates in TS, so I have removed that ability from the PR. This feature will now only be available for modders instead of being used in the default mods.

@pchote pchote force-pushed the pchote:place-alternate-building branch from e4962db to 794a429 May 31, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

rebased.

@pchote pchote force-pushed the pchote:place-alternate-building branch from 794a429 to 92786fd May 31, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

Updated.

@matjaeck
Copy link
Contributor

left a comment

Works. 👍 after testcase is removed

@pchote pchote force-pushed the pchote:place-alternate-building branch from 92786fd to fc84e98 Jun 10, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

Testcase removed and rebased.

@abcdefg30 abcdefg30 merged commit b59ae47 into OpenRA:bleed Jun 10, 2019

2 checks passed

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

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

{
if (mi.Button == MouseButton.Right)
world.CancelInputMode();
if ((mi.Button == MouseButton.Left && mi.Event == MouseInputEvent.Down) || (mi.Button == MouseButton.Right && mi.Event == MouseInputEvent.Up))

This comment has been minimized.

Copy link
@GraionDilach

GraionDilach Jun 16, 2019

Contributor

The AI cannot click to pass this check, which have led to #16679.

@pchote pchote deleted the pchote:place-alternate-building branch Aug 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.