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

Training a unit when the queue is full throws Index out of Bounds #61

Open
ryanleecode opened this issue Sep 8, 2020 · 5 comments
Open
Labels
bwapi-serverside Depends on changes on the server side of bwapi

Comments

@ryanleecode
Copy link

ryanleecode commented Sep 8, 2020

Training a unit when the queue is full throws Index out of Bounds Exception. Tested on dev branch w/ latcom on.

Stack Trace:

java.lang.ArrayIndexOutOfBoundsException: Index 5 out of bounds for length 5
        at bwapi.CommandTemp.execute(CommandTemp.java:725)
        at bwapi.CommandTemp.execute(CommandTemp.java:41)
        at bwapi.Unit.issueCommand(Unit.java:2119)
@dgant
Copy link
Contributor

dgant commented Sep 10, 2020

Solution would be for here to ignore train commands when the unit's training queue is already full.

There may be some fanciness required to accurately handle the case when the front of the queue will finish training prior to latency-delayed command execution, but I imagine the simpler solution is good-enough. Since there's never an advantage to queuing more than 2 units nobody should be harmed by brief prediction errors in that case.

@JasperGeurtz
Copy link
Collaborator

Code is identical to https://github.com/bwapi/bwapi/blob/e4a29d73e6021037901da57ceb06e37248760240/bwapi/include/BWAPI/Client/CommandTemp.h#L878

But Java will throw & C++ will just overwrite the next value in memory?
Ignoring those train commands is probably the "fix'

@JasperGeurtz
Copy link
Collaborator

JasperGeurtz commented Sep 11, 2020

Proposed fix:

diff --git a/src/main/java/bwapi/Unit.java b/src/main/java/bwapi/Unit.java
index 821f648..87524ee 100644
--- a/src/main/java/bwapi/Unit.java
+++ b/src/main/java/bwapi/Unit.java
@@ -2116,6 +2116,13 @@ public class Unit implements Comparable<Unit> {
         }

         if (game.isLatComEnabled()) {
+            // reject training commands when the trainingQueue is full
+            if (command.type == UnitCommandType.Train) {
+                final Integer queueCount = self().trainingQueueCount.get();
+                if (queueCount != null && queueCount >= self().trainingQueue.length) {
+                    return false;
+                }
+            }
             new CommandTemp(command, game).execute();
         }

@ryanleecode can you check if the fix-training-oob branch fixes it for you?

@ryanleecode
Copy link
Author

Doesn't seem the work the trainingQueueCount is out of sync (1) with unitData.getTrainingQueueCount() (5).

@JasperGeurtz
Copy link
Collaborator

OK, seems like I was too optimistic

Debugging a bit more it seems like it's a BWAPI Server latcom bug

Game table mapping not found.
Game table mapping not found.
0 | 2572 | 0 | 99881625
1 | 0 | 0 | 0
2 | 0 | 0 | 0
3 | 0 | 0 | 0
4 | 0 | 0 | 0
5 | 0 | 0 | 0
6 | 0 | 0 | 0
7 | 0 | 0 | 0
Connected
Connection successful
added UnitCommand: 15
added UnitCommand: 15
added UnitCommand: 15
added UnitCommand: 15
######## FRAME 1158 ########
getTrainingQueueCount: 0
before: [null, null, null, null, null], null
after: [Terran_SCV, null, null, null, null], 1
added UnitCommand: 4
training worker: 1
trainingQueue: [Terran_SCV]
######## FRAME 1159 ########
getTrainingQueueCount: 1
before: [Terran_SCV, null, null, null, null], 1
after: [Terran_SCV, Terran_SCV, null, null, null], 1
added UnitCommand: 4
training worker: 2
trainingQueue: [Terran_SCV, Terran_SCV]
######## FRAME 1160 ########
getTrainingQueueCount: 3
before: [Terran_SCV, Terran_SCV, null, null, null], 1
after: [Terran_SCV, Terran_SCV, null, Terran_SCV, null], 1
added UnitCommand: 4
training worker: 3
trainingQueue: [Terran_SCV, Terran_SCV, null, Terran_SCV]
######## FRAME 1161 ########
getTrainingQueueCount: 4
before: [Terran_SCV, Terran_SCV, null, Terran_SCV, null], 1
after: [Terran_SCV, Terran_SCV, null, Terran_SCV, Terran_SCV], 1
added UnitCommand: 4
training worker: 4
trainingQueue: [Terran_SCV, Terran_SCV, null, Terran_SCV, Terran_SCV]
######## FRAME 1162 ########
getTrainingQueueCount: 5
before: [Terran_SCV, Terran_SCV, null, Terran_SCV, Terran_SCV], 1
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 5
	at bwapi.CommandTemp.execute(CommandTemp.java:729)
	at bwapi.CommandTemp.execute(CommandTemp.java:43)
	at bwapi.Unit.issueCommand(Unit.java:2126)
	at bwapi.Unit.train(Unit.java:2238)
	at game.TrainingQueueTest.onFrame(TrainingQueueTest.java:40)
	at bwapi.EventHandler.operation(EventHandler.java:29)
	at bwapi.Client.update(Client.java:269)
	at bwapi.BWClient.startGame(BWClient.java:56)
	at bwapi.BWClient.startGame(BWClient.java:35)
	at game.TrainingQueueTest.<init>(TrainingQueueTest.java:16)
	at game.TrainingQueueTest.main(TrainingQueueTest.java:50)

Process finished with exit code 1

the unitData.getTrainingQueueCount() skips from 1 to 3 for some reason

getTrainingQueueCount: 1
getTrainingQueueCount: 3

With latcom disabled, this bug does not occur

@N00byEdge any ideas where this could happen?

@JasperGeurtz JasperGeurtz added the bwapi-serverside Depends on changes on the server side of bwapi label Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bwapi-serverside Depends on changes on the server side of bwapi
Projects
None yet
Development

No branches or pull requests

3 participants