Skip to content

Commit 858048c

Browse files
JasperGeurtzJasperGeurtz
authored
Merge pull request #102 from JavaBWAPI/all-cgpt-fixes
Round of fixes for jbwapi
2 parents 2afe1a2 + 810617e commit 858048c

11 files changed

Lines changed: 384 additions & 51 deletions

File tree

‎src/main/java/bwapi/BuildingPlacer.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,8 @@ private static void ReserveAllStructures(final PlacementReserve reserve, final U
243243
reserve.restoreIfInvalid();
244244
}
245245

246-
private static void ReserveExistingAddonPlacement(final PlacementReserve reserve, final TilePosition desiredPosition, final Game game) {
247-
final TilePosition start = desiredPosition.subtract(new TilePosition(MAX_RANGE, MAX_RANGE)).divide(2);
248-
246+
static void ReserveExistingAddonPlacement(final PlacementReserve reserve, final TilePosition desiredPosition, final Game game) {
247+
final TilePosition start = desiredPosition.subtract(new TilePosition(MAX_RANGE, MAX_RANGE).divide(2));
249248
//Exclude addon placement locations
250249
reserve.backup();
251250
game.self().getUnits().stream()
@@ -259,7 +258,7 @@ private static void ReserveExistingAddonPlacement(final PlacementReserve reserve
259258
reserve.restoreIfInvalid();
260259
}
261260

262-
private static void ReserveDefault(final PlacementReserve reserve, final UnitType type, final TilePosition desiredPosition, final Game game) {
261+
static void ReserveDefault(final PlacementReserve reserve, final UnitType type, final TilePosition desiredPosition, final Game game) {
263262
reserve.backup();
264263
final PlacementReserve original = reserve;
265264

@@ -274,15 +273,14 @@ private static void ReserveDefault(final PlacementReserve reserve, final UnitTyp
274273
case Terran_Missile_Turret:
275274
case Protoss_Robotics_Facility:
276275
case Protoss_Gateway:
277-
case Protoss_Photon_Cannon:
276+
default:
277+
ReserveStructure(reserve, it, 2, type, desiredPosition);
278+
break;
278279
case Terran_Barracks:
279280
case Terran_Bunker:
280281
case Zerg_Creep_Colony:
281282
ReserveStructure(reserve, it, 1, type, desiredPosition);
282283
break;
283-
default:
284-
ReserveStructure(reserve, it, 2, type, desiredPosition);
285-
break;
286284
}
287285
}
288286

‎src/main/java/bwapi/Game.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* game state information from Starcraft Broodwar. Game state information includes all units,
1717
* resources, players, forces, bullets, terrain, fog of war, regions, etc.
1818
*/
19-
public final class Game {
19+
public class Game {
2020
private static final int[][] damageRatio = {
2121
// Ind, Sml, Med, Lrg, Non, Unk
2222
{0, 0, 0, 0, 0, 0}, // Independent
@@ -680,10 +680,12 @@ public List<Unit> getUnitsOnTile(final int tileX, final int tileY) {
680680
* given build tile.
681681
*/
682682
public List<Unit> getUnitsOnTile(final int tileX, final int tileY, final UnitFilter pred) {
683-
return getAllUnits().stream().filter(u -> {
684-
final TilePosition tp = u.getTilePosition();
685-
return tp.x == tileX && tp.y == tileY && pred.test(u);
686-
}).collect(Collectors.toList());
683+
final TilePosition tile = new TilePosition(tileX, tileY);
684+
if (!tile.isValid(this)) {
685+
return Collections.emptyList();
686+
}
687+
final Position p = tile.toPosition();
688+
return getUnitsInRectangle(p.x, p.y, p.x + 32, p.y + 32, pred);
687689
}
688690

689691
public List<Unit> getUnitsInRectangle(final int left, final int top, final int right, final int bottom) {
@@ -740,7 +742,7 @@ public List<Unit> getUnitsInRadius(final Position center, final int radius) {
740742

741743
public List<Unit> getUnitsInRadius(final Position center, final int radius, final UnitFilter pred) {
742744
return getAllUnits().stream()
743-
.filter(u -> center.getApproxDistance(u.getPosition()) <= radius && pred.test(u))
745+
.filter(u -> u.getDistance(center) <= radius && pred.test(u))
744746
.collect(Collectors.toList());
745747
}
746748

@@ -2380,10 +2382,9 @@ public int getLastEventTime() {
23802382
* parameter.
23812383
*/
23822384
final public boolean setMap(final String mapFileName) {
2383-
if (mapFileName == null || mapFileName.length() >= 260 || mapFileName.charAt(0) == 0) {
2385+
if (mapFileName == null || mapFileName.isEmpty() || mapFileName.length() >= 260 || mapFileName.charAt(0) == 0) {
23842386
return false;
23852387
}
2386-
23872388
addCommand(CommandType.SetMap, mapFileName, 0);
23882389
return true;
23892390
}

‎src/main/java/bwapi/Player.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -544,14 +544,14 @@ public Color getColor() {
544544
* @return character code to use for text in Broodwar.
545545
*/
546546
public Text getTextColor() {
547-
switch (color.id) {
547+
switch (getColor().id) {
548548
case 111: // red
549549
return Text.BrightRed;
550550
case 165: // blue
551551
return Text.Blue;
552552
case 159: // teal
553553
return Text.Teal;
554-
case 164: // purp
554+
case 164: // purple
555555
return Text.Purple;
556556
case 156: // orange with fix from @n00byEdge
557557
return Text.Orange;
@@ -561,7 +561,7 @@ public Text getTextColor() {
561561
return Text.PlayerWhite;
562562
case 135: // yellow
563563
return Text.PlayerYellow;
564-
case 185: // green p9
564+
case 117: // green
565565
return Text.DarkGreen;
566566
case 136: // p10
567567
return Text.LightYellow;

‎src/main/java/bwapi/Unit.java‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,7 @@ public List<Unit> getUnitsInRadius(final int radius, final UnitFilter pred) {
12701270
getTop() - radius,
12711271
getRight() + radius,
12721272
getBottom() + radius,
1273-
u -> getDistance(u) <= radius && pred.test(u));
1273+
u -> u != this && getDistance(u) <= radius && pred.test(u));
12741274
}
12751275

12761276
public List<Unit> getUnitsInWeaponRange(final WeaponType weapon) {
@@ -1301,30 +1301,29 @@ public List<Unit> getUnitsInWeaponRange(final WeaponType weapon, final UnitFilte
13011301
getRight() + max,
13021302
getBottom() + max,
13031303
u -> {
1304-
if (!pred.test(u)) {
1305-
return false;
1306-
}
13071304
// Unit check and unit status
13081305
if (u == this || u.isInvincible()) {
13091306
return false;
13101307
}
1311-
13121308
// Weapon distance check
13131309
final int dist = getDistance(u);
13141310
if ((weapon.minRange() != 0 && dist < weapon.minRange()) || dist > max) {
13151311
return false;
13161312
}
1317-
13181313
// Weapon behavioural checks
1314+
// NOTE: The target flag checks below (after targetsOwn) are inverted, matching an upstream bug in C++ BWAPI's UnitInterface::getUnitsInWeaponRange (BWAPILIB/Source/Unit.cpp).
13191315
final UnitType ut = u.getType();
1320-
return (!weapon.targetsOwn() || u.getPlayer().equals(getPlayer())) &&
1321-
(weapon.targetsAir() || u.isFlying()) &&
1322-
(weapon.targetsGround() || !u.isFlying()) &&
1323-
(!weapon.targetsMechanical() || !ut.isMechanical()) &&
1324-
(!weapon.targetsOrganic() || !ut.isOrganic()) &&
1325-
(!weapon.targetsNonBuilding() || ut.isBuilding()) &&
1326-
(!weapon.targetsNonRobotic() || ut.isRobotic()) &&
1327-
(!weapon.targetsOrgOrMech() || (!ut.isOrganic() && !ut.isMechanical()));
1316+
if ((weapon.targetsOwn() && !u.getPlayer().equals(getPlayer())) ||
1317+
(!weapon.targetsAir() && !u.isFlying()) ||
1318+
(!weapon.targetsGround() && u.isFlying()) ||
1319+
(weapon.targetsMechanical() && ut.isMechanical()) ||
1320+
(weapon.targetsOrganic() && ut.isOrganic()) ||
1321+
(weapon.targetsNonBuilding() && !ut.isBuilding()) ||
1322+
(weapon.targetsNonRobotic() && !ut.isRobotic()) ||
1323+
(weapon.targetsOrgOrMech() && (ut.isOrganic() || ut.isMechanical()))) {
1324+
return false;
1325+
}
1326+
return pred.test(u);
13281327
});
13291328
}
13301329

@@ -3913,7 +3912,7 @@ public boolean canAttackUnitGrouped(Unit targetUnit, boolean checkCanTargetUnit,
39133912
return false;
39143913
}
39153914

3916-
if (isInvincible()) {
3915+
if (targetUnit.isInvincible()) {
39173916
return false;
39183917
}
39193918

@@ -4717,16 +4716,14 @@ public boolean canGather(Unit targetUnit, boolean checkCanTargetUnit, boolean ch
47174716
return false;
47184717
}
47194718

4720-
if (!isCompleted()) {
4719+
if (!targetUnit.isCompleted()) {
47214720
return false;
47224721
}
47234722

4724-
if (!hasPath(getPosition())) {
4723+
if (!hasPath(targetUnit.getPosition())) {
47254724
return false;
47264725
}
4727-
4728-
return !uType.isRefinery() || getPlayer().equals(game.self());
4729-
4726+
return !uType.isRefinery() || targetUnit.getPlayer().equals(game.self());
47304727
}
47314728

47324729
public boolean canReturnCargo() {
@@ -6171,7 +6168,7 @@ public boolean canUseTechUnit(TechType tech, Unit targetUnit, boolean checkCanTa
61716168
if (!targetType.isOrganic() || targetType.isFlyer()) {
61726169
return false;
61736170
}
6174-
if (!targetUnit.getPlayer().isNeutral() && getPlayer().isEnemy(getPlayer())) {
6171+
if (!targetUnit.getPlayer().isNeutral() && getPlayer().isEnemy(targetUnit.getPlayer())) {
61756172
return false;
61766173
}
61776174
break;

‎src/main/java/bwapi/UnitFilter.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import java.util.function.Predicate;
44

55
public interface UnitFilter extends Predicate<Unit> {
6-
UnitFilter IsTransPort = u -> u.getType().spaceProvided() > 0 && u.getType() != UnitType.Terran_Bunker;
6+
UnitFilter IsTransport = u -> u.getType().spaceProvided() > 0 && u.getType() != UnitType.Terran_Bunker;
7+
@Deprecated // backwards compatible
8+
UnitFilter IsTransPort = IsTransport;
79
UnitFilter CanProduce = u -> u.getType().canProduce();
810
UnitFilter CanAttack = u -> u.getType().canAttack();
911
UnitFilter CanMove = u -> u.getType().canMove();

‎src/test/java/bwapi/BuildingPlacerTest.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package bwapi;
2+
3+
import org.junit.Test;
4+
5+
import java.util.Collections;
6+
7+
import static org.junit.Assert.assertEquals;
8+
import static org.mockito.Mockito.mock;
9+
import static org.mockito.Mockito.when;
10+
11+
public class BuildingPlacerTest {
12+
13+
@Test
14+
public void reserveExistingAddonPlacementShouldUseHalfMaxRangeOffset() {
15+
Game game = mock(Game.class);
16+
Player self = mock(Player.class);
17+
when(game.self()).thenReturn(self);
18+
19+
TilePosition desired = new TilePosition(40, 40);
20+
// start should be desired - (64, 64)/2 = (8, 8).
21+
// Factory at (40, 40) has its 2x2 addon at world tiles (44, 41)..(45, 42),
22+
// which corresponds to reserve coordinates (36, 33)..(37, 34).
23+
Unit factory = mock(Unit.class);
24+
when(factory.exists()).thenReturn(true);
25+
when(factory.getType()).thenReturn(UnitType.Terran_Factory);
26+
when(factory.getTilePosition()).thenReturn(new TilePosition(40, 40));
27+
when(self.getUnits()).thenReturn(Collections.singletonList(factory));
28+
29+
BuildingPlacer.PlacementReserve reserve = new BuildingPlacer.PlacementReserve(64);
30+
reserve.setRange(0, 0, 64, 64, (byte) 1);
31+
32+
BuildingPlacer.ReserveExistingAddonPlacement(reserve, desired, game);
33+
34+
assertEquals(0, reserve.getValue(36, 33));
35+
assertEquals(0, reserve.getValue(37, 34));
36+
}
37+
38+
@Test
39+
public void reserveDefaultShouldUsePaddingTwoForFactory() {
40+
Game game = mock(Game.class);
41+
Player self = mock(Player.class);
42+
when(game.self()).thenReturn(self);
43+
44+
TilePosition desired = new TilePosition(40, 40);
45+
Unit factory = mock(Unit.class);
46+
when(factory.exists()).thenReturn(true);
47+
when(factory.getType()).thenReturn(UnitType.Terran_Factory);
48+
when(factory.getTilePosition()).thenReturn(new TilePosition(40, 40));
49+
when(factory.getPosition()).thenReturn(new TilePosition(40, 40).toPosition());
50+
when(self.getUnits()).thenReturn(Collections.singletonList(factory));
51+
52+
BuildingPlacer.PlacementReserve reserve = new BuildingPlacer.PlacementReserve(64);
53+
reserve.setRange(0, 0, 64, 64, (byte) 1);
54+
55+
BuildingPlacer.ReserveDefault(reserve, UnitType.Terran_Supply_Depot, desired, game);
56+
57+
// For a 4x3 Factory at (40, 40) (reserve 32, 32) and a 3x2 Supply Depot:
58+
// padding=1 reserves x in [25, 35), y in [27, 35)
59+
// padding=2 reserves x in [24, 36), y in [26, 36)
60+
assertEquals(0, reserve.getValue(24, 26));
61+
assertEquals(0, reserve.getValue(35, 35));
62+
}
63+
}

‎src/test/java/bwapi/FilterTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,20 @@ public void testFilterCombinations() {
3939
assertTrue(units.stream().noneMatch(IsMineralField.and(IsRefinery)));
4040
assertTrue(units.stream().noneMatch(IsMineralField.negate()));
4141
}
42+
43+
@Test
44+
public void isTransportShouldMatchDropshipAndExcludeBunker() {
45+
Unit dropship = mock(Unit.class);
46+
when(dropship.getType()).thenReturn(UnitType.Terran_Dropship);
47+
48+
Unit bunker = mock(Unit.class);
49+
when(bunker.getType()).thenReturn(UnitType.Terran_Bunker);
50+
51+
Unit marine = mock(Unit.class);
52+
when(marine.getType()).thenReturn(UnitType.Terran_Marine);
53+
54+
assertTrue(IsTransport.test(dropship));
55+
assertFalse(IsTransport.test(bunker));
56+
assertFalse(IsTransport.test(marine));
57+
}
4258
}

‎src/test/java/bwapi/GameTest.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,15 @@
1313
import java.util.List;
1414

1515
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.junit.Assert.assertEquals;
17+
import static org.junit.Assert.assertFalse;
1618
import static org.junit.Assert.assertNull;
1719
import static org.junit.Assert.assertTrue;
1820
import static org.mockito.BDDMockito.given;
21+
import static org.mockito.Mockito.doReturn;
1922
import static org.mockito.Mockito.mock;
23+
import static org.mockito.Mockito.spy;
24+
import static org.mockito.Mockito.when;
2025

2126
@RunWith(Theories.class)
2227
public class GameTest {
@@ -116,4 +121,57 @@ public void hasPowerOnTile() throws IOException {
116121
// tile (31, 15) is pixel (992, 480), right next to the pylon
117122
assertTrue(game.hasPower(31, 15));
118123
}
124+
125+
@Test
126+
public void getUnitsOnTileShouldUseUnitBoundingBoxAndValidateTile() {
127+
Game game = spy(new Game());
128+
doReturn(128 * 32).when(game).mapPixelWidth();
129+
doReturn(128 * 32).when(game).mapPixelHeight();
130+
131+
// Command Center at top-left tile (10, 10) spans pixels [320..447, 320..415] (tiles 10..13, 10..12)
132+
Unit commandCenter = mock(Unit.class);
133+
when(commandCenter.getTilePosition()).thenReturn(new TilePosition(10, 10));
134+
when(commandCenter.getLeft()).thenReturn(320);
135+
when(commandCenter.getTop()).thenReturn(320);
136+
when(commandCenter.getRight()).thenReturn(447);
137+
when(commandCenter.getBottom()).thenReturn(415);
138+
139+
game.setAllUnits(Collections.singletonList(commandCenter));
140+
141+
// Tile (11, 11) is covered by the Command Center even though its top-left tile is (10, 10)
142+
List<Unit> onTile = game.getUnitsOnTile(11, 11);
143+
assertEquals(Collections.singletonList(commandCenter), onTile);
144+
145+
// Invalid tile should return empty list
146+
assertTrue(game.getUnitsOnTile(-1, -1).isEmpty());
147+
}
148+
149+
@Test
150+
public void getUnitsInRadiusShouldUseUnitEdgeDistance() {
151+
Game game = new Game();
152+
153+
// Unit centered at (100, 100) with bounding box [80..120, 80..120]
154+
Unit unit = mock(Unit.class);
155+
Position unitCenter = new Position(100, 100);
156+
Position queryCenter = new Position(150, 100);
157+
when(unit.getPosition()).thenReturn(unitCenter);
158+
when(unit.getLeft()).thenReturn(80);
159+
when(unit.getRight()).thenReturn(120);
160+
when(unit.getTop()).thenReturn(80);
161+
when(unit.getBottom()).thenReturn(120);
162+
when(unit.exists()).thenReturn(true);
163+
when(unit.getDistance(queryCenter)).thenCallRealMethod();
164+
165+
game.setAllUnits(Collections.singletonList(unit));
166+
167+
// Center-to-center distance is 50 (> 35), but edge-to-point distance is 30 (<= 35)
168+
List<Unit> result = game.getUnitsInRadius(queryCenter, 35);
169+
assertEquals(Collections.singletonList(unit), result);
170+
}
171+
172+
@Test
173+
public void setMapWithEmptyStringShouldReturnFalseInsteadOfThrowing() {
174+
Game game = new Game();
175+
assertFalse(game.setMap(""));
176+
}
119177
}

‎src/test/java/bwapi/PlayerTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import org.junit.Test;
44

5+
import static org.junit.Assert.assertEquals;
56
import static org.junit.Assert.assertFalse;
67
import static org.mockito.Mockito.mock;
78
import static org.mockito.Mockito.when;
@@ -18,4 +19,13 @@ public void neutralIsNotEnemy() {
1819

1920
assertFalse(self.isEnemy(neutral));
2021
}
22+
23+
@Test
24+
public void getTextColorShouldReturnDarkGreenForColorGreen() {
25+
Player player = mock(Player.class);
26+
when(player.getColor()).thenReturn(Color.Green); // Color(117)
27+
when(player.getTextColor()).thenCallRealMethod();
28+
29+
assertEquals(Text.DarkGreen, player.getTextColor());
30+
}
2131
}

0 commit comments

Comments
 (0)