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.

Sign up for GitHub

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

Jump to bottom

Zerg ResourceDepots refactor/training functionality, MobileUnit build time, BWEM initalization #63

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Zerg ResourceDepots refactor/training functionality, MobileUnit build time, BWEM initalization #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a91dd08
Zerg ResourceDepots refactor
kdlin10 Aug 18, 2018
05dcac6
Added getRemainingBuildTime()
kdlin10 Aug 18, 2018
82c4c36
Added assigning starting locations to Bases as part of initialization
kdlin10 Aug 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion BWAPI4J/src/main/java/bwem/map/MapInitializerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,13 @@ public void initialize(final boolean enableTimer) {
logger.info("Map::createBases: " + timer.elapsedMilliseconds() + " ms");
timer.reset();
}


assignStartingLocationsToSuitableBases();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function will throw an exception if it fails to match suitable base locations to all starting locations. Lots of maps play around with start locations and observer player starting locations. So, this change breaks more than it fixes, IMO. If the intent of this change is so the user is not required to manually invoke this function during the bot's initialization stages, the thrown exception needs to be removed or dealt with. As something to consider, BWEM generates a list of starting locations not assigned to bases if the function fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, I'll withdraw this proposal. Was just struck by how user-unfriendly it was to have an initialize() then have to manually do something that seems like core functionality, not an option.

if (enableTimer) {
logger.info("Map::assignStartingLocationsToSuitableBases: " + timer.elapsedMilliseconds() + " ms");
timer.reset();
}

// /// bw << "Map::initialize: " << overallTimer.elapsedMilliseconds() << " ms" << endl;
if (enableTimer) {
logger.info("Map::initialize Total: " + overallTimer.elapsedMilliseconds() + " ms");
Expand Down
11 changes: 8 additions & 3 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/Player.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,14 @@ && hasResearched(type.requiredTech())
}

