#400 Add follow command for non-DMs, allow following into permanent apartments

[Kevin Zheng]

The three patches attached here enable the 'follow' command for non-DM players. It allows players to follow each other, including through exits and permanent apartments.

When two players are on the same tiled map, this code simply calls player_move() in the direction of the player being followed. If the player being followed moves through an exit, that the exit object is copied to a new field in the player structure. The follower then moves to the exit (instead of the player) and applies the exit once on top. This means that gates, check_inv's, and other forms of blocked movement
will still continue to be effective.

This patch reduces the possibility of abuse by requiring players be in the same party in order to follow each other. If the followed player leaves the party at any point, players can no longer follow them.

Patch 2 allows followers to follow into unique maps such as permanent apartments. This allows players to visit each others' permanent apartments.

[Nicolas Weeger]

Seems ok to me on the concept :)

Maybe the followed player could be warned, just in case?

Also, if the followed player moves faster than the follower, and uses 2 exits, follower may lose track.

I'm slightly concerned by "failure" handling, if the following player can't move, things like that. Are all cases covered?

[Kevin Zheng]

I agree that the player should be warned. I'll add "Zebulon starts following you." and "Zebulon stops following you." shortly.

To losing tracks after two exits, yes. A more common occurrence is that the follower loses track because the simple rv_direction pathfinding sometimes gets stuck. Whenever a player gets stuck, it will keep trying to follow the leader, calling player_move() in the zero direction (stay) until it knows where to go. This means if the player returns, the follower continues to follow the leader.

I made an effort to keep this code path as similar to player movement (as a result of commands) as possible. This checks for speed_left and runs player_move(), which would take some of these edge cases into account. But an extra pair of eyes and some more testing would certainly be welcome.

[Andreas Kirschbaum]

0001-Enable-follow-command-for-non-DMs.patch:

Not sure here: "partial" in the function name suggests that the player name is not checked for equality. Therefore: what happens if you are following "abc" but there is also "abcd" logged in? (This is just code moved around, so maybe not relevant to this patch.)

+    player *followed = find_player_partial_name(pl->followed_player);

Other than that, I cannot see any problems with this patch, therefore no objections from me.

The following comments are only nitpicks about code style. Feel free to ignore them.

• Nitpick about style:

+    return !(other->ob->contr->party == NULL || op->contr->party != other->ob->contr->party);

I would prefer the IMO simpler to understand variant:

+    return other->ob->contr->party != NULL && op->contr->party == other->ob->contr->party;

• Nitpick about consistency:

+    if (op->contr) {

A few lines further:

+        if (op->contr->last_exit != NULL) {

That is: the first check treats a pointer as a boolean value, the second one explicitly checks for a non-NULL pointer.

• Nitpick about style:

+    if (rv.direction != 0 && rv.distance > mindist) { 
+        if (ob->speed_left > 0) {

I would merge both if statements into a single one:

+    if (rv.direction != 0 && rv.distance > mindist && ob->speed_left > 0) {

• Nitpick about style:

+static int move_towards(object* ob, object* towards, int mindist) {

IMO the function should have a comment. The same holds for "do_follow" later on.

• Nitpick about style:

+static int move_towards(object* ob, object* towards, int mindist) {
[...]
+    return rv.direction;

The caller considers the return value as a boolean. Therefore I would explicitly return a boolean:

+static bool move_towards(object* ob, object* towards, int mindist) {
[...]
+    return rv.direction != 0;

• Nitpick about style:

+    return (ob->x >= x+sx2) && (ob->x <= x+sx) && (ob->y >= y+sy2) && (ob->y <= y+sy);

I would omit the parentheses. I would also swap some conditions because I think it makes the range checks easier to understand:

+    return x+sx2 <= ob->x && ob->x <= x+sx && y+sy2 <= ob->y && ob->y <= y+sy;

• Nitpick about style:

+                if (space == -1) 
+                    /** This is a DM, just teleport on the top of player. */
+                    space = 0;

I would add braces in this case because it visually looks as if there were two statements following the if line. (This is just code moved around, so maybe not relevant to this patch.)

• Nitpick about style (the code is (currently) perfectly fine, at least until somebody changes the calling code):

+static void do_follow(player* pl) {
+    player *followed = find_player_partial_name(pl->followed_player);

I would add some kind of "assert(pl->followed_player != NULL);" statement here. Just to make sure the caller adheres to the implicit contract of this function.

• Nitpick about style;

+            int can_move = move_towards(pl->ob, followed->ob, 1);
+            if (can_move == 0 && followed->ob->contr->last_exit != NULL) {

The variable can_move is used only here, therefore I would eliminate the variable by inlining the (relatively small) function call into the if statement.

0002-Follow-followed-player-through-unique-exits.patch:

• Nitpick about consistency:

     mapstruct        *newmap;
+    const char* player = op->name;

The pointer "" in the first line is visually attached to the variable, in the second line to the type. I strongly prefer the first variant because then the syntax matches the semantic. This problem manifests in declarations like "int a, b;" very clearly: this declaration declares "a" as a pointer to an int and b as a plain int. Visually is looks as if two pointers were declared. Using "int *a, b;" instead does not have this problem.

• Not sure here: Could this patch be abused to enter the permanent apartment of the player being followed even if the player being followed is not entering his permanent apartment? Like when the player being followed is standing in Scorn and the following player manually applies the exit into the permanent apartment?

• Not sure here: What happens if the following player applies the savebed in the permanent appartment of the player being followed? Can he abuse this to gain permanent access to the other player's permanent apartment?

0003-Update-follow-command-help.patch:

No comments. This patch seems perfectly fine to me.

[Kevin Zheng]

First patch was committed, with most of the suggested changes.

[Kevin Zheng]

Closing, since these patches seem to have now been committed.