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

Overhaul building placement previews #16553

Merged
merged 8 commits into from May 31, 2019

Conversation

@pchote
Copy link
Member

commented May 18, 2019

This PR extracts the building preview rendering from PlaceBuildingOrderGenerator into new interfaces and traits to provide more flexibility for mods.

Fixes #16538
Fixes #13632
Closes #9579
Supersedes #16543

This also implements some polish that I've been wanted for a long time, but never got around to doing:

  • Building previews are now rendered with with translucency above the clear footprint (blocked tiles are still shown on top).
  • TS plugs now display a placement preview.
  • Enabled the subtler line-build footprint idea from TS into RA, D2k, TD.

I already gave this a run past the discord channel, and got positive feedback after some tweaks to the preview translucency.

We can't automatically add the *PlaceBuildingPreview traits using an update rule, and the game doesn't crash if they are missing. This is therefore best mentioned in the update notes rather than adding a dummy message to the update rule that won't be relevant to maps etc.

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


public class ActorPreviewPlaceBuildingPreview { }

class ActorPreviewPlaceBuildingPreviewPreview : FootprintPlaceBuildingPreviewPreview

This comment has been minimized.

Copy link
@pchote

pchote May 18, 2019

Author Member

I am aware of how ridiculous this name is, but it comes logically from the naming scheme - it is the concrete preview object for the PlaceBuildingPreview, that happens to be drawn using the ActorPreview.

I'm open for other suggestions...

@pchote pchote force-pushed the pchote:place-building-preview branch 2 times, most recently from 518f935 to 1454f16 May 18, 2019

@pchote pchote force-pushed the pchote:place-building-preview branch 3 times, most recently from 96c95aa to eb59525 May 18, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

Updated with some additional polish and added placement previews to TS building plugs.

@pchote pchote force-pushed the pchote:place-building-preview branch from eb59525 to a5835db May 18, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

The invalid cells are drawn over the preview by default, so set FootprintOverPreview: None to disable that.

@evgeniysergeev

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

The invalid cells are drawn over the preview by default, so set FootprintOverPreview: None to disable that.

Works.

@evgeniysergeev
Copy link
Contributor

left a comment

Works as expected

@pchote pchote added the PR: Needs +2 label May 18, 2019

@pchote pchote force-pushed the pchote:place-building-preview branch from a5835db to b5e6d86 May 18, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

Fixed

@matjaeck
Copy link
Contributor

left a comment

Could we change the building placement overlay to always use white color as part of this overhaul? It improves the contrast of valid / invalid cells IMO, see https://imgur.com/a/tlJmrgK, and old RA also used white for all terrains. It would also be nice to change the footprint pattern to match the original.

Another point I'd like to suggest is to change that the invalid footprint is drawn above the blocking actor - in old RA the building artwork was never covered by a footprint. The invalid red footprint should if possible be drawn above the building bib, below the building body (the powerplant in the linked image) and above the building preview (the refinery).

Not sure how realistic it is that these requests are addressed here, the PR looks otherwise good to me.

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented May 18, 2019

Can we do something about #9579 too while you are in it?

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

@MustaphaTR do we have artwork for that?

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented May 18, 2019

No, only the PS version had that distinction as i said in the issue. Should be easy to do tho.

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

Could we change the building placement overlay to always use white color as part of this overhaul? It improves the contrast of valid / invalid cells IMO, see https://imgur.com/a/tlJmrgK, and old RA also used white for all terrains. It would also be nice to change the footprint pattern to match the original.

Done.

@pchote pchote force-pushed the pchote:place-building-preview branch from 9ca9456 to c6d85d5 May 18, 2019

@matjaeck
Copy link
Contributor

left a comment

Looks really nice now, noticed just one more thing.

mods/ts/rules/palettes.yaml Outdated Show resolved Hide resolved
@pchote

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

Can we do something about #9579 too while you are in it?

Done, but differently. We now show yellow for any tile that isn't on concrete and would cause the actor to take damage (excluding the bib).

@pchote pchote force-pushed the pchote:place-building-preview branch from f179491 to 429934b May 18, 2019

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented May 18, 2019

Looks good to me.

There is a small issue not sure if should be fixed here, a lot of turreted defences across all mods have preview facings not matching the initial facing. This includes RPG Plug on TS too, tho other plugs seems correct.

@matjaeck
Copy link
Contributor

left a comment

LGTM and fixes #9579 as far as I can tell.

@pchote pchote force-pushed the pchote:place-building-preview branch from 429934b to e65a077 May 19, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 19, 2019

Fixed.

invalidTile = world.Map.Rules.Sequences.GetSequence(info.OverlaySpriteGroup, info.InvalidTileSequence).GetSprite(0);
sourceTile = world.Map.Rules.Sequences.GetSequence(info.OverlaySpriteGroup, info.SourceTileSequence).GetSprite(0);
var sequences = world.Map.Rules.Sequences;
var tilesetValid = info.ValidFootprintSequence + "-" + world.Map.Tileset.ToLowerInvariant();

This comment has been minimized.

Copy link
@teinarss

teinarss May 21, 2019

Contributor

Could be moved to ctor

This comment has been minimized.

Copy link
@pchote

pchote May 21, 2019

Author Member

This is already in the ctor...?

This comment has been minimized.

Copy link
@teinarss

teinarss May 24, 2019

Contributor

Im blind!

@pchote pchote force-pushed the pchote:place-building-preview branch from e65a077 to d14c421 May 21, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 21, 2019

Added a commit to remove Enum.HasFlag and rebased.

@abcdefg30

This comment has been minimized.

Copy link
Member

commented May 24, 2019

Needs a rebase again.

@pchote pchote force-pushed the pchote:place-building-preview branch from d14c421 to e09ac9c May 24, 2019

@pchote pchote removed the PR: Rebase me! label May 24, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

Rebased.

@teinarss
Copy link
Contributor

left a comment

Looks good now

@reaperrr reaperrr merged commit 6723306 into OpenRA:bleed May 31, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30 abcdefg30 referenced this pull request Jun 23, 2019

@pchote pchote deleted the pchote:place-building-preview 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
7 participants
You can’t perform that action at this time.