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 D2k bots wasting cash on building repairs #16149

Merged
merged 1 commit into from Feb 3, 2019

Conversation

Projects
None yet
4 participants
@reaperrr
Copy link
Contributor

reaperrr commented Feb 1, 2019

D2k bots not repairing buildings when damaged due to placement without concrete (damage is dealt by 'neutral') was intentional, and this was actually bleed's default behavior before BuildingRepairBotModule got introduced, too.

Fixes #16063.

@reaperrr reaperrr added this to the Next Release milestone Feb 1, 2019

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Feb 2, 2019

Would be better if this would be tied to damagetypes longterm.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Feb 2, 2019

The proper long term fix will be to make bots place concrete, and then enable repairing if there are situations where it doesn't/can't.

@obrakmann
Copy link
Contributor

obrakmann left a comment

This does fix bots wasting money on repairs. However, with this fix, they will never repair a building that starts with 50% health due to the check against the PreviousDamageState in this line:

if (e.DamageState > DamageState.Light && e.PreviousDamageState <= DamageState.Light && !rb.RepairActive)

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Feb 3, 2019

However, with this fix, they will never repair a building that starts with 50% health due to the check against the PreviousDamageState in this line:

That should be the case before this fix already (the fix just makes a difference on D2k buildings because they receive 50% neutral damage on placement).

This module definitely needs some improvements for Next+1, but for now I'd prefer to not take any chances and just restore the pre-module behavior, as I don't see a simple way to fix all issues this module has without risking regressions.

On that note, I'm wondering whether I should just hardcode that stance check completely for now like HackyAI did, to avoid a situation where we decide to drop/replace the stance field on Next+1 already, resulting in an update rule for something short-lived.
Any opinions on that?

@reaperrr reaperrr force-pushed the reaperrr:fix-d2k-botrep branch from 6616c99 to b100c5a Feb 3, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Feb 3, 2019

Updated:

I'm wondering whether I should just hardcode that stance check completely for now

Done, as discussed on IRC.

Fix D2k bots wasting cash on building repairs
D2k bots not repairing buildings when damaged due to placement
without concrete was intentional, and this was bleed's default behavior
before BuildingRepairBotModule got introduced, too.

@reaperrr reaperrr force-pushed the reaperrr:fix-d2k-botrep branch from b100c5a to 09512f0 Feb 3, 2019

@pchote

pchote approved these changes Feb 3, 2019

Copy link
Member

pchote left a comment

Not tested, but this matches the previous HackyAI implementation.

@pchote pchote added the PR: Needs +2 label Feb 3, 2019

@obrakmann obrakmann merged commit 31f4b0a into OpenRA:bleed Feb 3, 2019

2 checks passed

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

This comment has been minimized.

Copy link
Contributor

obrakmann commented Feb 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment