Here's some code I came up with for C that seems to be doing the job pretty well.
The iterator is also reusable anywhere that you would want to traverse all of the rooms for whatever reason efficiently.

// Just returns the first room of our list.
ROOM_INDEX_DATA *get_room_iterator() {
    return room_index_hash[0];
}

// Returns the next iterator
ROOM_INDEX_DATA *next_room_iterator(ROOM_INDEX_DATA *it) {
    ROOM_INDEX_DATA *pRoomIndex = it->next;
    int i = (it->vnum % MAX_KEY_HASH); // Resume in the bucket we left off at.

    if (pRoomIndex == 0)
        while(++i < MAX_KEY_HASH && !pRoomIndex)
            pRoomIndex = room_index_hash[i];

    return pRoomIndex;
}

// Invalid random rooms are rooms that are private or safe, or can become private. All other rooms are valid.
// Other checks should be made after the get_rand_room() call to see if you need to call it more
// than once. Like things effecting a certain character. Like can_see_room(ch, room) etc.
int valid_random_room(ROOM_INDEX_DATA *pRoom) {
    if (room_is_private((ROOM_INDEX_DATA*)pRoom) ||
        IS_SET(pRoom->room_flags, ROOM_PRIVATE) ||
        IS_SET(pRoom->room_flags, ROOM_SOLITARY) ||
        IS_SET(pRoom->room_flags, ROOM_SAFE))
        return 0; // false
    return 1; // true
}

// Grabs a random valid room for selection.
ROOM_INDEX_DATA *get_rand_room() {
    static int count;
    static ROOM_INDEX_DATA **array; // Dynamically assigned after we get the count.
                                    // If it's the same array then there's no reason to make a new one across calls.
    ROOM_INDEX_DATA *it; // Iterator for later use.

    if (!count) // Generators the count once.
        // Let's traverse all of the rooms in the mud and get a count.
        for(it = get_room_iterator(); it != 0; it = next_room_iterator(it))
            if (valid_random_room(it))  // If the room is indeed meeting our criteria
                count++;

    if (!array) {// Generates the array once.
        array = (ROOM_INDEX_DATA**) malloc(sizeof(ROOM_INDEX_DATA*) * count);
        for(it = get_room_iterator(); it != 0; it = next_room_iterator(it))
            if (valid_random_room(it))  // The room meets the criteria. Add it to our array.
                array[(count–) - 1] = it;
    }

    // Now simply return a valid room.
    return array[number_range(0, count-1)];
}

The room checks are still in place but the ch specific checks should be done after the call, and recall it if the check didn't flesh out.
This is because you might want to grab a random room for some purpose other than sending a character to it.

That's not what you said earlier:

Note that you say that the flag has to be set and there are two people. Under that assumption, the following holds: "room_is_private <=> private && some_other_condition(s)"

But obviously if that earlier statement was wrong, well, the above assumption doesn't hold. :smile:

The conditions that make the room private are separate from the fact if a flag is set or not. It's really not that hard to understand. If the flag is set the room MAY be private. If you want to know if the room CAN become private you need to check to see if the flag is set. If it is set it DOES NOT have to be private.

I said the conditions that make the room private are separate from the private flag because they are. A room can be private without having the flag set at all. One of the ways a room becomes private is if it has the flag set and 2 or more people currently occupy it. Another way is if it has the solitude flag set and there is already 1 or more people in it. There's ways for it to be private without either of those flags set. They have absolutely nothing to do with the logic behind the room being private, regardless of how many logic tables we draw.

Edit: And for the record I've never said anything different from that. If you've misunderstood it's probably because you're commenting on a rom features without knowing how the rom features work, obviously.

Oookk… I think there is some serious confusion here somewhere.

Let's take this one thing at a time. First off, is it or is it not the case that: room_is_private <=> private && some_other_condition(s)

Where "private" means "the private flag is set".

I'll explain this one more time so anyone reading this thread can understand it and that's all I have to say on the subject.

I don't have to draw a logic chart to explain this.

The flag is only used to determine if the room can possibly become private by means of 2 people or more being in the room. The flag doesn't mean it's private. The check is_room_private returns true if the room is private.

What does this mean?

It means that if you want to know if the room can ever be private, calling that check isn't enough my friends.

Why is that?

Because even with the flag set the room doesn't have to be private, but it gives it the opportunity to be in the future.
The same thing goes for the solidarity flag.

If those aren't checked for then you may be issuing a room that could become private at a later time. Which for the purpose of the get_random_room() code in rom means it would be wrong without those flag checks.


That's all I have to say on the subject, if someone else wants to dispute it they can hit their head against a brick wall all day long, cause I'm done.

Edit: And to be totally clear, it can become private from means other than even having any flags set. Ownership of a room, or future additions can cause a room to be private.

I really don't see why you feel the need to be patronizing or (albeit indirectly) insult my intelligence. I am trying to ask you a simple question because there is obviously confusion somewhere and you keep answering questions I didn't ask.

I'm trying to go by the things you have said. You have said that is_room_private returns true if the flag is set, and there are two people. I'll ask the very simple question again, in an attempt to remove all ambiguity: is that or is not that the case?

If that is not the case, then fine, this all goes away. If that is the case, I already explained why I believe the logic (not the intention) says that there is redundancy. Instead of waving me away, I would really appreciate an explanation of where my reasoning fails.

What I've said about it returning true if the flag is set and more than 1 person is in the room is true, and it also returns true for other reasons completely separate. Which is why time and time again I have said it has nothing to do with only the flag being set. There are other conditions that can constitute a room being private so the call is still needed even with the flag checks. Conditions like ownership of the room. The room being an immortal room, and future additions that could make a room always private.

