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 EngineerRepair stances and make the trait more customisable. #16407

Merged
merged 6 commits into from Apr 22, 2019

Conversation

@MustaphaTR
Copy link
Member

commented Apr 13, 2019

Engineer Repair stances didn't work for 2 reasons. In the order generator it was not actually checking info.ValidStances, but directly if the target is Ally or not. 2 the Stance check in the activity was wrong way around.

This PR in addition to that:

  • Adds Types: to EngineerRepair and EngineerRepairable. Both being empty is also a valid match.
  • Made cusors definable in yaml, it was hardcoded to goldwrench and goldwrench-blocked.
  • Made EngineerRepair conditional, tho not EngineerRepairable.
  • Added RepairSounds to EngineerRepair. In RA2 repairing structures played a sound.

TESTCASE is only for stance fix. I didn't set up for anything else.

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:engi-repair branch from cc8d666 to c7fe921 Apr 13, 2019

OpenRA.Mods.Common/Activities/RepairBuilding.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/EngineerRepair.cs Outdated Show resolved Hide resolved

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:engi-repair branch from c7fe921 to f604ccd Apr 20, 2019

@MustaphaTR

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

Updated.

@pchote
Copy link
Member

left a comment

I assume you already know about the conflict between this and capturing, and that it doesn't matter for your usecase here.

Code changes look reasonable and this doesn't appear to regress anything in the default mods, so LGTM.

I'm assuming of course that the testcase will be removed before merging.

@pchote pchote added the PR: Needs +2 label Apr 20, 2019

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:engi-repair branch from f604ccd to fb2a0a7 Apr 22, 2019

@MustaphaTR

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

Updated.

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

👍 after testcase is removed.

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:engi-repair branch from fb2a0a7 to 38a3563 Apr 22, 2019

@MustaphaTR

This comment has been minimized.

Copy link
Member Author

commented Apr 22, 2019

Removed testcase.

@reaperrr reaperrr merged commit 1f730fb into OpenRA:bleed Apr 22, 2019

2 checks passed

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

@MustaphaTR MustaphaTR deleted the MustaphaTR:engi-repair branch May 25, 2019

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