Question Nasty, insidious bug with room events, with proposed solution.

Hi everyone,

I have been trying to track down this bug for a while and I finally found it. I am using room events in my wilderness to determine if I can recycle vnums. The issue is when you delete/add a room with OLC and a room event is processed and freed.

Here is a small portion of my free_mud_event procedure:

As you can see with the comment, the issue is with storing a pointer to the room_data struct in the event. OLC willy-nilly deallocates and reallocates the room structures, invalidating the pointer. This can be REALLY hard to find, as the data remains pseudo-valid until overwritten.

So the solution - Store a different struct that contains the vnum, etc. and use real_room to get the rnum and corresponding struct if it is needed. I will be implementing this shortly and can post the solution here later, I just wanted to get this bug out in the open. Events are awesome, really, and they need to be robust!

- Ripley/Ornir Elunari @ Luminari MUD

I agree. Generally an "event" only care about an event_obj, which can be any type of structure. But some things to consider:
Rnums are almost as bad. If you allocate a room, then add or remove a room lower in the list, the whole list is renumbered to match the new count. This is actually the same issue you'd see with saving room pointers.

Given your special situation with a room pool and wilderness, I'd recommend switching the logic somewhat around:
When creating an event, save one of two types of info:
- the coordinates (for generated rooms, etc) or
- the vnum (for handbuilt rooms)
These are the only non-variables in the system.

Then use some kind of lookup to get the actual room from either of those data.

Thanks for the reply Thomas.

I plan on using VNUM, since the wilderness code does not recycle rooms with pending events, thus rooms with events remain assigned to the same coordinate location throughout the duration of the event. In fact - I am having issues here because I use events to control the allocation/deallocation of rooms.

The process is this: A player enters a coordinate location with no allocated room, pick a room from the pool and set an occupied flag. Create an event that fires in 10 seconds to see if that room is still occupied (there are many ways a room can be occupied.) If the room is not occupied, clear the flag and return 0, else return 10 to re-queue the event for another 10 seconds.

What happens, is I build some static rooms in the wilderness (say, a road) and delete one or two. Since I have been moving around I get errors as the events fire and are removed from bogus memory locations, until it crashes.

With the new system, storing a room vnum, I can use world[real_room(vnum)] and get the room, with a slight performance decrease (we have 50k rooms...And more coming. If it becomes an issue I will just have to find a better way to manage rnums.)

code forthcoming

Ok - I know I said I would provide code, but it is so ugly I feel bad sharing!

SO, instead, I will describe how to fix this and leave it as an exercise.

The problem, in a nutshell, is that the event structure has a pStruct field - This is set to a pointer to the room_data struct for the room the event is assigned to.

When you delete a room in OLC, all of the data for rooms with a HIGHER RNUM are moved down one. When you add a room, the opposite happens. Regardless, the pointer is now pointing to either the NEXT or the PREVIOUS room, depending on what you are doing and if the event is assigned to a room with an rnum above the changed room.

So for example:

This example shows how painful this can be to debug. Everything is fine with events of rooms that are lower in the world structure - Their data has not moved. Above the deleted room, however, the data has shifted downwards. The data LOOKS ok, but obviously we will run into some issues processing an event on a room other than the one it originally was assigned to.

To prevent this, you pass a pointer to the vnum (&world[rnum].number) to the event, and in mud_events.c change the add event function for rooms to make a copy of the data, allocating space and assigning the pStruct element of the event to this new data structure. When you delete room events you must also free this allocated memory.

This was my solution, and I hope this post clarified the issue a little.

Oh, I understood perfectly.

Another way to fix it would be to update all events on room deletion/addition. This is how rnums are currently handled in the OLC.

Yeah, I thought about that and may actually do it later.

I know you understood, I posted more for those with only a passing understanding of vnums and rnums - it can be one of the more difficult things to wrap your brain around starting out.