Help me make my new variant! (please!)

Collapse
X
 
  • Time
  • Show
Clear All
new posts
  • Julian
    Adept
    • Apr 2021
    • 122

    #91
    This looks like a bug in slot_by_name:

    Code:
    int slot_by_name(struct player *p, const char *name)
    {
    	int i;
    
    	/* Look for the correctly named slot */
    	for (i = 0; i < p->body.count; i++) {
    		if (streq(name, p->body.slots[i].name)) {
    			break;
    		}
    	}
    
    	/* Index for that slot */
    	return i;
    }
    If the name isn’t valid, you run off the end of the loop, and i is body.count.

    Then this bad index is fed to:
    Code:
    struct object *slot_object(struct player *p, int slot)
    {
    	/* Ensure a valid body */
    	if (p->body.slots && p->body.slots[slot].obj) {
    		return p->body.slots[slot].obj;
    	}
    
    	return NULL;
    }
    The test on body.slots[slot].obj is pointing off the end of the array, and you return… something

    slot_by_name() should be returning i where it breaks, and returning an error value at the end, probably -1, and slot_object() should be checking that i is within 0<=I<body.count

    Comment

    • Nick
      Vanilla maintainer
      • Apr 2007
      • 9647

      #92
      Originally posted by Julian
      slot_by_name() should be returning i where it breaks, and returning an error value at the end, probably -1, and slot_object() should be checking that i is within 0<=I<body.count
      I'm actually going to be more hard-core than that - I've put asserts in both places in the latest update.
      One for the Dark Lord on his dark throne
      In the Land of Mordor where the Shadows lie.

      Comment

      • will_asher
        DaJAngband Maintainer
        • Apr 2007
        • 1124

        #93
        It looks like most people didn't notice my edit at the bottom of the post:
        Originally posted by will_asher
        EDIT: I think I fixed the problem. Apparently the name of the slot is "arm" not "shield"...
        Will_Asher
        aka LibraryAdventurer

        My old variant DaJAngband:
        http://sites.google.com/site/dajangbandwebsite/home (defunct and so old it's forked from Angband 3.1.0 -I think- but it's probably playable...)

        Comment

        • Pete Mack
          Prophet
          • Apr 2007
          • 6883

          #94
          @Julian- 0xDDDDDDD is not -1. I suspect it is uninitialized memory left as a buffer by debug allocator.

          Comment

          • Julian
            Adept
            • Apr 2021
            • 122

            #95
            Originally posted by Pete Mack
            @Julian- 0xDDDDDDD is not -1. I suspect it is uninitialized memory left as a buffer by debug allocator.
            Almost certainly — whatever was after body.slots in memory was pointing to uninitialized memory when read as an object.

            -1 (or something clearly wrong that can be tested for) is what slot_by_name() should’ve been returning when the name didn’t map to a slot.

            Comment

            • Pete Mack
              Prophet
              • Apr 2007
              • 6883

              #96
              NULL is the usual. But it is immaterial, since Nick will use a hard assertion.

              Comment

              • Julian
                Adept
                • Apr 2021
                • 122

                #97
                Originally posted by Pete Mack
                NULL is the usual. But it is immaterial, since Nick will use a hard assertion.
                It’s returning an index, not a pointer.

                Comment

                • will_asher
                  DaJAngband Maintainer
                  • Apr 2007
                  • 1124

                  #98
                  Quick question:

                  Say you have:
                  Code:
                  if ((something) || (get_check(really?))) {
                  do_stuff();
                  }
                  If (something) is true, will it still ask you the get_check question?
                  Will_Asher
                  aka LibraryAdventurer

                  My old variant DaJAngband:
                  http://sites.google.com/site/dajangbandwebsite/home (defunct and so old it's forked from Angband 3.1.0 -I think- but it's probably playable...)

                  Comment

                  • Nick
                    Vanilla maintainer
                    • Apr 2007
                    • 9647

                    #99
                    Originally posted by will_asher
                    Quick question:

                    Say you have:
                    Code:
                    if ((something) || (get_check(really?))) {
                    do_stuff();
                    }
                    If (something) is true, will it still ask you the get_check question?
                    I believe the answer is no, but it's just possible it's compiler dependent - I'm not the world's greatest C expert.
                    One for the Dark Lord on his dark throne
                    In the Land of Mordor where the Shadows lie.

                    Comment

                    • Pete Mack
                      Prophet
                      • Apr 2007
                      • 6883

                      No, the second clause will not be executed. (If you use a single |, it will.)

                      Originally posted by will_asher
                      Quick question:

                      Say you have:
                      Code:
                      if ((something) || (get_check(really?))) {
                      do_stuff();
                      }
                      If (something) is true, will it still ask you the get_check question?

                      Comment

                      • will_asher
                        DaJAngband Maintainer
                        • Apr 2007
                        • 1124

                        cool. thanks.
                        Will_Asher
                        aka LibraryAdventurer

                        My old variant DaJAngband:
                        http://sites.google.com/site/dajangbandwebsite/home (defunct and so old it's forked from Angband 3.1.0 -I think- but it's probably playable...)

                        Comment

                        • Pete Mack
                          Prophet
                          • Apr 2007
                          • 6883

                          BTW, if you do decide to use boolean arithmetic (|, &, ~, ^) for any reason, always use parentheses.
                          Code:
                             x == 1 | y
                          is not synonymous with
                          Code:
                            ( x == 1)| (y)
                          Even if the unparenthesized code is correct, anyone reading it will flag it as a likely error (and the compiler will warn.)

                          Comment

                          • Julian
                            Adept
                            • Apr 2021
                            • 122

                            Originally posted by Pete Mack
                            No, the second clause will not be executed. (If you use a single |, it will.)
                            I’d not recommend using bit-operators in if clauses like this. (They’ll _work_ , at least most of the time, but it’s not what they’re for.)

                            If you want the second part of your or to evaluate regardless, either rearrange things, or just evaluate it into temporary variable first.

                            Comment

                            • Julian
                              Adept
                              • Apr 2021
                              • 122

                              Originally posted by Nick
                              I believe the answer is no, but it's just possible it's compiler dependent - I'm not the world's greatest C expert.
                              As Pete said, || is guaranteed to only evaluate the second part if the first is false.

                              This is also true of &&. It must short-circuit evaluation if the first part is false.

                              Code:
                              If (self_destruct_flag && initiate_self_destruct()) {
                                  printf("Ten seconds remain";
                              }
                              Is not going to blow you up unexpectedly.

                              (I do not, however, suggest writing your doomsday device in C. Use a safer language.)

                              Comment

                              • will_asher
                                DaJAngband Maintainer
                                • Apr 2007
                                • 1124

                                Today when I opened up Visual Studio to work on RubberBand, it gave me a ton of errors saying "pointer to incomplete class type is not allowed". These errors weren't there yesterday. I hadn't changed anything before those errors appeared and it seems to build and run just fine. Does anyone know why it might suddenly give me all these errors that weren't there when I closed VS yesterday?

                                EDIT: So I went on working on Rubberband ignoring the errors for now, and after a while they disappeared for no discernable reason. weird.
                                EDIT: ...and then they reappeared. wth?
                                Last edited by will_asher; May 24, 2021, 12:24.
                                Will_Asher
                                aka LibraryAdventurer

                                My old variant DaJAngband:
                                http://sites.google.com/site/dajangbandwebsite/home (defunct and so old it's forked from Angband 3.1.0 -I think- but it's probably playable...)

                                Comment

                                Working...
                                😀
                                😂
                                🥰
                                😘
                                🤢
                                😎
                                😞
                                😡
                                👍
                                👎