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

Let immovable actors block individual minefield cells. #17101

Merged
merged 1 commit into from Feb 6, 2020

Conversation

@tovl
Copy link
Contributor

tovl commented Sep 15, 2019

As referenced in #17019, this PR makes cells occupied by immovable actors appear red when ordering a minefield and filters them out of the minefield list upon ordering, just like we do with blocking terrain.

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 10, 2019

The changes have the side effect that the footprint of actors below fog of war is leaked to the minefield. Could this be limited to the frozen actor's footprint?

@tovl tovl force-pushed the tovl:mineblocking branch 2 times, most recently from bb4c935 to 1b1b34b Oct 11, 2019
@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Oct 11, 2019

Added a third type of tileoverlay for tiles that are obscured by fog so that no information about those tiles is leaked. I copied the artwork from the d2k no-concrete building overlay.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Oct 13, 2019

I copied the artwork from the d2k no-concrete building overlay.

That won't do here then - D2k cells are 32x32 compared to RA's 24x24. The artwork requires cropping.

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 20, 2019

That won't do here then - D2k cells are 32x32 compared to RA's 24x24. The artwork requires cropping.

This indeed does not look correct:

Bildschirmfoto vom 2019-10-20 15-53-43

@tovl tovl force-pushed the tovl:mineblocking branch from 1b1b34b to 4c71bed Nov 10, 2019
@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Nov 10, 2019

I wasn't planning to segue into making artwork here. So, instead I have now unhardcoded the overlay sequences used by the Minelayer trait and added a line to the RA minelayer yaml to set the overlay sequence used for obscured tiles to be the same as the one used for unobstructed tiles.

This way, if someone does make a third overlay type at some point (there are bound to be other uses), it would just be a matter of deleting this one line in yaml.

@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Nov 21, 2019

Bump to remove the label.

@tovl tovl force-pushed the tovl:mineblocking branch from 706441b to a60bc58 Dec 30, 2019
@tovl tovl added this to the Next+1 milestone Jan 2, 2020
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Jan 11, 2020

This doesn't seem to be working quite right. I issued a minefield order over the shroud that I was pretty sure was going to be blocked by terrain. The minelayer moved to reveal them, but got stuck on the first tile which it wasn't able to enter.

Screenshot 2020-01-11 at 21 34 39

@tovl tovl force-pushed the tovl:mineblocking branch from a60bc58 to 8e0fcb6 Jan 12, 2020
@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Jan 12, 2020

Fixed.

OpenRA.Mods.Cnc/Activities/LayMines.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/Activities/LayMines.cs Outdated Show resolved Hide resolved
@tovl tovl force-pushed the tovl:mineblocking branch from 8e0fcb6 to 3736623 Jan 16, 2020
@tovl

This comment has been minimized.

Copy link
Contributor Author

tovl commented Jan 16, 2020

Fixups applied.

@abcdefg30 abcdefg30 force-pushed the tovl:mineblocking branch from 3736623 to 05d02a4 Feb 6, 2020
@abcdefg30 abcdefg30 force-pushed the tovl:mineblocking branch from 05d02a4 to 45b05f6 Feb 6, 2020
Copy link
Member

abcdefg30 left a comment

Rebased (first force push) and fixed the indentation (second). Lgtm now. We should think of a proper unknown overlay in a future PR.

@abcdefg30 abcdefg30 merged commit c18857f into OpenRA:bleed Feb 6, 2020
2 checks passed
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

abcdefg30 commented Feb 6, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.