Page MenuHomeWildfire Games
Differential D2947

Address concerns in rP23917
ClosedPublic
Actions

Authored by irishninja on Aug 10 2020, 6:44 PM.

Details

Reviewers
Itms
vladislavbelov
Commits
rP24631: Address concerns in rP23917 (RL interface)
Summary

This revision is addressing concerns raised by @vladislavbelov from rP23917.

There might be a better name for RLGameCommand but it should at least avoid collisions.

Also, I added a comment explaining the RL Interface at a high level. Let me know if you would like comments for individual methods, more details in the comment, etc.

Test Plan

The existing sample should still work and it should still compile.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
irishninja added inline comments.Sep 7 2020, 2:20 AM
source/main.cpp
661 ↗(On Diff #13400)

In the visual case, the game messages are applied in the Frame function similar to when the game simulation step is called when running w/o --rl-interface (should be line 397)

source/rlinterface/RLInterface.cpp
40 ↗(On Diff #13400)

Yeah, that would've been a good idea to make it cleaner. Updated now.

source/rlinterface/RLInterface.h
27 ↗(On Diff #13400)

I moved this to source/ps/GameSetup/GameSetup.h

irishninja updated this revision to Diff 13445.Sep 7 2020, 2:20 AM
irishninja marked an inline comment as done.
  • Formatting fixes, code cleanup
Vulcan added a comment.Sep 7 2020, 2:28 AM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1519/display/redirect

Harbormaster completed remote builds in B13136: Diff 13445.Sep 7 2020, 2:37 AM
Stan added inline comments.Sep 7 2020, 7:35 AM
source/ps/GameSetup/GameSetup.h
28 ↗(On Diff #13445)

Add newline between the two

irishninja updated this revision to Diff 13446.Sep 7 2020, 4:44 PM
  • Add newline (and reorder)
Vulcan added a comment.Sep 7 2020, 4:57 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1520/display/redirect

Harbormaster completed remote builds in B13137: Diff 13446.Sep 7 2020, 6:05 PM
vladislavbelov added inline comments.Sep 12 2020, 12:55 PM
source/rlinterface/RLInterface.cpp
50 ↗(On Diff #13446)

It should be solved.

141 ↗(On Diff #13446)

What's happening if the val is incorrect integer?

142 ↗(On Diff #13446)

Incorrect type of std::unique_ptr<char>, it should be std::unique_ptr<char[]>.

source/rlinterface/RLInterface.h
47 ↗(On Diff #13446)

Braces on new line.

68 ↗(On Diff #13178)

It's unclear where does it get the message from.

irishninja updated this revision to Diff 13484.Sep 12 2020, 5:46 PM
irishninja marked 2 inline comments as done.
  • Update SendGameMessage to rvalue ref. Fix char types
irishninja added inline comments.Sep 12 2020, 5:47 PM
source/rlinterface/RLInterface.cpp
50 ↗(On Diff #13446)

My bad. I forgot to make it an rvalue ref here, too.

141 ↗(On Diff #13446)

Hmm... I was assuming that it was a properly formed HTTP request. Would you like me to be more pessimistic about malformed HTTP requests?

So far, I haven't worried too much about it since "Content-Length" is often handled by http request libraries and the RL interface requires both a flag to be set and then only accepts requests from localhost (unless you change the address to 0.0.0.0). I also figured that there wasn't much motivation for malicious agents to attack the game via the RL interface as it is likely being used for development of an AI rather than a hosting the game engine as a server of sorts. (In that context, I had actually imagined there would still be an orchestrator of sorts collecting and applying the commands as all actions at a given step are part of a single call to Step - and it likely wouldn't make sense for both players to control the stepping of the game arbitrarily.)

Regardless, I can certainly update it if preferred. Let me know what you think!

Vulcan added a comment.Sep 12 2020, 5:52 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1545/display/redirect

Harbormaster completed remote builds in B13175: Diff 13484.Sep 12 2020, 5:59 PM
vladislavbelov added inline comments.Sep 12 2020, 10:03 PM
source/rlinterface/RLInterface.cpp
47 ↗(On Diff #13484)

r-value doesn't make things easier here.

50 ↗(On Diff #13484)

The issue is still here. You're storying reference to a local variable.

irishninja updated this revision to Diff 13485.Sep 12 2020, 10:46 PM
  • Move game message ownership correctly
irishninja added a comment.Sep 12 2020, 10:47 PM

Just updated the code again to move the ownership of the message in SendGameMessage to the member variable. Hopefully this addresses the outstanding issue.

Vulcan added a comment.Sep 12 2020, 10:52 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1546/display/redirect

Harbormaster completed remote builds in B13176: Diff 13485.Sep 12 2020, 10:56 PM
wraitii updated this revision to Diff 13761.Nov 6 2020, 4:12 PM
wraitii added a subscriber: wraitii.

Minor tweaks + add comments (I didn't know if you were available initially).

I think this looks good but I'll ping Vladislav as well.

wraitii added inline comments.Nov 6 2020, 4:14 PM
source/rlinterface/RLInterface.cpp
141 ↗(On Diff #13446)

atoi will return 0 if it can't be converted, which ought to be fairly safe. I think we have to trust mongoose here.

source/rlinterface/RLInterface.h
22 ↗(On Diff #13761)

I've added this back because it makes it possible to put the mongoose callback in the class, thus cleaning up the private/public interface split. I don't think it's a huge issue since the header included in two c++ files only.

Vulcan added a comment.Nov 6 2020, 4:16 PM

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-gcc6.txt
g++: internal compiler error: Segmentation fault (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[1]: *** [obj/gui_Debug/L10n.o] Error 4
make: *** [gui] Error 2

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3410/display/redirect

Vulcan added a comment.Nov 6 2020, 4:18 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1758/display/redirect

Harbormaster completed remote builds in B13477: Diff 13761.Nov 6 2020, 4:18 PM
wraitii added inline comments.Nov 6 2020, 4:19 PM
source/rlinterface/RLInterface.h
87 ↗(On Diff #13761)

Shouldn't this just be done in the constructor? It seems enabling the RL interface without actually enabling the HTTP will just prevent you from playing.

93 ↗(On Diff #13761)

Your call, but this might be better named ProcessPendingMessage or PollMessage perhaps( to parallel the SDL convention of SDL_PollEvent).

irishninja marked 4 inline comments as done.Nov 10 2020, 3:27 AM
irishninja added inline comments.
source/rlinterface/RLInterface.h
87 ↗(On Diff #13761)

Yeah, I am happy to do that. I was following the convention from Profiler2 though it isn't really the best example to follow given that it doesn't rely on EnableHTTP to function.

93 ↗(On Diff #13761)

I was calling it TryApplyMessage to (loosely) follow the convention from buffered channels in boost (ie, try_pop) and make it obvious that it was non-blocking. Technically, it should probably return a bool to follow the convention more closely but the return value isn't needed anywhere so it seemed unnecessary.

wraitii added a comment.Nov 10 2020, 7:54 AM

I'll let you change the EnableHTTP thing, but as said from my end this looks good.

source/rlinterface/RLInterface.h
93 ↗(On Diff #13761)

I see. I definitely didn't see that parallel :P

IMO it's not really that fitting as a parallel, and it being obviously non-blocking isn't such a necessity (it could very well be blocking and still be entirely functional, from a "no human input" POV).

Though as I said, I'm not sure my suggestions are much better.

Stan added a comment.Nov 10 2020, 10:18 AM

Just a question, but it seems we use std::string everywhere. Shouldn't we use CStr instead?

source/rlinterface/RLInterface.h
57 ↗(On Diff #13178)

This was not adressed. See NONCOPYABLE(className) in source/lib/code_annotation.h:217

irishninja updated this revision to Diff 13837.Nov 11 2020, 3:17 AM
irishninja marked 4 inline comments as done.
  • Make non-copyable; start http server in ctor
irishninja added a comment.Nov 11 2020, 3:26 AM

Just updated the revision to address some of the remaining comments. Let me know if there are any remaining issues/comments/etc!

Stan - I am using std::string everywhere is because it is referring to the game state which is returned by ScriptInterface.StringifyJSON(&state, false) as a std::string. Eventually these are all converted to c strings but I put that off until later so I can use convenience methods like string.empty(). Happy to change it if it isn't ideal or I am missing something :)

source/rlinterface/RLInterface.cpp
47 ↗(On Diff #13484)

My thinking had been that game messages do not need to be used after they are "sent" so I could pass them as an r-value. This should help avoid any additional copying and make it more clear in the code that the message has been "consumed" (so it shouldn't be edited afterwards, etc).

Does that make sense?

50 ↗(On Diff #13484)

Adding m_GameMessage = std::move(msg); should fix this here, right? I was thinking that it should help since the variable is moved to the data member and will no longer be freed after it goes out of scope (ie, when the variable frame is popped off the stack). Is this correct or am I missing something?

source/rlinterface/RLInterface.h
93 ↗(On Diff #13761)

So, the reason why blocking vs non-blocking mattered was when running w/ a GUI since you may want to pan around and view things while the AI is playing the game. I believe it is a bit choppy and problematic if it was blocking when running with a GUI.

Not to make any argument for the naming - I am happy to defer to whatever the consensus is - but I figured I would mention the significance here :) (Technically, I think I ran into this awhile back when I actually was using boost channels for communicating between the threads. :) )

Vulcan added a comment.Nov 11 2020, 3:29 AM

Successful build - Chance fights ever on the side of the prudent.

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1822/display/redirect

Harbormaster completed remote builds in B13558: Diff 13837.Nov 11 2020, 3:34 AM
wraitii added inline comments.Nov 11 2020, 8:33 AM
source/rlinterface/RLInterface.cpp
47 ↗(On Diff #13484)

I think vlad's comment in the context that you kept a reference to that r-value below, which didn't help, but since you've fixed that, I think your reasoning holds up.

source/rlinterface/RLInterface.h
93 ↗(On Diff #13761)

Yes I understand that actually, my comment was more that, while it is useful, it isn't required for this to "work". Being blocking would degrade the experience, but isn't a complete deal-breaker. Ergo I'd say it's not a fundamental property of this function that it is non-blocking, just a nice bonus.

Anyways :p

Stan added inline comments.Nov 11 2020, 11:03 AM
source/rlinterface/RLInterface.cpp
49 ↗(On Diff #13837)

Do you need 6 threads? AFAIK you're not using a browser?

283 ↗(On Diff #13837)

Early return;

286 ↗(On Diff #13837)

Invert, early return, to remove indent.

288 ↗(On Diff #13837)

Could be a helper function? ApplyMessageMaybe?

386 ↗(On Diff #13837)

Maybe g_Game != nullptr? No strong feelings.

irishninja marked 5 inline comments as done.Nov 12 2020, 4:48 AM
irishninja updated this revision to Diff 13864.Nov 12 2020, 4:48 AM
  • Refactor TryApplyMessage; decrease thread count
Vulcan added a comment.Nov 12 2020, 4:55 AM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1842/display/redirect

Harbormaster completed remote builds in B13589: Diff 13864.Nov 12 2020, 5:21 AM
Stan added a comment.Dec 18 2020, 2:11 AM

@vladislavbelov anything else?

vladislavbelov added inline comments.Jan 11 2021, 12:40 PM
source/main.cpp
482 ↗(On Diff #13864)

g_RLInterface = std::make_unique<RL::Interface>(server_address.c_str());, since we have C++17.

source/rlinterface/RLInterface.cpp
39 ↗(On Diff #13864)

No need to assign nullptr to a global object.

152–156 ↗(On Diff #13864)

Instead of duplicating allocations you might create a string with enough space:

const int contentSize = std::atoi(val);
std::string content(' ', contentSize);
mg_read(conn, content.data(), content.size());
scenario.content = std::move(content);
177–179 ↗(On Diff #13864)

The same here.

216–219 ↗(On Diff #13864)

And here. It seems a code duplication, might be unified.

361 ↗(On Diff #13864)

Might be const float.

irishninja updated this revision to Diff 15223.Jan 13 2021, 2:33 AM

Fix minor C++ issues around type and unique_ptr creation

irishninja added a comment.Jan 13 2021, 2:34 AM

Thanks for the feedback! I have made a few of the changes and left a question about the last remaining ones.

source/rlinterface/RLInterface.cpp
152–156 ↗(On Diff #13864)

Makes sense. However, it looks like content.data() is const and can't be used to set the contents of the string using mg_read. Am I missing something?

Vulcan added a comment.Jan 13 2021, 2:35 AM

Build has FAILED

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/3910/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/vs2015-differential/3910/display/redirectconsole

Vulcan added a comment.Jan 13 2021, 2:38 AM

Build has FAILED

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2818/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/macos-differential/2818/display/redirectconsole

Vulcan added a comment.Jan 13 2021, 2:42 AM

Build has FAILED

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/4481/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/docker-differential/4481/display/redirectconsole

Harbormaster completed remote builds in B14968: Diff 15223.Jan 13 2021, 2:42 AM
irishninja updated this revision to Diff 15279.Jan 14 2021, 2:34 AM

Applying the changes on top of the latest version of 0 AD to hopefully fix the build error.

Vulcan added a comment.Jan 14 2021, 2:35 AM

Build has FAILED

builderr-debug-macos.txt
../../../source/rlinterface/RLInterface.cpp:303:5: error: use of undeclared identifier 'EndGame'
                                EndGame();
                                ^
../../../source/rlinterface/RLInterface.cpp:307:15: error: no viable conversion from 'shared_ptr<ScriptContext>' to 'JSContext *'
                        JSContext* cx = scriptInterface.GetContext();
                                   ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../source/rlinterface/RLInterface.cpp:308:4: error: unknown type name 'JSAutoRequest'
                        JSAutoRequest rq(cx);
                        ^
../../../source/rlinterface/RLInterface.cpp:353:16: error: no viable conversion from 'shared_ptr<ScriptContext>' to 'JSContext *'
                                JSContext* cx = scriptInterface.GetContext();
                                           ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../source/rlinterface/RLInterface.cpp:354:5: error: unknown type name 'JSAuto

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2855/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/macos-differential/2855/display/redirectconsole

Vulcan added a comment.Jan 14 2021, 2:36 AM

Build has FAILED

builderr-debug-gcc7.txt
../../../source/rlinterface/RLInterface.cpp: In member function 'void RL::Interface::ApplyMessage(const RL::GameMessage&)':
../../../source/rlinterface/RLInterface.cpp:303:5: error: 'EndGame' was not declared in this scope
     EndGame();
     ^~~~~~~
../../../source/rlinterface/RLInterface.cpp:303:5: note: suggested alternative: 'g_Game'
     EndGame();
     ^~~~~~~
     g_Game
../../../source/rlinterface/RLInterface.cpp:307:47: error: cannot convert 'std::shared_ptr<ScriptContext>' to 'JSContext*' in initialization
    JSContext* cx = scriptInterface.GetContext();
                                               ^
../../../source/rlinterface/RLInterface.cpp:308:4: error: 'JSAutoRequest' was not declared in this scope
    JSAutoRequest rq(cx);
    ^~~~~~~~~~~~~
../../../source/rlinterface/RLInterface.cpp:308:4: note: suggested alternative: 'JSAutoRealm'
    JSAutoRequest rq(cx);
    ^~~~~~~~~~~~~
    JSAutoRealm
../../../source/rlinterface/RLInterface.cpp:326:47:

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/4514/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/docker-differential/4514/display/redirectconsole

Vulcan added a comment.Jan 14 2021, 2:36 AM

Build has FAILED

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/3943/display/redirect
See console output for more information: https://jenkins.wildfiregames.com/job/vs2015-differential/3943/display/redirectconsole

Harbormaster completed remote builds in B15020: Diff 15279.Jan 14 2021, 2:36 AM
wraitii updated this revision to Diff 15329.Jan 15 2021, 9:35 AM

This should compile. Will run a docker noPCH build to make sure there's no issues.

Vulcan added a comment.Jan 15 2021, 9:44 AM

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libscriptinterface_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ..

See https://jenkins.wildfiregames.com/job/macos-differential/2871/display/redirect for more details.

Harbormaster completed remote builds in B15058: Diff 15329.Jan 15 2021, 10:35 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 15 2021, 5:30 PM
Closed by commit rP24631: Address concerns in rP23917 (RL interface) (authored by wraitii). · Explain Why
This revision was automatically updated to reflect the committed changes.
wraitii added a commit: rP24631: Address concerns in rP23917 (RL interface).

Revision Contents
Changeset List

PathSize
ps/
trunk/
source/
70 lines
ps/
GameSetup/
2 lines
scripting/
3 lines
rlinterface/
153 lines
310 lines
scriptinterface/
2 lines

Diff 15349

View Options

ps/trunk/source/main.cpp

Loading...
View Options

ps/trunk/source/ps/GameSetup/GameSetup.h

Loading...
View Options

ps/trunk/source/ps/scripting/JSInterface_Game.cpp

Loading...
View Options

ps/trunk/source/rlinterface/RLInterface.h

Loading...
View Options

ps/trunk/source/rlinterface/RLInterface.cpp

Loading...
View Options

ps/trunk/source/scriptinterface/ScriptEngine.h

Loading...
Wildfire Games · Phabricator