public boolean canMake(Unit builder, UnitType type) {
if (!canMake(type) || !builder.isA(type.whatBuilds().getFirst())) {
return false;
}
if (!canMake(type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since builder is a Unit, you can get rid of the three checks for "is a hatchery or is a lair or is a hive" and just write builder instanceof ZergResourceDepot which reduces the amount of code. Also, what is the purpose of checking whatBuilds for UnitType.Zerg_Larva? Can we not omit that check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check is intended to allow the case where type builds from Zerg_Larva and the builder is a ZergResourceDepot to pass through without returning false. Removing that check would allow all cases where builder is a ZergResourceDepot to go through without returning false.

Thought I'd keep the checks against UnitType since that seemed to be the style - will revise to instanceof ZergResourceDepot

Copy link
Collaborator

@adakitesystems adakitesystems Aug 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand correctly, you're saying that self.canMake(myHatchery, UnitType.Zerg_Hydralisk will return true with your change? Then, I assume all other production facilities will need to be considered as well for cases such as self.canMake(myFactory, UnitType.Terran_Goliath)?

Edit: Nevermind that. It probably already returns true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - the conditional is intended to filter out any invalid builder / type combos. Everything else passes through fine, but Zerg MobileUnits are an exception because they are paired with Zerg_Larva and would get filtered out when combined with a ZergResourceDepot

|| !(builder.isA(type.whatBuilds().getFirst())
|| (type.whatBuilds().getFirst().equals(UnitType.Zerg_Larva)
&& (builder.isA(UnitType.Zerg_Hatchery)
|| builder.isA(UnitType.Zerg_Lair)
|| builder.isA(UnitType.Zerg_Hive))))) {
return false;
}
return type.requiredUnits()
.stream()
.filter(UnitType::isAddon)
Expand Down
128 changes: 25 additions & 103 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/unit/Hatchery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// This file is part of BWAPI4J.
//
// BWAPI4J is free software: you can redistribute it and/or modify
// it under the terms of the Lesser GNU General Public License as published
// it under the terms of the Lesser GNU General Public License as published
// by the Free Software Foundation, version 3 only.
//
// BWAPI4J is distributed in the hope that it will be useful,
Expand All @@ -20,117 +20,39 @@

package org.openbw.bwapi4j.unit;

import static org.openbw.bwapi4j.type.UnitCommandType.Morph;
import static org.openbw.bwapi4j.type.UnitType.Zerg_Lair;

import java.util.List;
import java.util.stream.Collectors;

import org.openbw.bwapi4j.type.TechType;
import org.openbw.bwapi4j.type.UnitType;
import org.openbw.bwapi4j.type.UpgradeType;

public class Hatchery extends BuildingImpl
implements Organic, ResearchingFacility, ResourceDepot, Morphable {
protected Hatchery(int id, UnitType type, int timeSpotted) {
super(id, type, timeSpotted);
}

protected Hatchery(int id, int timeSpotted) {
this(id, UnitType.Zerg_Hatchery, timeSpotted);
}

@Override
public boolean isReadyForResources() {
return isCompleted;
}

@Override
public boolean trainWorker() {
return super.train(UnitType.Zerg_Drone);
}

/**
* Retrieves a list of larvae present at this hatchery.
*
* @return list of larvae
*/
public List<Larva> getLarva() {
return super.getAllUnits()
.stream()
.filter(u -> u instanceof Larva && ((Larva) u).getHatchery().getId() == this.getId())
.map(u -> (Larva) u)
.collect(Collectors.toList());
}

public boolean researchBurrowing() {
return super.research(TechType.Burrowing);
}

@Override
public boolean isUpgrading() {
return isUpgrading;
}

@Override
public boolean isResearching() {
return isResearching;
}

@Override
public boolean cancelResearch() {
return super.cancelResearch();
}
import static org.openbw.bwapi4j.type.UnitCommandType.Morph;
import static org.openbw.bwapi4j.type.UnitType.Zerg_Lair;

@Override
public boolean cancelUpgrade() {
return super.cancelUpgrade();
}
public class Hatchery extends ZergResourceDepotImpl {

@Override
public boolean morph(UnitType type) {
if (type != Zerg_Lair) {
throw new IllegalArgumentException("Cannot morph to " + type);
protected Hatchery(int id, UnitType type, int timeSpotted) {

super(id, type, timeSpotted);
}

protected Hatchery(int id, int timeSpotted) {

this(id, UnitType.Zerg_Hatchery, timeSpotted);
}

return issueCommand(this.id, Morph, -1, -1, -1, Zerg_Lair.getId());
}

public boolean morph() {
return morph(Zerg_Lair);
}

@Override
public boolean canResearch(TechType techType) {
return super.canResearch(techType);
}

@Override
public boolean canUpgrade(UpgradeType upgradeType) {
return super.canUpgrade(upgradeType);
}

@Override
public boolean research(TechType techType) {
return super.research(techType);
}

@Override
public boolean upgrade(UpgradeType upgradeType) {
return super.upgrade(upgradeType);
}

@Override
public UpgradeInProgress getUpgradeInProgress() {
return super.getUpgradeInProgress();
}

@Override
public ResearchInProgress getResearchInProgress() {
return super.getResearchInProgress();
}
@Override
public boolean morph(UnitType type) {
if (type != Zerg_Lair) {
throw new IllegalArgumentException("Cannot morph to " + type);
}

@Override
public int supplyProvided() {
return type.supplyProvided();
}
return issueCommand(this.id, Morph, -1, -1, -1, Zerg_Lair.getId());
}

@Override
public boolean morph() {
return morph(Zerg_Lair);
}
}
26 changes: 17 additions & 9 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/unit/Hive.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// This file is part of BWAPI4J.
//
// BWAPI4J is free software: you can redistribute it and/or modify
// it under the terms of the Lesser GNU General Public License as published
// it under the terms of the Lesser GNU General Public License as published
// by the Free Software Foundation, version 3 only.
//
// BWAPI4J is distributed in the hope that it will be useful,
Expand All @@ -22,13 +22,21 @@

import org.openbw.bwapi4j.type.UnitType;

public class Hive extends Lair {
protected Hive(int id, int timeSpotted) {
super(id, UnitType.Zerg_Hive, timeSpotted);
}
public class Hive extends ZergResourceDepotImpl {

@Override
public boolean morph(UnitType type) {
return false;
}
protected Hive(int id, int timeSpotted) {

super(id, UnitType.Zerg_Hive, timeSpotted);
}

@Override
public boolean morph(UnitType type) {
return false;
}

@Override
public boolean morph() {
// TODO Auto-generated method stub
return false;
}
}
69 changes: 36 additions & 33 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/unit/Lair.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// This file is part of BWAPI4J.
//
// BWAPI4J is free software: you can redistribute it and/or modify
// it under the terms of the Lesser GNU General Public License as published
// it under the terms of the Lesser GNU General Public License as published
// by the Free Software Foundation, version 3 only.
//
// BWAPI4J is distributed in the hope that it will be useful,
Expand All @@ -20,48 +20,51 @@

package org.openbw.bwapi4j.unit;

import org.openbw.bwapi4j.type.UnitCommandType;
import org.openbw.bwapi4j.type.UnitType;
import org.openbw.bwapi4j.type.UpgradeType;

import static org.openbw.bwapi4j.type.UnitCommandType.Morph;
import static org.openbw.bwapi4j.type.UnitType.Zerg_Hive;

import org.openbw.bwapi4j.type.UnitType;
import org.openbw.bwapi4j.type.UpgradeType;
public class Lair extends ZergResourceDepotImpl {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There seems to be only formatting changes with this file. This formatting does not match the rest of the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isReadyForResources() was removed and Lair now extends from a ZergResourceDepotImpl abstract. I believe not having Lair extend Hatchery better reflects the game. For example, somebody might want to check if there is a Hatchery to research Burrow and use their Lair for other upgrades - previously, checking instanceof Hatchery on our `Lair would return true.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sorry my mistake.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't agree here. I'm actually using the Lair is a Hatch mechanic in my bot, as it makes it easier to check if I already have a Lair although it is updated to a Hive already.
As we discussed on Discord, both ways have their pros and contras.

Currently you'll have to check if you supposed Hatchery is actually a Lair or a Hive.
After the change, you'd have to check if you've got a Lair or a Hive. You only move the check to another position.

Unless there's an additional benefit, this is just toggling code back and forth.

Copy link
Contributor Author

@kdlin10 kdlin10 Aug 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The proposed scheme would have 4 different types of checks that could be accomplished with a single comparison (If something is a Hatchery, Lair, Hive, or all three) - no second check necessary. If one wants to select two types, that can be accomplished with a second comparison.

With the current arrangement, with a single check, one can filter a Unit down to all a Hatchery || Lair || Hive, a Lair || Hive, or a Hive. Further necking down requires a second check.

More importantly, I think it is not intuitive to check if a Unit is a Lair and have it return true for a Hive. When I ask if a Unit is a Hatchery, it's not obvious at all Lair and Hive qualify as a Hatchery

Copy link
Collaborator

@Bytekeeper Bytekeeper Aug 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We're repeating our discord chat :)
This is not at all an objective argument we're having here. It's just "taste".
I for example find it very intuitive that a Hive isa Lair- it's the same way it is presented to a player in-game.

There's no objective benefit doing it either way. And I believe every change breaking API should have one.


protected Lair(int id, int timeSpotted) {

super(id, UnitType.Zerg_Lair, timeSpotted);
}

public class Lair extends Hatchery {
protected Lair(int id, int timeSpotted) {
super(id, UnitType.Zerg_Lair, timeSpotted);
}
protected Lair(int id, UnitType type, int timeSpotted) {

protected Lair(int id, UnitType type, int timeSpotted) {
super(id, type, timeSpotted);
}
super(id, type, timeSpotted);
}


public boolean upgradeVentralSacs() {

return super.upgrade(UpgradeType.Ventral_Sacs);
}

@Override
public boolean isReadyForResources() {
return true;
}
public boolean upgradeAntennae() {

public boolean upgradeVentralSacs() {
return super.upgrade(UpgradeType.Ventral_Sacs);
}
return super.upgrade(UpgradeType.Antennae);
}

public boolean upgradeAntennae() {
return super.upgrade(UpgradeType.Antennae);
}
public boolean upgradePneumatizedCarapace() {

public boolean upgradePneumatizedCarapace() {
return super.upgrade(UpgradeType.Pneumatized_Carapace);
}
return super.upgrade(UpgradeType.Pneumatized_Carapace);
}

@Override
public boolean morph(UnitType type) {
if (type != Zerg_Hive) {
throw new IllegalArgumentException("Cannot morph to " + type);
@Override
public boolean morph(UnitType type) {
if (type != Zerg_Hive) {
throw new IllegalArgumentException("Cannot morph to " + type);
}
return issueCommand(this.id, Morph, -1, -1, -1, Zerg_Hive.getId());
}
return issueCommand(this.id, Morph, -1, -1, -1, Zerg_Hive.getId());
}

@Override
public boolean morph() {
return morph(Zerg_Hive);
}
@Override
public boolean morph() {
return morph(Zerg_Hive);
}
}
2 changes: 2 additions & 0 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/unit/MobileUnit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,6 @@ public interface MobileUnit extends PlayerUnit {
boolean isEnsnared();

double getTopSpeed();

int getRemainingTrainTime();
}
4 changes: 4 additions & 0 deletions BWAPI4J/src/main/java/org/openbw/bwapi4j/unit/MobileUnitImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,8 @@ public boolean isEnsnared() {
public double getTopSpeed() {
return getUnitStatCalculator().topSpeed(type);
}

public int getRemainingTrainTime() {
return remainingTrainTime;
}
}
Loading