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

Fix FCOM still providing space while being captured #17364

Merged
merged 1 commit into from Nov 27, 2019

Conversation

@reaperrr
Copy link
Contributor

reaperrr commented Nov 20, 2019

Fixes #17354.

@reaperrr reaperrr mentioned this pull request Nov 20, 2019
12 of 19 tasks complete
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Nov 20, 2019

OpenRA.Utility(1,1): Warning: Actor type fcom grants conditions that are not consumed: being-captured

I'm actually at a loss, no friggin' clue what's missing here.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Nov 21, 2019

The campaign-rules.yaml overrides FCOM, removing the BaseProvider to turn it into a normal campaign structure. You’ll need to remove the granted condition there too.

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Nov 21, 2019

The campaign-rules.yaml overrides FCOM, removing the BaseProvider to turn it into a normal campaign structure. You’ll need to remove the granted condition there too.

...which brings us back to #16278, because

	CaptureManager:
		-BeingCapturedCondition:

	CaptureManager:
		BeingCapturedCondition:

	-CaptureManager:
	CaptureManager:

none of these attempts worked.

I guess we could not remove BaseProvider and change its range to something useless like 0c1 instead, but that's awfully hacky...

@reaperrr reaperrr force-pushed the reaperrr:fix-fcom branch from 74664e5 to bf4ad7a Nov 21, 2019
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Nov 21, 2019

Updated. It seems the Capturable ~disabled approach was taken for the same reason, so until these yaml merge issues are solved, I think this is a tolerable work-around, especially since this should be picked for a potential bugfix release.

@tovl
tovl approved these changes Nov 21, 2019
Copy link
Contributor

tovl left a comment

👍 to this workaround until #16278 gets a proper fix.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Nov 22, 2019

Do we want to add a yaml comment indicating that this is a workaround?

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Nov 22, 2019

There are probably quite a few other work-arounds for the yaml merge bug that don't have a comment either, and I'm not in the mood to look for them and add comments everywhere.
Filed #17375 instead.

@abcdefg30 abcdefg30 merged commit 074ebef into OpenRA:bleed Nov 27, 2019
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 Nov 27, 2019

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