-
Notifications
You must be signed in to change notification settings - Fork 43
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
Bugfixes to Tournament Module permissions for BWAPI versions 4.2.0 & 4.1.2 #32
Comments
For BWAPI versions 4.2.0 and 4.1.2 only, fix an argument type in the onAction function, but allow (only) the permissions for LeaveGame, SetLatCom, SetCommandOptimizationLevel, even if overriding the command optimization level to a value of 0 (default is still 1 though so no worries)
Move source source files into a "Source" subfolder. Content hasn't changed.
Add missing official version of vcxproj files
Add some source files to the vcxproj file, and look up the BWAPI_DIR environment variable to find the location of BWAPI's include and lib dependencies.
…and 4.1.2 Fixed TournamentModule.dll. No other changes. Notes: compiled using the official downloads of BWAPI without modifications/rebuilding BWAPI. For BWAPI 4.1.2 it was compiled using Microsoft Visual Studio Express 2013 for Windows Desktop version 12.0.31101.00 Update 4. For BWAPI 4.2.0 it was compiled using Microsoft Visual Studio Community 2017 version 15.7.4 (linking with an unpacked https://github.com/bwapi/bwapi/releases/download/v4.2.0/BWAPI.VS.15.7.3.7z).
I've fixed it for both points, tested the new TournamentModule.dll files on my Win7 SP1 VMs (I modified the latest version & an old version of my bot to try using all the various permissions for the bugged TournamentModule.dll files for BWAPI 4.2.0 and 4.1.2 and confirmed that all permissions are allowed, then tested against the fixed TournamentModule.dll files for BWAPI 4.2.0 and 4.1.2 and verified the resulting behavior is as expected), and committed and pushed all the changes to the ladder branch of my fork, but only for BWAPI versions 4.2.0 and 4.1.2. I have not fixed it for BWAPI version 4.0.1 Beta (nor changed it for BWAPI version 3.7.4) because I don't think it is worth bothering and because I don't have the old compiler versions nor builds of old BWAPI versions set up (which would be a lot of hassle). |
Note: BASIL's Tournament Module (https://github.com/Bytekeeper/sc-tm) allows and disallows the same permissions as my new version except that BASIL also allows sendText/printf/setTextSize and has a config setting for the UserInput flag that defaults to allowing it. StarcraftAITournamentManager disallows them for BWAPI 3, so to avoid rocking the boat, I decided not to allow them in my changes. I know SSCAIT allows leaveGame, sendText, and printf at least, and I gather it also allows setLatCom(false). I don't know whether it allows setCommandOptimizationLevel(0 or other values). Apart from that, I have no idea what SSCAIT allows and disallows because I don't think the source code is available (or if it is, at least not the up-to-date source code). I also have no idea what the defaults are for things like isLatComEnabled(), or what the command optimization level is initially. |
Fix from @chriscoxe merged into ladder branch. |
Re. 4.0.1 Beta, the AIIDE Rules page and the CIG Rules page don't mention 4.0.1 Beta, so no worries. |
From #31 points 7 then 2. I.E.:
#31 point 7 is that there is a bug in the Tournament Module that causes it to allows all permissions for BWAPI version 4 (e.g. 4.2.0, 4.1.2, 4.0.1 Beta)!
Due to a change to a BWAPI function's argument type as part of the first release of version 4 that was not correspondingly migrated into the Tournament Module (i.e. the TournamentModule::onAction function's first argument type was changed from an into to a BWAPI::Tournament::ActionID), permissions like enableFlag(CompleteMapInformation), setLocalSpeed(42) are not enforced by TournamentModule.dll, because the default implementation of the function that is defined in BWAPI is being used, which always returns true. This is why the Sling bot was able to successfully do setLocalSpeed(42) and setFrameSkip(0) until Sling's source code was patched, and the Tyr/TyrProtoss bot was able to successfully do setLocalSpeed(10) in the AIIDE and CIG competitions, which slowed down the runs. If any bot that uses BWAPI 4 had cheated by doing things like enableFlag(CompleteMapInformation), the tournament module wouldn't have stopped them. Note that this is true for non-DLL bots and for DLL bots (originally I thought it only affected non-DLL bots, but after investigating and finding the cause of the bug I realized it affects DLL bots too).
#31 point 2 should be addressed after point 7 is fixed. I.E. ensure the Tournament Module allows some permissions for some versions of BWAPI.
After the bug in point 7 is fixed, rebuild the TournamentModule.dll's (for BWAPI 4.2.0 and 4.1.2 at least - I wouldn't bother fixing 4.0.1 Beta, and I wouldn't bother changing 3.7.4 which doesn't allow any permissions) to allow bots to do setLatCom(false) and setCommandOptimizationLevel(a value other than 1, i.e. especially 0 or 2, but perhaps some other authors might want 3 or 4). They don't affect the opponent bot at all - it only affects what info BWAPI returns to you, and some bots rely on them. The default should probably stay as 1, but I think bots should be allowed to set it to 0 and other values if they want to, even though a value of 0 may increase the bot's APM and replay file sizes, because some bots rely on them and currently there is no way for a bot to check what the command optimization level is. And anyway, currently, BWAPI version 4.2.0/4.1.2/4.0.1 Beta bots have always done this until now, due to the bug, so I don't think it's a problem. I suggest to also allow leaveGame(), so that bots can leave the game to indicate they are resigning/surrendering in lost positions, or in bugged states to attempt to avoid crashing later (and this may help address bwapi/bwapi#719
). This might also help increase the overall game rate, so long as Starcraft or bots don't/rarely crash when leaving, because bot-vs-bot endgames can often take a lot of time if the winning bot is weak/slow. Personally, I don't care whether sendText/printf/setTextSize are allowed, so I would probably disallow them on anti-collusion grounds and in case they help to increase performance, but I think the others should be disallowed.
The text was updated successfully, but these errors were encountered: