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

SKoutpost directive and overload: improvements and fixes #134

Closed
wants to merge 35 commits into from

Conversation

zGeneral
Copy link
Contributor

@zGeneral zGeneral commented May 18, 2019

Pull request summary

Edit: verified and checked again on 14/June, waiting for PR review.

(all tested and working properly now)!

SKOuptpost container construction sites for minerals are now pushed to colony worker build list.
similarly,
SKOuptpost containers with hits < 50% are now maintained by workers (previously, harvesters maintain their own containers, but mineral drones have their container unrepaired and decay to destruction)

When there is no container:
drones extracting minerals assume containers already exist to transfer mineral in.
when there is no container (mainly in SK rooms due to container build delay) the drones stay idle and die of age.
this PR makes them travel back to colony for to drop it off as a temp alternative option until the container is built.

this should also fix issue #120, miners spawn in own room without having terminal/storage

sourceReapers now do not spawn in center SK room

Description:

Added:

  • None

Changed:

  • None

Removed:

  • None

Fixed:

issue #120

  • None

Testing checklist:

  • Changes are backward-compatible OR version migration code is included
  • Codebase compiles with current tsconfig configuration
  • Tested changes on {choose PUBLIC/PRIVATE} server OR changes are trivial (e.g. typos)

@zGeneral zGeneral changed the title Fix handleDrone Fix SKOutpost building container@mineral site and handleDrone May 18, 2019
@zGeneral zGeneral changed the title Fix SKOutpost building container@mineral site and handleDrone Fix SKOutpost building/repair container@mineral site and handleDrone May 19, 2019
@zGeneral zGeneral changed the title Fix SKOutpost building/repair container@mineral site and handleDrone Fix SKOutpost building/repair container@mineral site and handleDrone adjustment May 19, 2019
@zGeneral zGeneral changed the title Fix SKOutpost building/repair container@mineral site and handleDrone adjustment SKoutpost directive and overload: improvements and fixes Jun 8, 2019
Copy link
Owner

@bencbartlett bencbartlett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments -- there are some good changes in here but also some things I'm confused about

package.json Outdated
@@ -31,14 +31,15 @@
"rollup-plugin-node-resolve": "3.3.0",
"rollup-plugin-progress": "^0.4.0",
"rollup-plugin-screeps": "0.1.2",
"rollup-plugin-typescript2": "0.16.1",
"rollup-plugin-typescript2": "^0.21.1",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to make changes to dependencies, as it can break the checksum generation process. Is there a reason you are updating the rollup plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, check latest PR


constructor(flag: Flag) {
super(flag, colony => colony.level >= 7);
}

isCenterRoom(): boolean {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unnecessary; use something like const isCenterRoom = Cartographer.roomType(this.pos.roomName) == ROOMTYPE_CORE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, check latest PR


spawnMoarOverlords() {
this.overlords.sourceReaper = new SourceReaperOverlord(this);
if(this.memory.isCenterRoom == undefined){
this.memory.isCenterRoom = this.isCenterRoom();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't store isCenterRoom in memory. it's inexpensive to calculate and can be stuck directly in the DirectiveSKOutpost constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, it is now checked in constructor.
existing Overmind users that already have the directive flag in place will not benefit from this, would you suggest to add it under init() too?

if(Game.time % 150 == 0){
const containerNeedRepair = this.getContainersToRepair();
if (containerNeedRepair && !this.colony.overlords.work.repairStructures.includes(containerNeedRepair)) {
this.colony.overlords.work.repairStructures.push(containerNeedRepair);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sk miners already build and repair their own containers -- why do you want to offload this to the colony workers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drones that harvest minerals do not repair their containers like the drones harvesting energy.
I have noticed in replays, that mineral containers always decays out without repair.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes does not have a filter for mineral container only, the worker will repair all containers in SKroom below 50%, which will most probably apply to mineral container being below the threshold

@@ -36,7 +36,10 @@ export class DirectiveExtract extends Directive {
} else {
priority = OverlordPriority.remoteSKRoom.mineral;
}
this.overlords.extract = new ExtractorOverlord(this, priority);
//Fix: Only spawn drones if there is a terminal or storage
if (!!this.colony.terminal || !!this.colony.storage){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@@ -105,6 +105,16 @@ export class ExtractorOverlord extends Overlord {
}

private handleDrone(drone: Zerg): void {
//fix: if there is no container, then transfer the minerals yourself!
if(!this.container && _.sum(drone.carry) == drone.carryCapacity ){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably just shouldn't spawn any extractors unless there is a container

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea,
what about changing the amount to zero in wishlist if there is no container, something like this:
const amount = this.mineral && this.mineral.mineralAmount > 0 && this.container ? this.mineral.pos.availableNeighbors().length : 0;

@@ -95,7 +95,7 @@ export class SourceReaperOverlord extends CombatOverlord {
reaper.healSelfIfPossible();
}
// Kite around ranged invaders until a defender arrives
if (this.room.invaders.length > 2 && _.filter(this.defenders, def => def.room == this.room).length == 0) {
if (this.pos.findInRange(this.room.invaders,10).length > 2 && _.filter(this.defenders, def => def.room == this.room).length == 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will revert this one back.

@zGeneral zGeneral closed this Jan 10, 2021
@zGeneral zGeneral deleted the fixHandleDrone branch January 10, 2021 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants