- User Since
- Aug 4 2019, 2:04 AM (60 w, 2 d)
Sat, Sep 12
Just updated the code again to move the ownership of the message in SendGameMessage to the member variable. Hopefully this addresses the outstanding issue.
- Move game message ownership correctly
- Update SendGameMessage to rvalue ref. Fix char types
Mon, Sep 7
- Add newline (and reorder)
- Formatting fixes, code cleanup
Thanks for the review! I am pushing an updated version now.
Fri, Sep 4
- Add RL namespace
@vladislavbelov, @Itms - I still need to add a namespace or prefix the more general names (such as GameMessageType) but otherwise this should be ready for review! Let me know if there are any other questions/comments!
- add missing "new"
Aug 25 2020
Switch to smart pointer for g_RLInterface and use rvalue refs to make ownership more clear in SendGameMessage. Clean up RLInterface
Aug 22 2020
I have made most of the changes locally and will be updating the revision soon! I also added a couple inline responses to some of the questions.
Aug 12 2020
- Small refactor of game loop when nonvisual
I removed the mod installation from RunRLServer then realized it was pretty easy to decrease the code duplication from that method (as @vladislavbelov suggested over irc, iirc). Removing the code duplication resulted in keeping the mod installation logic. I am happy to revert if that is preferred; let me know what you think!
I just took another pass at it and removed the duplicate logic from RunRLServer. (I believe @vladislavbelov mentioned this over IRC.) I still need to test it when running the GUI but thought I would push it anyway to get feedback. As this reuses more code, it does still include mod installation but I am open to changing it, if preferred!
- Merge RunRLServer logic into main loop
Yeah, that makes sense. It is probably also unnecessary for the containerized case since you can just mount the files into the container and the environment can still be spun up with a single command.
Aug 11 2020
Makes sense. I admit that I had been thinking that there was no need to change most of the initial startup for the game when running the RL interface so I just left it in.
I have addressed the concerns in D2947. Let me know what you think!
- Add more const's
Aug 10 2020
Address some of Stan's feedback
- Add extra const's
- Use reference in for loop
@Itms, Yeah, I will check those issues out and make a new revision!
Aug 4 2020
Adding the missing include fixes the issue for me. Good catch, @Nescio about the unused includes - those were left over from an earlier version that I forgot to remove :/
Jul 31 2020
Fixed minor style issues
Fixed seg fault due to user exiting the game from the UI when the script is using it, too.
Jul 30 2020
Style fixes mostly from the last round of feedback. I am still digging into the seg fault from earlier. I had a couple questions about some of the earlier comments that I made inline and will update the code after I have a clearer idea of the approach to take!
Updated the code following the review from Itms. I still need to check out the seg fault but everything else should be addressed!
Jul 24 2020
Jul 18 2020
Remove regex, boost parsing. Still running into linking errors so need more testing but wanted to push these updates regardless to get feedback and see how it does in jenkins. (I suspect that the linking issues are specific to my machine...)
Jul 11 2020
After rebasing the changes, I have been seeing the following linking error:
Fix issues in main.cpp and rebased (using the github mirror).
I have made most of the changes. To my knowledge, the only remaining issues are:
- Update GetGameState to use GetEntitiesWithInterface (as described in the earlier comment). I asked a question about this in the earlier comment.
- Use json_spirit to make it less std::string heavy.
Jun 29 2020
Removed sdl linking from premake (only removed the include last time!)
Jun 27 2020
GetTemplates is now const and removed unused include
Jun 24 2020
Miscellaneous code cleanup. Still need to address the GetGameState comments and the use of jsonspirit rather than strings by @wraitii.
Made the miscellaneous code fixes. Pushing them momentarily!
Jun 21 2020
Revert extern_libs5 file.
Fixed various linting/style issues. Rebased to the latest version of 0 AD.
I have made most of the changes and will be updating the revision soon (arcanist is hanging a bit after the rebase). I still need to update the GetGameState function and make it a little less string heavy as mentioned by @wraitii but most other changes should have been applied.
Jun 6 2020
Removed some grpc includes that I missed
Remove old build option, reorder python args, and minor update to the python readme. Python arguments for reset were reordered from reset(config, player_id=1, save_replay=False) to reset(config, save_replay=False, player_id=1) as I suspect it will be more common for users to want to save the replay rather than change the player's ID.
Remove empty requirements.txt file
Updated the diff to use mongoose instead of gRPC and cleaned up python API.
May 22 2020
Thanks, @wraitii ! I am happy to have a review (and someone else who shares my excitement for this! :)
Apr 25 2020
Fixed remaining call to to_proto (from old ScenarioConfig logic).
Apr 12 2020
Remove programmatic creation of scenario config/game setup JSON and updated the example.
Feb 8 2020
Added --with-rlinterface to the script for building libraries on OSX (only built fiber if flag exists). Removed a condition which is always false (checking if on windows with the --with-rlinterface flag since it throws an unsupported error message earlier if so).
Jan 27 2020
Added support for OSX (required updates to script for building libraries and premake).
Dec 22 2019
This adds a couple dependencies when building with the --with-rlinterface (passed to ./update-workspaces.sh):
- GRPC (C++ and Python)
- Python support requires grpcio-tools package
- boost fiber
- boost system
Added error when building on unsupported platform (windows). (@Stan mentioned in IRC that windows support was not necessary for merging.)
Dec 5 2019
Updated python dependencies and fixed bug when built w/o the RL interface.
Nov 12 2019
Updated installation/testing instructions in README for the python client and removed the user option for the RL interface address as it must be set on start and may be confusing for users.
Added tests for actions and configuring scenarios. I also removed the extra example since it didn't contain anything new as compared to the better one ;)
Nov 11 2019
As an update, I am currently working on adding tests for the actions and scenario config (using pytest). I have tests for all the actions in zero_ad/actions.py and a number of the different scenario settings. I am currently working on testing random map usage. After finishing the tests, I am planning on training a simple agent as an example demonstrating how it can be used to expose an OpenAI Gym API to 0 AD :) (I will probably do this in an external repo and then just post a link to it.)
Oct 24 2019
Miscellaneous code cleanup per Stan's feedback.
Oct 23 2019
- Define ScenarioConfig in Python. Added basic helpers for defining victory conditions as well as more generic setters (set_map_setting, set_game_setting).
- Refactored RL Server main loop to own method
- Changed default build to not include RL interface (uses --with-rlinterface)
Sep 14 2019
Added more actions (construct, gather, train, attack-walk) and an example script showing a lot of these basic actions.
Aug 24 2019
Aug 23 2019
- Added --without-rlinterface compile flag
- Removed generated files
- Added config option for the rpc server address (both to the default.cfg and to the options.json used in-game)
Aug 22 2019
Cleaned up the updates to the SaveReplayMetadata function (mentioned in the prior reviews). I also made the argument for the fn const since it is read-only.
I will be updating the diff with the most recent fixes. Dumb question: when I update the diff, does it add the changes to the existing diff or overwrite it with the most recent diff? I usually use git and GitHub and am still getting used to Phabricator...
Aug 21 2019
- Removed duplicate functions for getReplayMetadata to more directly call the fn from the GuiInterface.
- Updated VisualReplay::SaveReplayMetadata to call GuiInterface directly from the C++. After the code change, I removed the (now unused) ScriptInterface argument from the method signature.
- Replaced the single invocation of getReplayMetadata with Engine.GuiInterfaceCall("GetReplayMetadata")
- Added the registration of GetInitAttributes which should have been included in the original diff.
- Added myself to the credits
Thanks for the feedback! I added some responses inline and will be updating the diff (hopefully addressing all the remaining issues)!
As @Stan said, there is more discussion on the PR on GitHub. The goal of this contribution is to provide the fundamental underlying capabilities for exploring RL/ML agents in 0 AD (such as an OpenAI gym environment which can actually be implemented on top of this quite easily). I am intentionally not adding any action/observation spaces or reward as expected by OpenAI gym as these are not obvious and there are many different candidates for each of these (this is discussed more on the GitHub thread).
Thanks for the feedback! I can move it to be behind a feature flag and remove the generated files.
Aug 20 2019
Changed NULL to nullptr
I added GetReplayMetadata to the GuiInterface component to reduce code duplication introduced by the original diff.
Thanks for the feedback - I have added my responses inline!