OK. Thank you for answering that. In that case, do you or do not not agree that this:

!room_is_private(room) && !IS_SET(room->room_flags, ROOM_PRIVATE)

is equivalent to:

!(IS_SET(room->room_flags, ROOM_PRIVATE) && some_other_conditions) && !IS_SET(room->room_flags, ROOM_PRIVATE)

which, applying De Morgan's, is in turn equivalent to:

(!IS_SET(room->room_flags, ROOM_PRIVATE) || !some_other_conditions) && !IS_SET(room->room_flags, ROOM_PRIVATE)

where some_other_conditions includes there being more than two people and all these other things you are talking about.

And it doesn't matter because it's still not redundant to do the check. It's no more redundant than if we had two separate checks: is_room_private() and can_room_become_private()

I'm not asking if it matters, please just bear with me. Do you or do you not agree that the conditions I listed are equivalent? My question pertains only to the text, not the intention of other context.

The issue at hand is do we need to do flag checks after is_room_private. The answer is yes, you do. With the way the system works if you elect to use is_room_private you still need to do the checks. It's not a separate situation. The flag checks are only there to see if the room can ever be private in the future.

I've already said the entire thing could be rewritten but the idea that the flag checks aren't needed or are redundant are not true. The truth is there should be a can_room_become_private() and then you just need to call that. But since there isn't, there's nothing redundant there.

Please, that is not the question I asked…

And I'm fully aware of the point you're attempting to come to. The point is irrelevant because you're working off of the foundation that was place:

The foundation that the flag checks were unneeded because is_room_private had been called.

If that's the case then removing those flag checks should be fine. And it's not fine. It produces bugs. There's no fix to do within is_room_private either.

The whole situation is ridiculous that you're trying to swing around another argument using boolean logic when the fact stands. Those checks are not redundant.

The only conclusion I can come to if you're seriously suggesting that logic is not helpful is that you have not told me everything that is going on. I am not working off of any "foundation" other than the logic of the checks that I am being told about. I have zero prejudice about this code. I don't know what the intention is. All I'm doing is looking at the logic.

Why do you not answer my question? Is there a flaw in the equivalences I gave, and if so, where?

Since we are rewriting history before we go too far let's take a look at the past.

In response to my explanation of why the code as it stands is not redundant(which it is not) here is what you said.



Which is like saying it's redundant. After I explain why it's not redundant you come back with some boolean logic and:




So here's where the crux of the matter really is. I've already said many times that the code could be rewritten to be done better, but the question of redundancy is were the checks already accomplished in is_room_private(). And the answer is no. Therefore, the checks aren't redundant. The only way logically you can argue that they are redundant is to say that without those checks the code would operate the same. And it will not.

So to recap, the whole debate is over the redundancy of the additional checks after is_room_private(). There is no redundancy. They are required. It's just a fact.

edit: Of course we can rewrite the code to be done better, but that means not using is_room_private because it needs to remain the way it operates for other places.

I said that because I believe that the logic says that. I have no idea what the intention of the code is. I have no idea why things would break if this is changed because of sound rearranging of boolean conditionals.

You are still refusing to answer my question about the equivalences. I don't know why you are doing that. It is a very simple question, and should have a very simple answer. Either the conditions I gave are equivalent, or they are not and you can tell me why.

This is what you call talking in circles. I'll leave it up to the readers to decide if they want to remove those "redundant" calls. It just might take them a while to realize they are getting invalid rooms afterwards.

Of course it's talking in circles, because you refuse to give me a straight answer even when given simple and direct questions!

(!A || !B) && !A
is true exactly when
!A
is true. End of story.

If the equivalences hold, then we are in the above situation. If they don't, then we are not. It is very simple. Don't look at me for an explanation why sound boolean logic operations break the code. My only explanation is that more things are happening than have been said before.

This could have been solved ages ago if you'd just answer straight questions. :sad:

I managed to grill steak before it started raining. Cheap steak, but steak nonetheless. Tasty!

While I was eating my steak and a baked potato, you guys argued about some things up there.

It might be helpful to show the actual function in question:

bool room_is_private( ROOM_INDEX_DATA *pRoomIndex )
{
    CHAR_DATA *rch;
    int count;


    if (pRoomIndex->owner != NULL && pRoomIndex->owner[0] != '\0')
        return TRUE;

    count = 0;
    for ( rch = pRoomIndex->people; rch != NULL; rch = rch->next_in_room )
        count++;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_PRIVATE)  && count >= 2 )
        return TRUE;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_SOLITARY) && count >= 1 )
        return TRUE;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_IMP_ONLY) )
        return TRUE;

    return FALSE;
}

This is one of the classic cases where we should really have a is_private() and a can_be_private() pair of functions. A great many things tend to break down into is_foo() and can_foo(), and this is one of them. The companion function would look something like

bool room_can_be_private( ROOM_INDEX_DATA *pRoomIndex )
{
    if (pRoomIndex->owner != NULL && pRoomIndex->owner[0] != '\0')
        return TRUE;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_PRIVATE))
        return TRUE;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_SOLITARY))
        return TRUE;

    if ( IS_SET(pRoomIndex->room_flags, ROOM_IMP_ONLY) )
        return TRUE;

    return FALSE;
}

If we had a room_can_be_private(), we wouldn't need to check both the flags (which are a pre-requisite for SOME of the conditions) and call room_is_private().

There we go – it is that simple. The function is testing more than a conjunction of conditions. It is a disjunction. This would have been solved many, many posts ago if that simple, direct question about the equivalences had been answered. :rolleyes: