Author Topic: Add player related functions to nettool (work in progress)  (Read 4512 times)

0 Members and 1 Guest are viewing this topic.

Offline Ashley

Add player related functions to nettool (work in progress)
« on: February 21, 2012, 12:22:18 AM »
This patch adds some functions to nettool, it's still a work in progress however...

Functions added:

Code: [Select]
      players
        Receive list of player information from server

      player-name <player number> <new name>
        Change the name of a player

      player-pass <player number> <new password>
        Change the password of a player

I also considered adding a "reset-player" command, which would delete a company from the game. But I am not sure if this is really a useful feature or not.

The player list is returned in a crude CSV form. I think it's better to return data from the game in either CSV or another form (CSV is very simple and easy to implement, unlike XML...) rather than passing back serialised native objects which the nettool does not need to know about. I'm planning on converting the other functions of nettool to a similar scheme. Then there can be a simple switch to make nettool print either the raw CSV data (useful for other scripts/utilities to print) or pretty-printing the CSV data for human consumption.

This approach makes it a lot easier to add functions to retrieve data.

Next step (underway) is writing a simple CSV parsing class for use by nettool and also in transferring things like the game listings from the listings server (to promote code re-use).

I also need to find a way to lookup the client ID of the client currently playing as each player.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Devotee
  • *
  • Posts: 14774
  • Total likes: 309
  • Helpful: 144
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: Add player related functions to nettool (work in progress)
« Reply #1 on: February 21, 2012, 01:25:08 AM »
These features will be very useful, I think. Not being able to reset a player's password can cause real difficulties in administration either with players who have dropped out or with players who have forgotten their passwords.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4070
  • Total likes: 98
  • Helpful: 146
  • Languages: EN, DE, AT
Re: Add player related functions to nettool (work in progress)
« Reply #2 on: February 21, 2012, 08:03:47 AM »
Not being able to reset a player's password can cause real difficulties in administration either with players who have dropped out or with players who have forgotten their passwords.
Any human, who has public player unlocked, can change passwords of any player.
Parsley, sage, rosemary, and maggikraut.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #3 on: February 21, 2012, 08:52:58 AM »
True Dwachs, but a big motivation for features in nettool (for me) is not having to actually open a copy of Simutrans in order to administer the server. This also allows automation, e.g. ensuring that the human and public service players are locked when a new map is loaded (and renaming human to spectator).

I just noticed that I ought to hash the password before sending it to the server. Also the game admin password is sent in clear text too, so I'll change both to use hashes.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline isidoro

Re: Add player related functions to nettool (work in progress)
« Reply #4 on: February 21, 2012, 05:19:18 PM »
[...]
I just noticed that I ought to hash the password before sending it to the server. Also the game admin password is sent in clear text too, so I'll change both to use hashes.

I don't understand this, but maybe I'm thick today.  What difference would it make?  An attacker can send the hash he spied upon before...  Remember that anyone can compile a modified version of ST...

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #5 on: February 21, 2012, 06:10:00 PM »
Good point, I am just allergic to sending passwords in plaintext. I only use the nettool to connect to the server on localhost anyway (others may use it in other ways). Another enhancement is to add a whitelist of IP addresses from which the server will permit nettool connections (so you can restrict it to localhost only). This I will do along with the blacklist enhancements, since it is basically the same thing.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8573
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: Add player related functions to nettool (work in progress)
« Reply #6 on: February 22, 2012, 10:03:11 AM »
A password may be used in other places. And simutrans can salt password with something not so easy to predict, like the map number/heighmap/current year. In that case sending hashes will make a difference, as one would need to know the time and savegame.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #7 on: February 23, 2012, 01:39:24 PM »
Another work in progress, simple CSV class, permits field-by-field building of CSV data, as well as reading through the contained data field-by-field as well. Depends on the cbuffer_t class for internal storage. There's a short test program as well (stick these all under /utils/ and use the Makefile to compile).

This will be useful to replace the server_frame_t::parse_csv_field method (which is less general purpose), and I plan on using it more extensively in nettool as well.

Still some bugs to work out and finish off though!
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #8 on: February 29, 2012, 04:26:24 PM »
Still working on this.

csv_patch_2.patch
This adds a simple CSV encoding/decoding library based around a CSV_t object, backed by a cbuffer_t. It also adds an overloaded append() method to the cbuffer_t to let you specify a maximum length to copy.

network_mem.patch
This adds an HTTP GET method which instead of receiving data to a file on disk, receives it instead into a cbuffer_t. This avoids having to write the network game listing to disk and then read it back in again. Similarly it adds a network_receive_data method which receives into a cbuffer_t.

server_frame_csv.patch
This switches the server frame (netgame dialog) to use the new CSV library instead of the existing class method. It also switches it to use the new memory-based HTTP method.

(All three patches are not final, and need further testing!)


The CSV library will then be used for the nettool as well, to provide a consistent data interchange method between Simutrans and nettool (with either pretty-printed formatted output, or CSV output to allow easy parsing by external tools).
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #9 on: March 03, 2012, 04:37:43 PM »
Why does cbuffer_t have private copy constructor and assignment operator overload? Obviously this prevents making copies (and passing by value) in the obvious way, but is there a reason for this?
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4070
  • Total likes: 98
  • Helpful: 146
  • Languages: EN, DE, AT
Re: Add player related functions to nettool (work in progress)
« Reply #10 on: March 03, 2012, 04:57:19 PM »
This is a consequence of the discussion here:

http://forum.simutrans.com/index.php?topic=8994.0

Ie the cbuffer class has a custom constructor & destructor, so the copy-constructor and copy-assignment operator need a custom implementation, too. For now these methods are declared private to prevent the compiler from using default methods. You can however implement these functions (both of them) and then use it.

You can also pass this class by reference
Code: [Select]
void something(cbuffer_t const& buf)
Passing by value would require a lot of redundant copy operations.
Parsley, sage, rosemary, and maggikraut.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #11 on: March 03, 2012, 05:48:29 PM »
Ok, I'll implement copy-constructor and assignment operator for cbuffer_t, since it makes the implementation of CSV_t a lot cleaner (since that then doesn't need destructor, copy-constructor and assignment operator itself...)

I am passing cbuffer_t objects by reference, they will only get copied if the CSV_t itself gets copied.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #12 on: March 03, 2012, 06:47:55 PM »
Ok I think this is done, patches should apply against r5477. I've tested the CSV library with a test program and it seems pretty stable (will probably find some more bugs as I use it more for nettool).

If these patches are satisfactory, could they be merged, since it makes it easier to keep track of things if the changes are in the trunk :)
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4070
  • Total likes: 98
  • Helpful: 146
  • Languages: EN, DE, AT
Re: Add player related functions to nettool (work in progress)
« Reply #13 on: March 04, 2012, 10:11:18 AM »
Where do you need the copy constructor / assignment ?

Why did you implement another network_receive_method ? Why did you not use network_receive_data?

I also would like to change the csv_T::get_field method to something like
Code: [Select]
int get_next_field (cbuffer_t &buf);
Parsley, sage, rosemary, and maggikraut.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #14 on: March 04, 2012, 10:49:39 AM »
The cbuffer_t copy constructor/assignment operator would be used if a CSV_t object were to be copied. It avoids having to implement a copy constructor/destructor/assignment operator overload for the CSV_t class.

I could change it to use network_receive_data instead, although then there's no progress bar.

Edit: Also using network_receive_data means having to allocate memory twice, since you'd need to allocate a buffer to receive the data into and then copy the data from that buffer into the cbuffer_t passed into network_http_get().

That last one is easy enough to do. Why are we using cbuffer_t anyway and not std::string? :)
« Last Edit: March 04, 2012, 11:07:57 AM by Timothy »
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #15 on: March 04, 2012, 11:48:29 AM »
Here are patches with the requested changes made :)
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4070
  • Total likes: 98
  • Helpful: 146
  • Languages: EN, DE, AT
Re: Add player related functions to nettool (work in progress)
« Reply #16 on: March 04, 2012, 01:53:22 PM »
Thanks! These are now incorporated in r5508.

What is the state of the patch in the first post?
Parsley, sage, rosemary, and maggikraut.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #17 on: March 04, 2012, 02:03:45 PM »
Working on that now :) Shouldn't be too much work with these other bits finished.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8573
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: Add player related functions to nettool (work in progress)
« Reply #18 on: March 04, 2012, 09:13:50 PM »
The hashing of password is aparently still omitted: Why? We should still avoid sending clear text password, as user might user their password elsewhere.

Offline Ashley

Re: Add player related functions to nettool (work in progress)
« Reply #19 on: March 04, 2012, 09:20:40 PM »
I haven't got to that bit yet, the accepted patches don't deal with nettool, the nettool patch will include hashing of the passwords before sending them over the wire.
Use Firefox? Interested in IPv6? Try SixOrNot the IPv6 status indicator for Firefox.
Why not try playing Simutrans online? See the Game Servers board for details.