auto-join Local PC game, second instance crashes on some config

[GoogleCodeExporter]

What steps will reproduce the problem?
1.in bwapi.ini: set game = BWAPI
2. set map = maps\AI\tournament 1\dragoons.scm (from AIIDE 2010 tournament 1)
3. set game_type = USE_MAP_SETTINGS
4. set both ai's to skynet (uses revision 3769)
5. set auto_manu = LAN
6. launch the games (one after another with a small delay)

What is the expected output? What do you see instead?
Expected to automatically start a match using the provided map
Instead, both instances create a separate game called BWAPI. 

What version of the product are you using? On what operating system?
BWAPI 3.7.3 under Windows 7 SP1

Please provide any additional information below.
When leaving 'game' blank - it doesn't seem to affect the algorithm
When leaving 'map' blank - the game reaches the "select a game" menu, and after 
selecting a map (in the first instance) the second instance automatically 
attempts to join it, but without success (the second client freezes)

Original issue reported on code.google.com by Leonid.T...@gmail.com on 2 Jun 2012 at 10:40

• Merged into: #443

[GoogleCodeExporter]

Will check it out.

Original comment by AHeinerm on 6 Jun 2012 at 12:28

[GoogleCodeExporter]

There were actually other issues with the auto menu as well which I am 
addressing today.

I am currently testing the setup.

Right now I've fixed the following:
 - If the "game" option is set and is not found, instances will now wait 3 seconds before creating a game if the "map" option is set.
 - Added a timer to limit the number of packets being sent for race changing (which caused strange issues).
 - Changed the behaviour of game starts so that (hopefully) games can't accidentally begin before data transfers can complete.

I did not experience the client freezing, but did experience hangs in the lobby 
after the client joined and crashes as the game was beginning before the above 
changes were made.

Original comment by AHeinerm on 4 Jul 2012 at 2:47

• Changed state: Started
• Added labels: Type-Defect, Priority-Critical, Milestone-Release, Component-Logic, Usability

[GoogleCodeExporter]

Fixed in r4152.

Original comment by AHeinerm on 4 Jul 2012 at 3:25

• Changed state: Fixed

[GoogleCodeExporter]

Hi,

Tested your version today, it's still crashes...
The crash appears just after the second instance attempts to join the game.

Is there any data that I can give you?

Original comment by Leonid.T...@gmail.com on 5 Jul 2012 at 8:59

[GoogleCodeExporter]

Yes. There is a BWAPI exception log in Starcraft/bwapi-data/logs/Exceptions/ if 
the folder doesn't exist then it needs to be created so that the logs write 
correctly. (You can create it then reproduce the crash)

There is also a Starcraft exception log in Starcraft/Errors/ .

It may be more helpful to provide the BWAPI exception log since it contains 
more relevant information than Starcraft's.

Because the original issue of joining was solved I have retitled this issue.

1. Does it work if you join the Local PC game manually?
2. Do you have sufficient permissions?

I've used HEAD revision on Win7_64 SP1 with Admin privileges and there is no 
crash for me.

Original comment by AHeinerm on 6 Jul 2012 at 6:18

• Changed title: auto-join Local PC game, second instance crashes on some config
• Changed state: Started

[GoogleCodeExporter]

Attached the crash log...

1. It works if I set the game manually
2. I use an admin account (I've also set "run as administrator" in 
ChaosLauncher)

P.S. Yesterday, I managed to solve it locally, but I used the 4111 revision as 
my baseline (since all my bots have already been compiled with that version, 
and I didn't want to deal with API changes all over again...)
If you are interested to see my solution, please tell me how to commit / upload 
it.

Original comment by Leonid.T...@gmail.com on 6 Jul 2012 at 9:17

Attachments:

• 2012_07_06.txt

[GoogleCodeExporter]

You could upload a patch file by right clicking on the modified files and going 
to TortoiseSVN -> Create patch...

Also the HEAD hasn't seen any API changes since r3969 (aside from an additional 
tournament module callback which has no effect on AI modules).

I think I received this exact crash but shrugged it off and didn't experience 
it a second time. I initially thought there was some kind of inconsistency with 
the crash, but there may not be any.

In Broodwar, the "Current Surface" pointer is set before a loop that iterates 
over the different drawing layers and calling their callback functions. BWAPI 
hooks 2 of the 7 layers. The crash occurs in the mouse layer callback, which is 
called after the dialog/GUI layer callback. The dialog layer hook in BWAPI is 
most likely interfering with "something", what that is I am not sure yet.

The crash occurs because something is setting this "Current Surface" pointer to 
NULL. Because there is a lot of intertwined BW and BWAPI code it's difficult to 
tell exactly where this is occurring. I will have to do some debugging in order 
to see what is setting this pointer to NULL.

Original comment by AHeinerm on 6 Jul 2012 at 9:52

[GoogleCodeExporter]

I applied my fix into the latest revision, but I'm unable to "commit" the 
changes...
Do I need to have special permissions to do that?

Original comment by Leonid.T...@gmail.com on 7 Jul 2012 at 5:00

[GoogleCodeExporter]

I just added you as a project committer, you should be able to commit now 
(after entering your google code account and key).

Original comment by AHeinerm on 7 Jul 2012 at 8:03

[GoogleCodeExporter]

Hopefully you can get this done soon, I will try to fit it into the 3.7.4 
release.

Original comment by AHeinerm on 7 Jul 2012 at 9:14

[GoogleCodeExporter]

I still can't commit and keep getting those errors:

Error: Commit failed (details follow):  
Error: Server sent unexpected return value (405 Method Not Allowed) in response 
to  
Error:  MKACTIVITY request for 
'/svn/!svn/act/858926a9-63cf-fa4e-a6d8-04bc79189baa'  

In the mean while, attaching the patch for this issue.

(Really like to get some help with this error, since I'm new to google code and 
SVN)

Original comment by Leonid.T...@gmail.com on 7 Jul 2012 at 10:03

Attachments:

• GameMenu.cpp.patch

[GoogleCodeExporter]

OK. I think I managed to commit.

Original comment by Leonid.T...@gmail.com on 7 Jul 2012 at 10:20

[GoogleCodeExporter]

I have some problems with the commit.

line 184: getInstanceNumber() == 1 would only mean the first instance can 
create. Also this value would be 0 on a non-multi-instance client, and would 
also mean you can't create more than one game. Lack of documentation on my 
part, but I also want to allow multiple games going on Local PC.

line 414: Not an important function, but you removed the ability for BWAPI to 
wait for a configurable amount of time after min_players has been reached.

lines ~420-430: Did the start game replacement code cause a problem? You 
uncommented the old code that's capable of forcefully starting the game 
regardless of any error that might be present (during download, etc).

Also I am not trying to bash your commit or anything. It's just important to 
make sure that fixing one thing doesn't break another.

Original comment by AHeinerm on 7 Jul 2012 at 10:59

[GoogleCodeExporter]

If you can make it on the IRC we can discuss it.

Original comment by AHeinerm on 7 Jul 2012 at 11:01

[GoogleCodeExporter]

Actually multiple games can't happen because of a bug in the Local PC network 
mode (Direct_IP.SNP module) and the game table appears to have a high chance of 
corruption.

The bugs are most likely related to issue 443. For some reason the second 
client downloads the same map, this could also be causing some instability 
among the clients. I've tried several things with no luck, even emulating the 
key presses still causes the map download to occur, but joining the game 
manually does not.

Original comment by AHeinerm on 8 Jul 2012 at 2:00

[GoogleCodeExporter]

I'll explain better what I was trying to do:

line 184: IMO it is important to maintain a deterministic design, where only 
one of the instances will always create a game, and all the other will join it. 
Otherwise it can easily create a race condition, where both instances try to 
create a new game and no one joins it. (Easily reproduced after the automatic 
restart of the match in auto_restart = true).
So, I think you should reconsider setting getInstanceNumber as 1 for the first 
instance, even when there is only one instance in the game.

line 414: You are correct. At first, I used that timer as the "wait for 
download" timer, but later It seemed to me that this feature doesn't work 
anyway (and I couldn't figure out when it might be needed). I can restore it, 
for future use.

lines 420-430: These lines were added somewhere after 4111. To my 
understanding, these lines just press the "OK" button when it becomes 
available. It might cause the instance to start a game, even if the other 
didn't download the map correctly. So I decided to comment this, to avoid such 
uncertain behavior. Was I wrong about understanding those lines?

Original comment by Leonid.T...@gmail.com on 8 Jul 2012 at 5:40

[GoogleCodeExporter]

The most important line in the fix is 414. It means that the host can't start a 
game until 14 sec have passed (the number was chosen empirically).

Line 184 solves a different bug with auto restart (that also didn't work for me)

Original comment by Leonid.T...@gmail.com on 8 Jul 2012 at 5:55

[GoogleCodeExporter]

Since the original issue is fixed, made a new issue concerning the auto-menu 
instability.

Original comment by AHeinerm on 12 Jul 2012 at 6:44

• Changed state: Duplicate