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

Add BaseBuilderBotModule and BuildingRepairBotModule #15857

Merged
merged 3 commits into from Dec 19, 2018

Conversation

Projects
None yet
3 participants
@reaperrr
Copy link
Contributor

reaperrr commented Nov 25, 2018

Extracts all logic related to base-building from HackyAI into a BaseBuilderBotModule. BaseBuilder is kept as separate .cs file, but of course adapted.

Additionally, replaced the ShouldRepairBuildings boolean with a separate RepairBuildingsBotModule, as some singleplayer missions may want only that.

Depends on #15786.

@reaperrr reaperrr added this to the Next Release milestone Nov 25, 2018

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch from 87b4265 to 5053c8c Nov 25, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 25, 2018

Ping @IceReaper. Would be great if you could first look at #15786 (unless it got merged in the meantime), since that's a dependency and will make this PR at least a little more readable as well.

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch 6 times, most recently from c2eb952 to e3407d1 Nov 25, 2018

@reaperrr reaperrr changed the title Add BaseBuilderBotModule Add BaseBuilderBotModule and BuildingRepairBotModule Nov 25, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 26, 2018

Oh wow, I broke unit production somehow...

Already debugged locally, will push a fix when I'm done with the other changes.

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch from e3407d1 to 9e95300 Nov 26, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 26, 2018

Updated.

Note: There's a weird bug in the update rule I haven't been able to figure out yet. The BaseBuilderBotModule of the first HackyAI in a yaml (rush in RA, for example) somehow gets the same RequiredCondition setup as the default HarvesterBotModule (fixed that manually for now). It works fine for the other AIs and I can't get my head around when, where and why it does that.

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch from 9e95300 to 1dce770 Nov 26, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 26, 2018

Updated.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Dec 4, 2018

Needs a rebase now too

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch from 1dce770 to 728c939 Dec 8, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Dec 8, 2018

Updated and rebased.

@reaperrr reaperrr removed the PR: Rebase me! label Dec 8, 2018

reaperrr added some commits Nov 5, 2018

Fix bot module update rule setting wrong RequiresCondition
Yaml nodes are reference types, so caching this meant changes would be applied on all of them.
Additionally, only add HarvesterBotModule if at least one AI is actually using it.

@reaperrr reaperrr force-pushed the reaperrr:modAI-BaseManager branch from 728c939 to 83cedf1 Dec 8, 2018

@obrakmann

This comment has been minimized.

Copy link
Contributor

obrakmann commented Dec 18, 2018

Looks ok to me, didn't notice any regressions while observing an all-AI match. 👍

@pchote

pchote approved these changes Dec 19, 2018

Copy link
Member

pchote left a comment

LGTM in the context of moving existing code around to enable future cleanups. There is a obviously a lot I would like to see change in this code, but this isn't the time or place to start.

Lets see what regressions fall out once more players get to test this...

@pchote pchote merged commit 9914848 into OpenRA:bleed Dec 19, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment