Page MenuHomeWildfire Games

[WIP] Replay Database
Needs ReviewPublic

Authored by gentz on Mon, Dec 31, 8:39 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#4346
Summary

While it's still work in progress, I though I'd post what I got so far here anyways.

I was getting Uploaded file is too large: current limit is 2M. To adjust this limit change 'upload_max_filesize' in php.ini. when trying to include my additions to libraries in the patch ('cause it's 28MB) so I've uploaded them separately here:https://drive.google.com/file/d/18jB1pm1ocW5pCrgZBE1lxeeAkOkPV0_q/view?usp=sharing

It's just boost asio & some json parsing library.

Oh, here is a screenie of the progress so far.
https://i.imgur.com/MKvmo1o.png

Test Plan

Well, first it aught to work.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6647
Build 10952: arc lint + arc unit

Event Timeline

gentz planned changes to this revision.Mon, Dec 31, 8:39 AM
gentz created this revision.
gentz updated this revision to Diff 7164.Mon, Dec 31, 8:42 AM

I forgot to include CMultilist.{cpp,h}

gentz planned changes to this revision.Mon, Dec 31, 8:42 AM
gentz edited the summary of this revision. (Show Details)Mon, Dec 31, 8:45 AM
elexis added a subscriber: elexis.Mon, Dec 31, 11:11 AM

Wanna post a description what this feature is about?
I suppose you might be better off using a git branch, rebase the things into logically independent commits (like D1717 being a separate git commit), and then export the individual commits (git show <id>).
(What is uploaded here seems to contain a lot of unrelated old commits, and the .git files should be excluded, but I guess that's obvious. If you have a git branch, you might as well post a link to that)

source/scriptinterface/ScriptConversions.cpp
81

No, been there already. There are reverted commits for that.
Max value of JS Number is 2^53 something. Use double.

gentz updated this revision to Diff 7170.Tue, Jan 1, 12:48 AM
gentz marked an inline comment as done.Tue, Jan 1, 1:13 AM
In D1723#68257, @elexis wrote:

Wanna post a description what this feature is about?

I suppose you might be better off using a git branch, rebase the things into logically independent commits (like D1717 being a separate git commit), and then export the individual commits (git show <id>).
(What is uploaded here seems to contain a lot of unrelated old commits, and the .git files should be excluded, but I guess that's obvious. If you have a git branch, you might as well post a link to that)

The git stuff was for testing out stuff relating to diesel. Removed. I've also gotten rid of the white space changes which I accidentally did while fiddling with the linter.

Anyways, I did a "couple" changes:

  • D1717's changes, ofc
  • Added a new list class called CMultiList. The multilist is like a normal COList except that it can stack columns. As for an example of how it looks, I'll refer you to this screenshot: https://i.imgur.com/MKvmo1o.png
  • Added a new gui page for the rdb under binaries/data/mods/public/gui/rdb/ and it's appropriate page_rdb.xml file.
  • Inserted a Replay Database button under Multiplayer on the main menu. https://i.imgur.com/1nch4wu.png
  • Changed the prelobby gui so that it's used for both loging in and registering to both the replay database and the multiplayer lobby. https://i.imgur.com/bNjhCMy.png
  • Made a separate Privacy Policy/Terms Of Use/Terms Of Service for the RDB. Updated to be applicable to the RDB. IANAL.
  • Messed with the build system to add an asio and json dependency. Also added a new rdb target. I basically threw code at it till it compiled. Hoping someone shows me how to do it properly.
  • On the C++ side, I added the RdbClient. Its job it to connect to the RDB server, which has been written in Rust and included under source/tools/rdb.
  • On the C++ side, I also added a JSInterface for the rdb.
  • I changed the VisualReplay code, so that the RdbClient code could also use it by passing in a stringstream. (Previously, it was only really usable with filestreams)
  • And of course, I added the server code under source/tools/rdb. The 2K lines of INSERT INTO blah blah blah are only for testing and will be removed... trust me. Ignore them for now.
gentz updated this revision to Diff 7177.Tue, Jan 1, 2:30 PM

Add submission date column and fix scrollbar.

gentz updated this revision to Diff 7183.Wed, Jan 2, 7:06 AM

Fix feedback. Make code more consistent. Add window that opens when replays are pressed on. Add UserDatas parsing code.

gentz updated this revision to Diff 7184.Wed, Jan 2, 8:43 AM

Username querying.

Stan added a subscriber: Stan.Wed, Jan 2, 9:12 AM

Some comments. Might be applicable in more places

binaries/data/mods/public/gui/pregame/mainmenu.js
245

Would probably better if disabled by a macro when a premake flag is set ?:)

binaries/data/mods/public/gui/rdb/replay_actions.js
22

Remove braces

31

No braces for single statements in ifs. Wrong indentation.

binaries/data/mods/public/gui/rdb/replay_menu.js
117

No curly braces.

141

Weird indent seem inneficient to have a variable and it's contrary below.

211

Don't use Parseint use + instead I believe parse int is slower

291

Remove if and for braces. Same for above.e.

317

Could be simplified.

smiley added a subscriber: smiley.Wed, Jan 2, 10:44 AM

(Might want to ask who ever maintains the WFG servers whether this is something that is affordable space wise)

gentz updated this revision to Diff 7185.Wed, Jan 2, 11:29 AM

Prep for the Replay viewer.

gentz marked 6 inline comments as done.Wed, Jan 2, 11:53 AM
gentz added inline comments.
binaries/data/mods/public/gui/pregame/mainmenu.js
245

Doesn't it do that? If we don't have a RdbStartClient function, that means we didn't compile with rdb support, right?

binaries/data/mods/public/gui/rdb/replay_actions.js
22

Oh god, I keep forgetting this. Most projects say that you should never get rid of braces, and I'm just used to that now.

binaries/data/mods/public/gui/rdb/replay_menu.js
211

ParseIntU uses Number(str), not ParseInt. I realize the name is partially misleading, but it did use to use it.

317

How?

gentz updated this revision to Diff 7186.Wed, Jan 2, 11:54 AM
gentz marked an inline comment as done.

Some of Stan's suggestions.

Before review I have arch-questions:

  • Why boost.asio was used? ModIo uses CURL and actually does the same thing. So I'd prefer to not have different ways for the same goal.
  • You use rdb lobby, but it's not a social thing. I think it should be like mods from ModIo on a separate page.
  • Why Rust was chosen as server language? I haven't written a web-application using Rust. So I'm worrying about support (e.g. a size of code) and security stuff (e.g. SQL injections or CRSF). Also for me it looks like there is more code than needed to write the same thing with a ready library (at least on languages like Java, Python or PHP). Currently a profit that I see from Rust is only a possible performance. So does it worth to use it? Or I'm wrong somewhere?
gentz added a comment.Wed, Jan 2, 11:58 AM
In D1723#68465, @smiley wrote:

(Might want to ask who ever maintains the WFG servers whether this is something that is affordable space wise)

I was planning to host it myself on an OVH VPN, as I don't imagine it will consume that much space.

gentz added a comment.Wed, Jan 2, 12:35 PM

Before review I have arch-questions:

  • Why boost.asio was used? ModIo uses CURL and actually does the same thing. So I'd prefer to not have different ways for the same goal.

I happen to have minimal asio experience and just wanted a quick POC. I can replace it with CURL latter.

  • You use rdb lobby, but it's not a social thing. I think it should be like mods from ModIo on a separate page.

Uhh, I ought to admit, that might not be the bestly named variable, but I assure you it's on a separate page: https://i.imgur.com/HuGfJ9N.png
They only share the same login screen.

I haven't written a web-application using Rust. So I'm worrying about support (e.g. a size of code) and security stuff (e.g. SQL injections or CRSF).

Diesel protects us from SQL injections. No clue what CRSF is however.

Also for me it looks like there is more code than needed to write the same thing with a ready library (at least on languages like Java, Python or PHP).

I've never made any other server, so I can't really compare it to other servers in terms of verbosity. However, for what the server does, it really isn't that verbose, especially when you note
that some files are auto generated (eg. src/schema.rs).

  • Why Rust was chosen as server language?

Currently a profit that I see from Rust is only a possible performance. So does it worth to use it? Or I'm wrong somewhere?

Rust was chosen as the server's language primarily because it's safe from things like buffer overflows, and other related memory safety bugs due to it's ownership model. *
Additionally, It's ownership model also allows one to easily multithread the code, which is useful for servers. It manages this while being as equally fast as the equivalently optimized C++ code.

. * Memory safety can't be guaranteed for non-Rust code called by the dependencies (e.g. OpenSSL).

vladislavbelov added a comment.EditedWed, Jan 2, 2:04 PM
In D1723#68484, @gentz wrote:

I was planning to host it myself on an OVH VPN, as I don't imagine it will consume that much space.

I'm not sure that it's possible to store WFG collected data on third party servers.

In D1723#68485, @gentz wrote:
  • Why boost.asio was used? ModIo uses CURL and actually does the same thing. So I'd prefer to not have different ways for the same goal.

I happen to have minimal asio experience and just wanted a quick POC. I can replace it with CURL latter.

I'm thinking about to probably have a base class with all needed network stuff.

  • You use rdb lobby, but it's not a social thing. I think it should be like mods from ModIo on a separate page.

Uhh, I ought to admit, that might not be the bestly named variable, but I assure you it's on a separate page: https://i.imgur.com/HuGfJ9N.png
They only share the same login screen.

I meant your implementation, you added g_ConfigPrefix and use separate authentication data. That means user have to remember more passwords. My points here:

  • You don't need to login to watch replays (like modio)
  • You don't need another login/password pair.

I haven't written a web-application using Rust. So I'm worrying about support (e.g. a size of code) and security stuff (e.g. SQL injections or CRSF).

Diesel protects us from SQL injections. No clue what CRSF is however.

Сross Site Request Forgery.

Rust was chosen as the server's language primarily because it's safe from things like buffer overflows, and other related memory safety bugs due to it's ownership model. *
Additionally, It's ownership model also allows one to easily multithread the code, which is useful for servers. It manages this while being as equally fast as the equivalently optimized C++ code.
. * Memory safety can't be guaranteed for non-Rust code called by the dependencies (e.g. OpenSSL).

Yeah, I understand that. I meant in comparison with other "server" languages (Python, Java, PHP, JS or Go). Because most of them doesn't have such problems (ofc less performance than C-like languages).

Why I'm asking? Because many production web-applications are written on these languages and I don't much about Rust web-application. So we may don't know some pitfalls.

gentz updated this revision to Diff 7187.Wed, Jan 2, 2:12 PM

Remove some things.

gentz added a comment.Wed, Jan 2, 2:26 PM
In D1723#68484, @gentz wrote:

I was planning to host it myself on an OVH VPN, as I don't imagine it will consume that much space.

I'm not sure that it's possible to store WFG collected data on third party servers.

Wouldn't be this be no different than modio? WFG doesn't host that either, afaik.

  • You use rdb lobby, but it's not a social thing. I think it should be like mods from ModIo on a separate page.

Uhh, I ought to admit, that might not be the bestly named variable, but I assure you it's on a separate page: https://i.imgur.com/HuGfJ9N.png
They only share the same login screen.

I meant your implementation, you added g_ConfigPrefix and use separate authentication data. That means user have to remember more passwords. My points here:

  • You don't need to login to watch replays (like modio)

Yes, but you will need to login to upload/vote on/comment on/report replays, so might as well have them log in.

  • You don't need another login/password pair.

I don't have access to the lobby's pairs. This is easier.

I haven't written a web-application using Rust. So I'm worrying about support (e.g. a size of code) and security stuff (e.g. SQL injections or CRSF).

Diesel protects us from SQL injections. No clue what CRSF is however.

Сross Site Request Forgery.

I'll have to add per session tokens to the todo list then.

Rust was chosen as the server's language primarily because it's safe from things like buffer overflows, and other related memory safety bugs due to it's ownership model. *
Additionally, It's ownership model also allows one to easily multithread the code, which is useful for servers. It manages this while being as equally fast as the equivalently optimized C++ code.
. * Memory safety can't be guaranteed for non-Rust code called by the dependencies (e.g. OpenSSL).

Yeah, I understand that. I meant in comparison with other "server" languages (Python, Java, PHP, JS or Go). Because most of them doesn't have such problems (ofc less performance that C-like languages).

Why I'm asking? Because many production web-applications are written on these languages and I don't much about Rust web-application. So we may don't know some pitfalls.

I can't imagine it will any more pitfalls than a server written in a C-like language.

elexis added a comment.Wed, Jan 2, 2:48 PM

On the feature design:

  • An online replay database suggests itself, there might even be a ticket on trac for it.
  • Dunno about rust.
  • Ideally it should work with the pyrogenesis engine in general, not only with 0ad that uses it. Other pyrogenesis games may use different replay attributes, so it would be more versatile to avoid hardcodings where possible. (Perhaps such a game would have to specify an own set of supported properties that the backend would have to load to support them.)
  • Scalability and traffic should be estimated.
  • Third party services can be used in theory, but there are also many advantages for self-hosting. If the code is released and freely licensed the host can be decided arbitrarily.

On the implementation:

  • The replay_menu.js copy is sad, it would be nicer to reuse the existing replay menu with a way to chose between the local disk and a remote server (could also be decided in the main menu before opening the replay menu, but switching afterwards would be nice too).
  • Reusing the prelobby is a quick and requirements satisfying way to address the Privacy Policy question. Reusing it that way however implies that one can't add lobby specific GUI objects that make no sense for the replay menu to the prelobby page anymore. If the server uses useraccounts, perhaps it's still better to reuse rather than to copy something which is 90% identical. (Perhaps abstraction / refactoring out the shared code works too. Only testing implementations and predicting requirements can tell what will be best.)
  • (Half the Privacy Policy is copied, the duplication exists for other services too. Perhaps we can fetch the policies from a remote server eventually to have only one copy of the shared part.)
  • ReplayDataBase would be more informative than rdb
  • Would be nice to avoid boost where possible
  • Dunno about that new list
  • (I recall there is some deformed duplication of visual replay and non-visual replay file parsing. But thats probably not tangent to this patch.)
source/rdb/scripting/JSInterface_Rdb.cpp
146

Perhaps it's easier to just use StringifyJSON on the attribs value and pass that string to the webinterface, rather than hardcoding all attributes?

In D1723#68498, @gentz wrote:

Wouldn't be this be no different than modio? WFG doesn't host that either, afaik.

Modio doesn't use login/password.

Yes, but you will need to login to upload/vote on/comment on/report replays, so might as well have them log in.

Should it be a requirement - to login?

I don't have access to the lobby's pairs. This is easier.

You could start a local lobby :)

I can't imagine it will any more pitfalls than a server written in a C-like language.

C-like yes, but the list above (Go, Java, Python, etc) isn't the list of C-like languages, they're pretty safe and have a lot of ready solutions for most cases of web-applications.

gentz added a comment.Thu, Jan 3, 12:22 AM
In D1723#68506, @elexis wrote:
  • Ideally it should work with the pyrogenesis engine in general, not only with 0ad that uses it. Other pyrogenesis games may use different replay attributes, so it would be more versatile to avoid hardcodings where possible. (Perhaps such a game would have to specify an own set of supported properties that the backend would have to load to support them.)

Yeah, that's a good idea. I'll add it to the todo list.

  • Third party services can be used in theory, but there are also many advantages for self-hosting. If the code is released and freely licensed the host can be decided arbitrarily.

The code is released and is licensed just like everything else.

On the implementation:

  • The replay_menu.js copy is sad, it would be nicer to reuse the existing replay menu with a way to chose between the local disk and a remote server (could also be decided in the main menu before opening the replay menu, but switching afterwards would be nice too).

Merging them back is already on my todo list. I just wanted a quick POC for now.

  • Reusing the prelobby is a quick and requirements satisfying way to address the Privacy Policy question. Reusing it that way however implies that one can't add lobby specific GUI objects that make no sense for the replay menu to the prelobby page anymore. If the server uses useraccounts, perhaps it's still better to reuse rather than to copy something which is 90% identical. (Perhaps abstraction / refactoring out the shared code works too. Only testing implementations and predicting requirements can tell what will be best.)

If some lobby-specific or rdb-specific things need to be added they could be split apart, however, as it currently stands, their requirements are basically identical.

  • Would be nice to avoid boost where possible

It will be replaced with curl.

  • Dunno about that new list

Any specific issues?

gentz added a comment.Thu, Jan 3, 12:26 AM
In D1723#68498, @gentz wrote:

Yes, but you will need to login to upload/vote on/comment on/report replays, so might as well have them log in.

Should it be a requirement - to login?

Well, requiring users to login for those actions will help moderators moderate. I guess the alternative would be to remember the IP which did an action, but that doesn't strike me as a good alternative.

elexis updated the Trac tickets for this revision.Tue, Jan 8, 11:16 AM