[CF-Devel] Bug in stealing code

[crossfire-devel at archives.real-time.com]

Andreas Kirschbaum wrote:
>
     
      When an attempt to steal an item fails, the moved object may vanish from
     
     >
     
      the inventory view in the client.
     
     >
     
     
     >
     
      In server/skills.c the function attempt_steal() reads
     
     >
     
     
     >
     
      	else if (roll < chance ) {
     
     >
     
      	    if (op->type == PLAYER)
     
     >
     
      		esrv_del_item(op->contr, tmp->count);
     
     >
     
      	    pick_up(who, tmp);
     
     >
     
     
     >
     
      The function pick_up() may fail (for various reasons). When this
     
     >
     
      happens, the item has been removed from the inventory view but may
     
     >
     
      actually remain in the inventory.
     
     
  Yep.

>
     
     
     >
     
      My attached patch tries to fix this. I tested it (player stealing from
     
     >
     
      monster into inventory or container; monster stealing from player with
     
     >
     
      or without success) but I still have some questions:
     
     
  The patch as provided does violate proper C - you can't legally declare new 
variables just anyplace - has to be at the head of a block.  That is allowed in 
C++, and many C compilers allow various forms of C++ code.

>
     
     
     >
     
       1. I assume that
     
     >
     
     
     >
     
              for(tmp = op->inv; tmp != NULL; tmp = next)
     
     >
     
                  assert(tmp->env == op);
     
     >
     
     
     >
     
          hold for all items in the game.
     
     >
     
     
     >
     
          Is this assumption correct? If not: how can I detect if the object
     
     >
     
          was actually moved? (The current "who == is_player_inv(tmp)"
     
     >
     
          does not hold for monsters stealing items, I presume.)
     
     
  That assertion is correct.  Note that for objects on maps, env will (or should 
be) null.

  But I think the code you had in place should be fine, and is what I'll commit.


>
     
     
     >
     
       2. If the function pick_up() destroys the object (though don't know if
     
     >
     
          that can happen here) attempt_steal() uses this object afterwards
     
     >
     
          (as "tmp" or "success"). Is this acceptable?
     
     
  I'd have to dig into the code more.  The only case where tmp could be 
'destroyed' is if it merges with another object.  It may be that pick_up covers 
that case, and sets tmp to that new (merged) object.

>
     
     
     >
     
       3. The character weight in the client does not get updated. Even the
     
     >
     
          command 'fix_me' executed from the client does not fix it.
     
     >
     
     
     >
     
          How should I fix this issue?
     
     
  Depends on the bug, as I'm not sure exactly what you are reporting.  It could be:

1) Player has item stolen, but the theif can't pick it up.  It disappears from 
the players inventory, but he still has it.  In this case, no bug, as the player 
still has it.

2) Player steals an item successfully - now in his inventory, but his weight is 
not updated.

3) Player has item stolen, and theif correctly has it in his inventory, but old 
player still shows higher weight.

  Case 2 & 3 are bugs.  I see that fix_player() actually doesn't fix the weight. 
  I've updated fix_me to also recalculate the players weight.

>
     
     
     >
     
     
     >
     
      ------------------------------------------------------------------------
     
     >
     
     
     >
     
      Index: server/skills.c
     
     >
     
      ===================================================================
     
     >
     
      RCS file: /cvsroot/crossfire/crossfire/server/skills.c,v
     
     >
     
      retrieving revision 1.51
     
     >
     
      diff -u -w -r1.51 skills.c
     
     >
     
      --- server/skills.c	12 May 2004 04:43:06 -0000	1.51
     
     >
     
      +++ server/skills.c	28 May 2004 19:41:19 -0000
     
     >
     
      @@ -146,15 +146,18 @@
     
     >
     
       	if((chance=adj_stealchance(who,op,(stats_value+skill->level * 10 - op->level * 3)))==-1)
     
     >
     
       	    return 0;
     
     >
     
       	else if (roll < chance ) {
     
     >
     
      -	    if (op->type == PLAYER)
     
     >
     
      -		esrv_del_item(op->contr, tmp->count);
     
     >
     
      +	    tag_t tmp_count = tmp->count;
     
     >
     
      +
     
     >
     
       	    pick_up(who, tmp);
     
     >
     
       	    /* need to see if the player actually stole this item -
     
     >
     
       	     * if it is in the players inv, assume it is.  This prevents
     
     >
     
       	     * abuses where the player can not carry the item, so just
     
     >
     
       	     * keeps stealing it over and over.
     
     >
     
       	     */
     
     >
     
      -	    if (who == is_player_inv(tmp)) {
     
     >
     
      +	    if(was_destroyed(tmp, tmp_count) || tmp->env != op) {
     
     >
     
      +		if(op->type == PLAYER) {
     
     >
     
      +		    esrv_del_item(op->contr, tmp_count);
     
     >
     
      +		}
     
     >
     
       		/* for players, play_sound: steals item */
     
     >
     
       		success = tmp;
     
     >
     
       		CLEAR_FLAG(tmp, FLAG_INV_LOCKED);
     
     >
     
     
     >
     
     
     >
     
      ------------------------------------------------------------------------
     
     >
     
     
     >
     
      _______________________________________________
     
     >
     
      crossfire-devel mailing list
     
     >
     
     
      crossfire-devel at lists.real-time.com
      
      
     >
     
     
      https://mailman.real-time.com/mailman/listinfo/crossfire-devel
      
      
     

_______________________________________________
crossfire-devel mailing list
     
     crossfire-devel at lists.real-time.com
     
     
     https://mailman.real-time.com/mailman/listinfo/crossfire-devel