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

Fixed sonar pulse getting deployed on land #17982

Merged
merged 1 commit into from Aug 11, 2021

Conversation

Mailaender
Copy link
Member

@Mailaender Mailaender commented Apr 25, 2020

Closes #6145.
Closes #17371.

@pchote
Copy link
Member

pchote commented Apr 25, 2020

Unfortunately this leaks state through the shroud: the cursor tells the player whether a cell hidden under the shroud is water or something else. We will also need to limit this to cells that are either currently visible or perhaps visible + fog.

@Mailaender
Copy link
Member Author

Added a shroud check.

@MlemandPurrs
Copy link

does Terrain= only accept a single type or a list of comma seperated terrain ? asking because, see:
#17371

@Mailaender
Copy link
Member Author

A list of comma seperated terrain types.

@MustaphaTR
Copy link
Member

Can we put these checks under base SupportPower instead of SpawnActorPower? i can see Terrain checks being used for other powers too.

@pchote
Copy link
Member

pchote commented Jan 8, 2021

Adding those kind of checks to SupportPower would not be a good idea. Some of our powers (e.g. ProduceActorPower, GrantPrerequisiteChargeDrainPower) do not have targeting cursors, so it does not make sense for them to expose terrain types.

This PR is only applying the checks in the client-side order targeting: this means that players can modify their client to allow them to target the power anywhere on the map without desyncing the game. The support power code does not expose a way for implementations to tell the manager that a targeting request is invalid and should be rejected - this is why the iron curtain and chronosphere misfires occur. We really should fix that before we try to proceed here.

@Orb370
Copy link
Contributor

Orb370 commented Jan 8, 2021

Maybe the cursor shouldn't change when it's on land? I think the name makes it pretty obvious it needs to be used in the water.

Copy link
Contributor

@obrakmann obrakmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised and seems reasonable. 👍

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also perform those checks in Activate before anything is done by the power. That will alleviate the cheating concerns.

@Mailaender
Copy link
Member Author

Added the checks to activation as well.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments then this should be good to go. (Bonus points if you managed to reduce duplication.)

@Mailaender
Copy link
Member Author

Removed the redundancy. I thought there might have been performance implications, but there aren't any.

@abcdefg30 abcdefg30 merged commit b7bba5d into OpenRA:bleed Aug 11, 2021
@abcdefg30
Copy link
Member

Changelog

@Mailaender Mailaender deleted the sonar-water branch August 12, 2021 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpawnActorPower: cannot filter for Terrain Sonar pulse can be used as an overpowered spyplane
7 participants