Changeset View
Standalone View
source/rlinterface/RLInterface.cpp
- This file was added.
/* Copyright (C) 2019 Wildfire Games. | |||||
* This file is part of 0 A.D. | |||||
* | |||||
* 0 A.D. is free software: you can redistribute it and/or modify | |||||
* it under the terms of the GNU General Public License as published by | |||||
* the Free Software Foundation, either version 2 of the License, or | |||||
* (at your option) any later version. | |||||
* | |||||
* 0 A.D. is distributed in the hope that it will be useful, | |||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | |||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |||||
* GNU General Public License for more details. | |||||
* | |||||
* You should have received a copy of the GNU General Public License | |||||
* along with 0 A.D. If not, see <http://www.gnu.org/licenses/>. | |||||
*/ | |||||
#include "rlinterface/RLInterface.h" | |||||
#include "simulation2/system/LocalTurnManager.h" | |||||
using grpc::ServerContext; | |||||
using boost::fibers::channel_op_status; | |||||
grpc::Status RLInterface::Step(ServerContext* UNUSED(context), const Actions* commands, Observation* obs) | |||||
{ | |||||
std::lock_guard<std::mutex> lock(m_lock); | |||||
// Interactions with the game engine (g_Game) must be done in the main | |||||
// thread as there are specific checks for this. We will pass our commands | |||||
// to the main thread to be applied | |||||
GameMessage msg = { GameMessageType::Commands }; | |||||
const int size = commands->actions_size(); | |||||
for (int i = 0; i < size; i++) | |||||
{ | |||||
Stan: range based for loop ? | |||||
Not Done Inline ActionsI don't think it is possible since this is using the autogenerated cpp files from the protobuf (it uses commands->actions(i) rather than commands->actions[i] so the "array" of actions is actually a functions which deserializes the given action from an index). Otherwise, I would definitely prefer it! irishninja: I don't think it is possible since this is using the autogenerated cpp files from the protobuf… | |||||
Not Done Inline ActionsAh right, I always suggest it, then look after at the code XD Stan: Ah right, I always suggest it, then look //after //at the code XD | |||||
Done Inline Actionshaha, no worries :) irishninja: haha, no worries :) | |||||
std::string json_cmd = commands->actions(i).content(); | |||||
player_id_t playerid = commands->actions(i).playerid(); | |||||
msg.data.push(std::make_tuple(playerid, json_cmd)); | |||||
Done Inline ActionsI think we have a specific type for player ids player_id_t or something. Stan: I think we have a specific type for player ids player_id_t or something. | |||||
} | |||||
m_GameMessages.push(msg); | |||||
std::string state; | |||||
m_GameStates.pop(state); | |||||
obs->set_content(state); | |||||
return grpc::Status::OK; | |||||
} | |||||
grpc::Status RLInterface::Reset(ServerContext* UNUSED(context), const ResetRequest* req, Observation* obs) | |||||
{ | |||||
std::lock_guard<std::mutex> lock(m_lock); | |||||
if (req->has_scenario()) | |||||
m_ScenarioConfig = req->scenario(); | |||||
Done Inline ActionsNo braces for single line statements see https://trac.wildfiregames.com/wiki/Coding_Conventions Stan: No braces for single line statements see https://trac.wildfiregames.com/wiki/Coding_Conventions | |||||
Done Inline ActionsNot done :) Stan: Not done :) | |||||
Done Inline ActionsMy bad. I will fix this :) irishninja: My bad. I will fix this :) | |||||
struct GameMessage msg = { GameMessageType::Reset }; | |||||
m_GameMessages.push(msg); | |||||
std::string state; | |||||
Not Done Inline Actionsconst method? Stan: const method? | |||||
Not Done Inline Actions? Stan: ? | |||||
Done Inline ActionsUnfortunately, GetTemplates cannot be const since it acquires m_lock :/ irishninja: Unfortunately, `GetTemplates` cannot be const since it acquires `m_lock` :/ | |||||
Not Done Inline ActionsYou could make the lock mutable. This is a common well defined pattern exactly for mutexes. See https://en.cppreference.com/w/cpp/language/cv wraitii: You could make the lock mutable. This is a common well defined pattern exactly for mutexes. See… | |||||
m_GameStates.pop(state); | |||||
obs->set_content(state); | |||||
return grpc::Status::OK; | |||||
} | |||||
grpc::Status RLInterface::GetTemplates(ServerContext* UNUSED(context), const GetTemplateRequest* req, Templates* res) | |||||
Not Done Inline Actionsconst &? Stan: const &? | |||||
Done Inline Actionsconst ? Stan: const ? | |||||
{ | |||||
std::lock_guard<std::mutex> lock(m_lock); | |||||
if (!g_Game) | |||||
{ | |||||
LOGERROR("Game not running. Have you started a scenario with a Reset message?"); | |||||
return grpc::Status(grpc::StatusCode::FAILED_PRECONDITION, "Game not running."); | |||||
} | |||||
Done Inline ActionsError ? Stan: Error ? | |||||
CSimulation2& simulation = *g_Game->GetSimulation2(); | |||||
CmpPtr<ICmpTemplateManager> cmpTemplateManager(simulation.GetSimContext().GetSystemEntity()); | |||||
Done Inline Actionsavoid auto when you can :) Stan: avoid auto when you can :) | |||||
const int size = req->names_size(); | |||||
for (int i = 0; i < size; i++) | |||||
{ | |||||
const std::string templateName = req->names(i); | |||||
Done Inline Actionsrange based for loop ? Stan: range based for loop ? | |||||
const CParamNode* node = cmpTemplateManager->GetTemplate(templateName); | |||||
Template* tpl = res->add_templates(); | |||||
tpl->set_name(templateName); | |||||
if (node != nullptr) | |||||
{ | |||||
std::string content = utf8_from_wstring(node->ToXML()); | |||||
Done Inline Actionsnullptr Stan: nullptr | |||||
tpl->set_content(content); | |||||
} | |||||
} | |||||
return grpc::Status::OK; | |||||
} | |||||
void RLInterface::Listen(std::string server_address) | |||||
{ | |||||
grpc::ServerBuilder builder; | |||||
builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); | |||||
builder.RegisterService(this); | |||||
m_Server = builder.BuildAndStart(); | |||||
} | |||||
void RLInterface::ApplyEvents() | |||||
{ | |||||
const bool nonVisual = !g_GUI; | |||||
Done Inline ActionsDoxygen. Stan: Doxygen. | |||||
const bool isGameStarted = g_Game && g_Game->IsGameStarted(); | |||||
if (m_NeedsGameState && isGameStarted) | |||||
{ | |||||
m_GameStates.push(GetGameState()); // Send the game state back to the request | |||||
m_NeedsGameState = false; | |||||
} | |||||
GameMessage msg; | |||||
while (m_GameMessages.try_pop(msg) == channel_op_status::success) | |||||
{ | |||||
switch (msg.type) | |||||
{ | |||||
case GameMessageType::Reset: | |||||
{ | |||||
if (isGameStarted) | |||||
Done Inline ActionsThis does not follow our conventions for switch, see https://trac.wildfiregames.com/wiki/Coding_Conventions wraitii: This does not follow our conventions for `switch`, see https://trac.wildfiregames. | |||||
EndGame(); | |||||
Done Inline Actionswe have a weird convention for this. -1 indent. Stan: we have a weird convention for this. -1 indent. | |||||
g_Game = new CGame(nonVisual, m_ScenarioConfig.savereplay()); | |||||
ScriptInterface& scriptInterface = g_Game->GetSimulation2()->GetScriptInterface(); | |||||
Done Inline Actionsbraces Stan: braces | |||||
JSContext* cx = scriptInterface.GetContext(); | |||||
JS::RootedValue attrs(cx); | |||||
Done Inline ActionsMight need JSAutoRequest pagerq(pcx); @elexis might know. Stan: Might need JSAutoRequest pagerq(pcx); @elexis might know. | |||||
scriptInterface.ParseJSON(m_ScenarioConfig.content(), &attrs); | |||||
g_Game->SetPlayerID(m_ScenarioConfig.playerid()); | |||||
g_Game->StartGame(&attrs, ""); | |||||
if (nonVisual) | |||||
{ | |||||
LDR_NonprogressiveLoad(); | |||||
ENSURE(g_Game->ReallyStartGame() == PSRETURN_OK); | |||||
m_GameStates.push(GetGameState()); // Send the game state back to the request | |||||
} | |||||
else | |||||
Done Inline ActionsComments on top missing final . Stan: Comments on top missing final . | |||||
{ | |||||
JS::RootedValue initData(cx); | |||||
scriptInterface.CreateObject(&initData); | |||||
scriptInterface.SetProperty(initData, "attribs", attrs); | |||||
JS::RootedValue playerAssignments(cx); | |||||
scriptInterface.CreateObject(&playerAssignments); | |||||
scriptInterface.SetProperty(initData, "playerAssignments", playerAssignments); | |||||
Done Inline ActionsStan: Avoid eval calls, we have a way to make empty objects @elexis or @Itms will tell you more. | |||||
Done Inline Actionsconst & ? Stan: const & ? | |||||
g_GUI->SwitchPage(L"page_loading.xml", &scriptInterface, initData); | |||||
m_NeedsGameState = true; | |||||
} | |||||
} | |||||
break; | |||||
case GameMessageType::Commands: | |||||
if (!isGameStarted) | |||||
{ | |||||
LOGWARNING("Cannot apply game commands w/o running game. Ignoring..."); | |||||
continue; | |||||
} | |||||
const ScriptInterface& scriptInterface = g_Game->GetSimulation2()->GetScriptInterface(); | |||||
CLocalTurnManager* turnMgr = static_cast<CLocalTurnManager*>(g_Game->GetTurnManager()); | |||||
while (msg.data.size() > 0) | |||||
{ | |||||
int playerid = std::get<0>(msg.data.front()); | |||||
Done Inline Actions. Stan: . | |||||
std::string json_cmd = std::get<1>(msg.data.front()); | |||||
msg.data.pop(); | |||||
Not Done Inline Actionsthis is probably not useful, I needed it during debugging. wraitii: this is probably not useful, I needed it during debugging. | |||||
JSContext* cx = scriptInterface.GetContext(); | |||||
JS::RootedValue command(cx); | |||||
scriptInterface.ParseJSON(json_cmd, &command); | |||||
turnMgr->PostCommand(playerid, command); | |||||
} | |||||
const double deltaRealTime = DEFAULT_TURN_LENGTH_SP; | |||||
if (nonVisual) | |||||
{ | |||||
const double deltaSimTime = deltaRealTime * g_Game->GetSimRate(); | |||||
Done Inline Actions. Stan: . | |||||
Not Done Inline Actionsno braces ? Itms: no braces ? | |||||
size_t maxTurns = static_cast<size_t>(g_Game->GetSimRate()); | |||||
g_Game->GetTurnManager()->Update(deltaSimTime, maxTurns); | |||||
} | |||||
else | |||||
g_Game->Update(deltaRealTime); | |||||
m_GameStates.push(GetGameState()); | |||||
break; | |||||
Done Inline ActionsI am guessing I should also remove the curly braces here, too. Right? irishninja: I am guessing I should also remove the curly braces here, too. Right? | |||||
Done Inline ActionsYup Stan: Yup | |||||
} | |||||
Done Inline Actionsuse cpp style casts like you did above. Stan: use cpp style casts like you did above. | |||||
} | |||||
} | |||||
std::string RLInterface::GetGameState() | |||||
{ | |||||
Done Inline Actionsnullptr here and below I guess Stan: nullptr here and below I guess | |||||
const ScriptInterface& scriptInterface = g_Game->GetSimulation2()->GetScriptInterface(); | |||||
const auto simContext = g_Game->GetSimulation2()->GetSimContext(); | |||||
CmpPtr<ICmpAIInterface> cmpAIInterface(simContext.GetSystemEntity()); | |||||
JSContext* cx = scriptInterface.GetContext(); | |||||
JS::RootedValue state(cx); | |||||
cmpAIInterface->GetFullRepresentation(&state, true); | |||||
return scriptInterface.StringifyJSON(&state, false); | |||||
} | |||||
Done Inline Actionssame here about auto. Stan: same here about auto. | |||||
Done Inline ActionsAutorequest here to. Stan: Autorequest here to. | |||||
Not Done Inline ActionsDepending on the AI interface for this isn't great. You can probably mimic the function from C++ however, by calling Simulation2's GetEntitiesWithInterface, and then making GUIInterfaceCalls using the ComponentManager. wraitii: Depending on the AI interface for this isn't great.
You can probably mimic the function from… | |||||
Done Inline ActionsWould this approach still work when running headlessly? irishninja: Would this approach still work when running headlessly? | |||||
Not Done Inline ActionsI believe it will, yes. Why do you think it might not work? Perhaps there's something more I can explain about how our simulation works :) wraitii: I believe it will, yes.
Why do you think it might not work? Perhaps there's something more I… | |||||
Not Done Inline ActionsI had assumed that GUIInterfaceCalls required a GUI to be present. I don't fully understand how it all fits together just yet :/ After looking at the code a little more, I see an example where script calls are made from the GuiInterface for getting the replay metadata. Is this what you mean by GuiInterfaceCalls? It also looks like that example uses CmpPtr rather than GetEntitiesWithInterface and I don't quite understand how these are meant to be combined. I had been getting the AI interface but it seems here that I need entities with the AI interface (since I am going to be calling "GetFullRepresentation") but they also need to have the GuiInterface (since I need to make ScriptCalls). I am sure I am misunderstanding but if you could provide some guidance or clarity, it would be much appreciated :) As an example of what I have been playing around with. The following code fails as expected but might help illustrate how I understand the suggestion (incorrect but maybe it will be helpful): CSimulation2* sim = g_Game->GetSimulation2(); CSimulation2::InterfaceList ents = sim->GetEntitiesWithInterface(IID_GuiInterface); entity_id_t ent = ents.front().first; const CSimContext simContext = g_Game->GetSimulation2()->GetSimContext(); CmpPtr<ICmpGuiInterface> cmpGuiInterface(simContext, ent); ScriptInterface& scriptInterface = sim->GetScriptInterface(); JSContext* cx = scriptInterface.GetContext(); JSAutoRequest rq(cx); JS::RootedValue arg(cx); JS::RootedValue state(cx); cmpGuiInterface->ScriptCall(INVALID_PLAYER, L"GetFullRepresentation", arg, &state); // This fails as the fn is only defined for the AI interface. Switching to use the AIInterface fails to compile since "ScriptCall" is not defined for ICmpAIInterface. return scriptInterface.StringifyJSON(&state, false); irishninja: I had assumed that GUIInterfaceCalls required a GUI to be present. I don't fully understand how… | |||||
Not Done Inline Actions
Indeed it doesn't. When you call something in the GuiInterface, you're calling the simulation. Components can also be "system" or local to an entity. The GuiInterface is a "system" component. Your (understandable) mistake was thinking that each entity has a GuiInterface like they have an AIProxy. AIInterface is also a system component, by the way. CmpPtr is just a C++ wrapper to a component pointer. So to recap, what you need to do is something like this (compilation not guaranteed): CSimulation2* sim = g_Game->GetSimulation2(); // Here I fetch a list of all entities in the game with an IID_AIProxy. CSimulation2::InterfaceList ents = sim->GetEntitiesWithInterface(IID_AIProxy); // Query the GUI Interface of the system. CmpPtr<ICmpGuiInterface> cmpGuiInterface(sim, SYSTEM_ENTITY); ScriptInterface& scriptInterface = sim->GetScriptInterface(); JSContext* cx = scriptInterface.GetContext(); JSAutoRequest rq(cx); for (entity_id_t ent : ents) { // this is per-entity CmpPtr<ICmpAIProxy> cmpAIProxy(sim, ent); std::string test = cmpAIProxy->GetFullRepresentation(); } cmpGuiInterface->ScriptCall(INVALID_PLAYER, L"GetSimulationState", arg, &state); return scriptInterface.StringifyJSON(&state, false); The thing is this still relies on AIProxy, so it's kind of the same as using AIInterface... And it need s aC++ AI Proxy component :/ So to be honest I'm not sure it's much better than what you have now, after all. wraitii: > I had assumed that GUIInterfaceCalls required a GUI to be present. I don't fully understand… | |||||
Not Done Inline ActionsMissing jsautorequest here and below? @wraitii might know more Stan: Missing jsautorequest here and below? @wraitii might know more | |||||
Not Done Inline ActionsProbably yes. wraitii: Probably yes. | |||||
Done Inline Actionsnullptr Stan: nullptr | |||||
Done Inline Actionsno braces Stan: no braces | |||||
Done Inline ActionsJSAutoRequest Itms: `JSAutoRequest` | |||||
Not Done Inline ActionsWhy did you add braces to the body of this case? They seem unneeded with the current code. Itms: Why did you add braces to the body of this `case`? They seem unneeded with the current code. | |||||
Done Inline ActionsWithout the braces, I am getting redeclaration errors and a "jump to case label" error. Is there a better way I should be handling this? irishninja: Without the braces, I am getting redeclaration errors and a "jump to case label" error. Is… | |||||
Done Inline ActionsOh sorry I always forget that the compiler doesn't understand that break avoids the collision. The code is good then. Just for the style, maybe put the break; inside the braces, and add the same braces to the next case, in case new cases are added in the future. Itms: Oh sorry I always forget that the compiler doesn't understand that `break` avoids the collision. | |||||
Done Inline ActionsJSAutoRequest Itms: `JSAutoRequest` | |||||
Done Inline ActionsJSAutoRequest Itms: `JSAutoRequest` | |||||
Not Done Inline Actionsno braces here either Itms: no braces here either | |||||
Not Done Inline Actionsand there Itms: and there |
range based for loop ?