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 bot module plumbing #15830

Merged
merged 1 commit into from Nov 24, 2018

Conversation

Projects
None yet
4 participants
@reaperrr
Copy link
Contributor

reaperrr commented Nov 18, 2018

Fixes the issues pointed out in #15750 (comment) after it was merged.
While I was at it, also pulled in the update rules merge from #15786 to reduce its diff after rebase.

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

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 18, 2018

I'll make a few more changes as discussed on IRC.

@reaperrr reaperrr force-pushed the reaperrr:fix-bot-plumbing branch 3 times, most recently from c8b1124 to 8ed7e7c Nov 18, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 18, 2018

Updated again.

Reintegrated BotOrderManager back into HackyAI as discussed on IRC.

@reaperrr reaperrr force-pushed the reaperrr:fix-bot-plumbing branch from 8ed7e7c to 4d8380c Nov 19, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 19, 2018

Updated.

@pchote
Copy link
Member

pchote left a comment

A couple of minor comments, otherwise LGTM

Show resolved Hide resolved OpenRA.Mods.Common/UpdateRules/Rules/20180923/ExtractHackyAIModules.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/AI/HackyAI.cs Outdated

@reaperrr reaperrr force-pushed the reaperrr:fix-bot-plumbing branch from 4d8380c to 1668d16 Nov 20, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 20, 2018

Updated.

@pchote

pchote approved these changes Nov 20, 2018

@pchote pchote added the PR: Needs +2 label Nov 20, 2018

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Nov 20, 2018

side note: it's appalling to see how much perf the HarvesterBotModule wastes (50% of the total tick time in a game with 10 bots).

t.BotTick(this);
});
}
foreach (var t in tickModules)

This comment has been minimized.

@chrisforbes

chrisforbes Nov 22, 2018

Member

I would prefer that you add config for this. Bots have unsynced behavior inside what the rest of the system thinks is synced context.

This comment has been minimized.

@reaperrr

reaperrr Nov 23, 2018

Contributor

Ok, but I'd prefer to do that in a follow-up, since adding that as sub-option will require UI changes (and I suspect those would delay getting this PR merged).

orders.Enqueue(order);
}

// DEPRECATED: Modules should use IBot.QueueOrder instead

This comment has been minimized.

@chrisforbes

chrisforbes Nov 22, 2018

Member

Do you need the deprecated mechanism to still exist?

This comment has been minimized.

@pchote

pchote Nov 22, 2018

Member

The idea is to fix the uses of this as logic is moved from HackyAI into their own modules.

@obrakmann
Copy link
Contributor

obrakmann left a comment

Just one minor comment, looks otherwise good to me

@reaperrr reaperrr force-pushed the reaperrr:fix-bot-plumbing branch from dd0dc37 to 17c2dbe Nov 23, 2018

Fix bot module plumbing
Fixes the issues pointed out after the original harvester module was merged.
Also merges the update rules as discussed on IRC.

@reaperrr reaperrr force-pushed the reaperrr:fix-bot-plumbing branch from 17c2dbe to 2c632fc Nov 23, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 23, 2018

Updated (and rebased).

@pchote

pchote approved these changes Nov 24, 2018

@pchote pchote merged commit 67cba65 into OpenRA:bleed Nov 24, 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