15 Jul, 2010, Rarva.Riendf wrote in the 1st comment:
Votes: 0
In case the problem is already solved, please point me to where to look, cause I searched for a while without finding.
Since I improved pet, people started to use it, and problem rised fast for the simplest reason.
pet logs with char, so they ALWAYS are char->next.
Since ROM code just loop through char_list using char->next as iterator, anything that makes ->next invalidated while in the loop screw everything.
When you code you do not do that without a safe iterator when you can modify your list.
How did you solved this ? puting is_valid(char) everytime you use a char ?
Other way ?

Personaly, since there is few loops I just made those iterator global and check when I extract char…it is ugly as hell but works.
I welcome any better solution though, I do not like the kind of hack I did but see nothing better ( I prefer to code in JAVA where I know how to solve these kind of problem, C is only for the mud)

I do not use the memory recycling factory either..I allocate and deallocate memory myself,this way valgrind can actually tell me wich variable where not initialised or if I have memory leak.
15 Jul, 2010, David Haley wrote in the 2nd comment:
Votes: 0
I thought that the standard solution here was to always store the next character in a char_next variable. It's ugly, but it works…
15 Jul, 2010, Rarva.Riendf wrote in the 3rd comment:
Votes: 0
'I thought that the standard solution here was to always store the next character in a char_next variable. It's ugly, but it works… '
Well it is ugly AND it does not work (and I had this already in the code when I took over heh.). Because it has the same logic problem, you cannot be assured this char_next wont be invalidated either (area spell as an example).
Basically this char_next was my char pet, so when I extract char, I also extract the pet ,meaning char and char_next were disapearing at the same time.
15 Jul, 2010, ralgith wrote in the 4th comment:
Votes: 0
They shouldn't be disappearing at the same time, they should be disappearing one after the other, with the char_next pointer being adjusted in between. If that isn't happening, then you know where your problem lies.
15 Jul, 2010, Rarva.Riendf wrote in the 5th comment:
Votes: 0
'They shouldn't be disappearing at the same time, they should be disappearing one after the other, with the char_next pointer being adjusted in between.'
Well I do not see how they could not disapearing at the same time.

A simple example:
You are in a violence_update loop
in this loop, a mob decide to use an area spell
he his char
char->next ANd char->next->next are in the room
he kills both, both becomes invalid. (or in my case I extract them right away)
How do you handle that ?
15 Jul, 2010, ralgith wrote in the 6th comment:
Votes: 0
I'm not sure offhand how my codebase (CircleMUD 3.1) handles it, but I don't have your issue.

But your extraction routine should be removing them one after another (since technically they die one after another, not at the same time) and thus removing them from the char_data struct one after another as well.
15 Jul, 2010, David Haley wrote in the 7th comment:
Votes: 0
Well, you can also truly remove at the end of each round, and in between mark as invalid as you said earlier. Or, you can just make sure that char_next is correctly maintained at all times.
15 Jul, 2010, Rarva.Riendf wrote in the 8th comment:
Votes: 0
'But your extraction routine should be removing them one after another (since technically they die one after another, not at the same time) and thus removing them from the char_data struct one after another as well. '
Off course they are removed one at a time, but your loop is in no way warned than the pointer has been modified.
Maybe you do not have this problem because you do not actually remove the char (but that means you have to check for char_validity everywhere before to do any action on them. I think I read that this is how CircleMud handle it.
In my case extract_char actually remove the char and free the memory
so char_next is now a totally invalid pointer.
ie
for (char = char_list; char; char = char_next) {
char_next = char->next;
call to a method than ends up killing char char->next and char->next->next (as an exemple);

what do we do now ?
My solution-> char_next is global, if I see I extract it, I replace it with the next valid char
solution 2: make the char_invalid -> need to check for char validity everywhere before doing anything on him (in my case I check for null. but at least it crashes if I forget too, better than weird behaviour)
solution 3 ?
}
15 Jul, 2010, David Haley wrote in the 9th comment:
Votes: 0
You can also make a local copy of the list before iterating over it, and making sure that list nodes are separate from the things in the list. (I never liked the munging together of container and contained items…)
15 Jul, 2010, ralgith wrote in the 10th comment:
Votes: 0
Actually looks like CircleMUD has a flag NOTDEADYET that is set on "killed" players, and all are extracted at the END of pulse_violence
15 Jul, 2010, Rarva.Riendf wrote in the 11th comment:
Votes: 0
Funny no one seems to have the problem but me….
Because when I look at this code

void violence_update( void )
{
CHAR_DATA *ch;
CHAR_DATA *ch_next;
CHAR_DATA *victim;

for ( ch = char_list; ch != NULL; ch = ch->next )
{
ch_next = ch->next;
if ( ( victim = ch->fighting ) == NULL || ch->in_room == NULL )
continue;

if ( IS_AWAKE(ch) && ch->in_room == victim->in_room )
multi_hit( ch, victim, TYPE_UNDEFINED );
else
stop_fighting( ch, FALSE );

if ( ( victim = ch->fighting ) == NULL )
continue;

/*
* Fun for the whole family!
*/
check_assist(ch,victim);
}
return;
}

multi_hit can kill everyone in a room if the mob has 'area attack', and could even kill himself…
and check_assist could too (and that is another funny ROM problem, that leads to char having two rounds of attack in one loop…but that is out of scope heh)

I cannot believe that with the sheer number of rom derivated mud, none implemented area attacks without the need of finding a solution to this basic problem.
And one that does not look like a ugly hack..or maybe I am too optimistic.


@ralgit: Ok i thought so, but I have a ROM based code to maintain heh…unfortunately….
15 Jul, 2010, David Haley wrote in the 12th comment:
Votes: 0
Quote
I cannot believe that with the sheer number of rom derivated mud, none implemented area attacks without the need of finding a solution to this basic problem.
And one that does not look like a ugly hack..or maybe I am too optimistic.

I wouldn't say that nobody has implemented this, however it is likely that many people stopped because it's annoying to deal with these issues nicely.

Other codebases solve this issue differently, such as properly extracting characters only at the end of a round (so that lists they are in keep integrity).
15 Jul, 2010, ralgith wrote in the 13th comment:
Votes: 0
@Rarva: Why not simply add a flag like CircleMUD has to your ROM codebase? I can post my violence routine if that would help you out.
15 Jul, 2010, ralgith wrote in the 14th comment:
Votes: 0
Actually, here's the start of my damage function, which is where the key things are at:
/*
* Alert: As of bpl14, this function returns the following codes:
* < 0 Victim died.
* = 0 No damage.
* > 0 How much damage done.
*/
long damage(struct char_data *ch, struct char_data *victim, long dam, int attacktype)
{
int i;

if (GET_POS(victim) <= POS_DEAD) {
/* This is "normal"-ish now with delayed extraction. -gg 3/15/2001 */
if (PLR_FLAGGED(victim, PLR_NOTDEADYET) || MOB_FLAGGED(victim, MOB_NOTDEADYET))
return (-1);

log("SYSERR: Attempt to damage corpse '%s' in room #%d by '%s'.",
GET_NAME(victim), GET_ROOM_VNUM(IN_ROOM(victim)), GET_NAME(ch));
die(victim, ch);
return (-1); /* -je, 7/7/92 */
}


You can see it calling if the victim is already a corpse, that's to make sure the victim is set up to be extracted like they should be as well.
15 Jul, 2010, Rarva.Riendf wrote in the 15th comment:
Votes: 0
Heh I thank you both, your solution is off course valid, but I would have to rewrite a hell lot of routine to take it in account.
It would be nicer though, I agree (ROM really sucks heh…)
Guess I will stick with my ugly hack till I whip myself to do it properly…

I still wonder why I could not find any discussions about it concerning ROM..maybe those are lost…
16 Jul, 2010, Kaz wrote in the 16th comment:
Votes: 0
Having a bit of a lolwut moment. Surely this is wrong…

for ( ch = char_list; ch != NULL; ch = ch->next )
{
ch_next = ch->next;
/* … */


This should be:

for ( ch = char_list; ch != NULL; ch = ch_next )
^^^^^^^
{
ch_next = ch->next;
/* … */
16 Jul, 2010, Rarva.Riendf wrote in the 17th comment:
Votes: 0
Rofl I did not even pay attention.
You are right of course, I just copy pasted the code from the original ROM code….
My code is now quite different snicker.

Both does not work anyway :p
16 Jul, 2010, Kaz wrote in the 18th comment:
Votes: 0
Rarva.Riendf said:
Rofl I did not even pay attention.
You are right of course, I just copy pasted the code from the original ROM code….
My code is now quite different snicker.

Both does not work anyway :p


Indeed. As was already mentioned, if you have a function that can remove an object from the list while it is being processed, then you need to flag them as not-to-be-processed instead, and remove all the not-to-be-processed objects after the list processing has completed.
16 Jul, 2010, JohnnyStarr wrote in the 19th comment:
Votes: 0
What I'm trying to come up with, is a good way to take advantage of std::list and yet, support the legacy singly LL.
To do this, I've thought about keeping the same handler functions like obj_to_char and obj_from_char for adding and removing list objects. I would just make sure to add or remove the game-object to the std::list somewhere in the function.

Now you don't have to worry about unsafe pointers so much. I wouldn't use a straight STL container though, it would have to be a wrapper object that would perform the same exact operations as those found in handler.c

Not to say that this is ideal, but then again, what is when refactoring Diku code?
16 Jul, 2010, David Haley wrote in the 20th comment:
Votes: 0
The problem is not just maintaining integrity of the container being iterated over, it's also maintaining integrity of the items you iterate over. Even if your container is fine, if you have a dangling pointer to a deleted object you'll be in trouble.
0.0/40