Issue with Affects

Posted by Trom on Tue 31 Dec 2013 05:47 AM — 9 posts, 31,190 views.

Trom Tue 31 Dec 2013 05:47 AM #0

I can't seem to figure out this issue. When Dispel Magic is cast on myself (no chance of failure) it dispels 4 of 5 of the spells. It always dispels all but one spell. I put an log_string() to list every spell that it finds in the loop and it finds all but the last one every time. Then the second cast works and gets the last one. The problem is that the loop shouldn't be skipping the last spell.

Another strange issue is that if I don't test for IS_VALID, it will list something like 650+ spells even though the character has less than 5.

This is the main part of the code from spell_dispel_magic. It takes the affects one by one and (with no chance of failure because ch == victim) it dispels each spell that is found within affects. Affects are loaded from the player file or is on them just after cast (checked both times).


for ( paf = victim->affected; paf != NULL; paf = paf_next )
	{
		paf_next = paf->next;

		if ( paf->where != TO_AFFECTS )
			continue;

		if ( !IS_VALID(paf) )
			continue;

                found = true;

		// Self casting is what's being tested so it will always go to ELSE
		if ( perform_save(victim,STAT_INT,16) && victim != ch )
		{
			strcpy( buf, skill_table[paf->type].name );
			strcat( buf, " was not {Rdispelled{x\n\r" );
			send_to_char( buf, victim );
			fail = TRUE;
		}
		else
		{
			strcpy( buf, skill_table[paf->type].name );
			strcat( buf, " was {Gdispelled{x\n\r" );
			send_to_char(buf, victim);
			affect_strip(victim,paf->type);
			success = TRUE;
		}

	}

Dentin Tue 31 Dec 2013 05:55 PM #1

If you're seeing 650+ invalid spells, my guess would be that your loop is somehow getting derailed and looping through the list of destructed affects. One thing to do would be to add a debug print loop -before- this one, and have it dump out the list of current affects, including the pointer to each affect and the valid/invalid flag. Then, do something similar in the destruct loop, and see how they differ.

If the debug loop also prints out 650 invalid spells, then you've got a problem with affects in general.

If the dispel loop is different from the debug loop, then the dispel loop has a problem.

Good luck.

Trom Wed 01 Jan 2014 03:17 AM #2

Well the victim in this case will be the caster since they are casting on themselves. Ideally it's supposed to loop through the affects of the caster and dispel each spell one by one.

Yeah the troubling part is that if IS_VALID is not there, it should still list only the spells on the player since the spells put on a player all have a 'where' set to TO_AFFECTS. Yet it lists a ton of spells that aren't on the player. I think I've confirmed that by checking the fread_char to load_char_obj() routine (ROM's method of translating a text file into a player filled object). I've also checked each of the spells I've used to make sure they are set correctly, they all have a 'where' set to TO_AFFECTS.

I'm also doing this from a freshly executed mud so the memory should be fresh and clean, so I'm not sure where the 600+ spells are coming from, quite a few are repeats.

Dentin Wed 01 Jan 2014 03:51 AM #3

I think the next thing I would do is log in two characters at the same time, and see what the dump looks like on both of them. If it's a list corruption issue, you should see pretty much the same stuff attached to both players.

Whatever you do though, I think its worth taking the time to hunt down where the invalid spells are coming from. Good luck.

Trom Wed 01 Jan 2014 06:26 PM #4

I agree with the emphasis. The listing of many spells that aren't on the player is a fairly interesting issue. I've been looking into it and thought I found the problem, now I'm uncertain.

The following is from recycle.c and is the method in which a new affect is created.


AFFECT_DATA *new_affect(void)
{
    //static AFFECT_DATA af_zero;
    AFFECT_DATA *af;

    if (affect_free == NULL)
		af = alloc_perm(sizeof(*af));
    else
    {
		af = affect_free;
		affect_free = affect_free->next;
    }

    //*af = af_zero;


    VALIDATE(af);
    return af;
}

I disabled the af_zero since it didn't seem like it was doing anything and was potentially an old version of what affect_free already is. However the problem persists, when someone casts dispel on itself i debug dumped the spells found and now it goes up to 950+ spells. The log would then stop at "write_to_descriptor: Resource temporarily unavailable" and stop (without crashing the mud or disconnecting the player).

My next concern is that maybe I'm passing an copy of 'af' instead of a pointer. So I'm still messing around with this, if you guys get any ideas feel free to say.

Dentin Thu 02 Jan 2014 07:39 PM #5

As a matter of policy, I would set af->next to NULL before returning from new_affect(). It's probably also a good idea to set all the other fields to a default value (which is probably what the af_zero line does), but definitely set all pointers in a newly allocated structure to a sane value in the allocator.

I don't know if that will fix this specific problem, but it's definitely saved me a heap of trouble in the past.

Trom Fri 03 Jan 2014 08:10 AM #6

I figured where the issue is though I don't understand it. It has to do with how affects list is handled when affects are removed using affect_strip. The function specifically cycles through the list of affects that a player has and removes any affects that match the skill number given:

affect_strip(ch,sn)


        paf = ch->affected;
	while (paf != NULL)
	{
		paf_next = paf->next;

		if (paf->type == sn)
			affect_remove(ch, paf);

		paf = paf_next;
	}

This for some reason the above makes the list go crazy. If dispel magic uses 'affect_remove()' instead of 'affect_strip()' than no craziness occurs. The problem is that for spells like Frenzy that have multiple affects, removing only one piece of the spell is not actually dispel the spell.

The variables are set to NULL to start to avoid that problem. This is the affect_remove() function.


void affect_remove( CHAR_DATA *ch, AFFECT_DATA *paf )
{
    int where;
    int vector;
    if ( ch->affected == NULL )
    {
		bug( "Affect_remove: no affect.", 0 );
		return;
    }
    affect_modify( ch, paf, FALSE );
    where = paf->where;
    vector = paf->bitvector;
    if ( paf == ch->affected )
    {
		ch->affected	= paf->next;
    }
    else
    {
		AFFECT_DATA *prev;
		for ( prev = ch->affected; prev != NULL; prev = prev->next )
		{
			if ( prev->next == paf )
			{
				prev->next = paf->next;
				break;
			}
		}
		if ( prev == NULL )
		{
			bug( "Affect_remove: cannot find paf.", 0 );
			return;
		}
    }
	if (IS_SET(ch->affected_by, paf->bitvector))
		REMOVE_BIT(ch->affected_by, paf->bitvector);
    free_affect(paf);
    affect_check(ch,where,vector);
    return;
}

Since affect_remove works in place of affect_strip, i'm guessing the problem doesn't involve affect_remove. The function was included just incase. I should probably also mention that performing just a straight loop listing the affects works fine. It does list all the proper spells and nothing more.

Dentin Fri 03 Jan 2014 05:34 PM #7

Looks to me like you're ending up doing a double remove somewhere along the line - one part of the code is removing the spell from the list and moving it to empty, then at some point later you're removing it from empty and re-adding it to empty.

I don't know how this code is set up, but I know how we've avoided this issue on Alter Aeon: by only having two functions. We have rem_spell_char(), which removes the spell affects as well as the spell structure from a character, and free_spell_list(), which destroys all effects on a character without removing them.

I don't know how effect_remove and effect_strip differ, but I'm guessing one of them is technically redundant. If not, perhaps a rename to make their purpose more obvious is in order.

Trom Sat 04 Jan 2014 05:24 PM #8

I'm somewhat figured out the issue. Though I believe all the involved pieces of code are available in this thread. I was using a loop through affects within dispel_magic in order to remove each affect. The function strip_affects was called when applicable and goes through the list with another pointer and removes all skills that match. The problem seemed to be that the original loop and affects then leaves a blank space (if two spells are removed) so when the next node is up for checking, it's actually blank with randomized values.

The part that confuses me is that the removal code in affect_remove appears to be of proper specification. It should not be an issue in the main loop of affects, when the affects are removed by another pointer, since the original list is still a pointer to the same list, it should also be modified and ready to go.

So to fix this weirdness I generated a list of skill numbers before hand by looping through the list. Then used the skill numbers to then affect_strip. This fixes the strange issue.

Thank you for the suggestions and help.