12 Nov, 2008, Sharmair wrote in the 82nd comment:
Quote
I thought that the original problem here was that the victim might have died and possibly been extracted as a consequence. It sounded to me earlier like you were saying that the victim dying wasn't an issue, but I think based on what you just said that it's not an issue only as far as the loop counter is concerned. Is that what you meant?
The code referred to here (and now a new problem with the new line added in the wrong place) is for
an extra attack on a victim fighting the actor (ch in the main loop) or a member of the actor's group,
and is part of the added weapon code. The original problem was in the primary victim of the actor dieing
in the main attack BEFORE this weapon code, and crashing when it was checking if the victim (now
extracted) can see the attacker. In this thread a number of things not really related to the main problem
were brought up. One of those being the need for a holder next in this loop. All the added info was
confusing the main issue and I mentioned as a side note that the holder was not really needed in this
loop as it was. As it turned out, in adding that holder, it was placed between the for clause and body (thus replacing the body) and making a new bug. But, this was not the victim that was crashing originally. I
hope that clears up the context of what all I am saying.
14 Nov, 2008, Igabod wrote in the 83rd comment:
Sharmair said:
No, not if you just move it up a line like I said. A lot of C/C++ statements (like for, if, while etc) have a
syntax that takes a statement after them (the closing ')'). If you have more then one, you use a
compound statement (ie. you enclose them in {}). In your case here, the for loop only has the setting
of the next in it and nothing else:
Is your whole loop.
After the for statement, vch is NULL, the exit condition. Then you define a scope
(the use of the {} after this) where you TRY to dereference vch and being it is NULL,
you get a crash. If it did not crash, that break would have broke you out of the main
violence_update() loop (skipping all the rest of the characters in the game after the current
ch).
In short, if you are going to use that vch_next holder, it should be in the loop's compound
statement, like:
Just to make it clear, you just have to MOVE the one { not add or delete any.
Igabod said:
just the { ??? wont that cause an error?
No, not if you just move it up a line like I said. A lot of C/C++ statements (like for, if, while etc) have a
syntax that takes a statement after them (the closing ')'). If you have more then one, you use a
compound statement (ie. you enclose them in {}). In your case here, the for loop only has the setting
of the next in it and nothing else:
for (vch = ch->in_room->people; vch != NULL; vch = vch_next)
vch_next = vch->next_in_room;
Is your whole loop.
After the for statement, vch is NULL, the exit condition. Then you define a scope
(the use of the {} after this) where you TRY to dereference vch and being it is NULL,
you get a crash. If it did not crash, that break would have broke you out of the main
violence_update() loop (skipping all the rest of the characters in the game after the current
ch).
In short, if you are going to use that vch_next holder, it should be in the loop's compound
statement, like:
for(vch = ch->in_room->people; vch != NULL; vch = vch_next){
vch_next = vch->next_in_room;
//…rest if loop here (those 2 ifs)
}Just to make it clear, you just have to MOVE the one { not add or delete any.
ok i did that and get a crash again. here's the gdb report.
Program received signal SIGSEGV, Segmentation fault.
room_is_dark (pRoomIndex=0x0) at handler.c:2539
2539 if (pRoomIndex->light > 0)
(gdb) tb
Breakpoint 1 at 0x4433e3: file handler.c, line 2539.
(gdb) bt
#0 room_is_dark (pRoomIndex=0x0) at handler.c:2539
#1 0x00443628 in can_see (ch=0x12975fc, victim=0x13297f0) at handler.c:2640
#2 0x00437672 in violence_update () at fight.c:107
#3 0x00479f33 in update_handler () at update.c:1199
#4 0x0042c442 in game_loop_unix (control=5) at comm.c:844
#5 0x0042bed8 in main (argc=2, argv=0x11419b0) at comm.c:448
(gdb)
sorry it's been a few days but i've been battling the flu and have slept for the past 2 days.
14 Nov, 2008, quixadhal wrote in the 84th comment:
Igabod said:
Program received signal SIGSEGV, Segmentation fault.
room_is_dark (pRoomIndex=0x0) at handler.c:2539
2539 if (pRoomIndex->light > 0)
(gdb) tb
Breakpoint 1 at 0x4433e3: file handler.c, line 2539.
(gdb) bt
#0 room_is_dark (pRoomIndex=0x0) at handler.c:2539
#1 0x00443628 in can_see (ch=0x12975fc, victim=0x13297f0) at handler.c:2640
#2 0x00437672 in violence_update () at fight.c:107
#3 0x00479f33 in update_handler () at update.c:1199
#4 0x0042c442 in game_loop_unix (control=5) at comm.c:844
#5 0x0042bed8 in main (argc=2, argv=0x11419b0) at comm.c:448
(gdb)
It looks like we've come full circle. This is the same crash you had earlier, no? During your violence_update(), some character involved in a fight has been extracted from the room, and then a can_see() check is attempted. Part of can_see() checks to see if the room the victim is in, is set to dark, but they aren't IN a room anymore, so…. BOOM.
From a conceptual standpoint, the victim's death really shouldn't happen until the very bottom of the update cycle, since attacking a dead person SHOULDN'T pose a problem (other than wasting mana/ammo/whatever). However, it would appear that the death is actually managed somewhere in the damage() function… Thus, any call to damage(), which includes calls to one_hit() and multi_hit(), as well as pretty much any skill or spell function, is likely to result in a character that is not only dead, but removed from the battle scene.
To properly fix this, you either have to ensure that ALL combat actions get routed through damage(), which checks that victim->position is DEAD before continuing, or you need to redesign the section that handles death and move it to a seperate part of the update loop, so it's only ever done once per round, AFTER all combat actions have occured.
15 Nov, 2008, Igabod wrote in the 85th comment:
oh yippee either route seems like it would take way more skill than i have.
16 Nov, 2008, Sharmair wrote in the 86th comment:
Post your violence_update() function again so we can see what we are working with now.
16 Nov, 2008, Igabod wrote in the 87th comment:
void violence_update (void)
{
CHAR_DATA *ch;
CHAR_DATA *ch_next;
CHAR_DATA *victim;
CHAR_DATA *vch_next;
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);
/*
* Swords will very rarely get in an extra round.
*/
if (get_weapon_sn (ch) == gsn_sword
&& ch->wait <= 0)
{
int chance;
CHAR_DATA * was_fighting;
CHAR_DATA * vch;
chance = get_skill (ch, gsn_sword) / 15;
/*
* Penalty if the opponent can see.
*/
if (can_see (victim, ch))
chance = chance * 3 / 5;
for (vch = ch->in_room->people; vch != NULL; vch = vch_next){
vch_next = vch->next_in_room;
if (vch->fighting == ch
|| is_same_group (vch->fighting, ch))
{
if (number_percent () < chance)
{
was_fighting = ch->fighting;
ch->fighting = vch;
multi_hit (ch, vch, TYPE_UNDEFINED);
check_improve (ch, gsn_sword, TRUE, 2);
ch->fighting = was_fighting;
WAIT_STATE (ch, PULSE_VIOLENCE);
break;
}
}
}
}
}
else
stop_fighting (ch, FALSE);
if ((victim = ch->fighting) == NULL)
continue;
/*
* Fun for the whole family!
*/
check_assist (ch, victim);
if (IS_NPC (ch))
{
if (HAS_TRIGGER (ch, TRIG_FIGHT))
mp_percent_trigger (ch, victim, NULL, NULL, TRIG_FIGHT);
if (HAS_TRIGGER (ch, TRIG_HPCNT))
mp_hprct_trigger (ch, victim);
}
}
return;
}
16 Nov, 2008, David Haley wrote in the 88th comment:
Aren't we back to where we were before? The victim can die in multi_hit, and then you try to do can_see with the victim.
17 Nov, 2008, Sharmair wrote in the 89th comment:
Yea, he never put in the fix I told him for the main problem. Again, using the line numbers from
this new post, COPY lines 63 and 64 (the check this code seems to use to detect if victim is good)
at line 18 (right after the main multi_hit() call). One of my last posts had the exact code for this
(message 68 in this thread) if you don't understand this way of presenting it.
this new post, COPY lines 63 and 64 (the check this code seems to use to detect if victim is good)
at line 18 (right after the main multi_hit() call). One of my last posts had the exact code for this
(message 68 in this thread) if you don't understand this way of presenting it.
17 Nov, 2008, Sandi wrote in the 90th comment:
/*
* Penalty if the opponent can see.
*/
if (victim->in_room != NULL && can_see (victim, ch))
chance = chance * 3 / 5;
What you're coding here is the chance that a wild swing with a sword will hit an innocent bystander. Why does the chance of this increase with skill? Or, whether or not your opponent can see how sloppy you are? Should these not be the other way 'round? And why the happy accident that you ONLY hit a random enemy?
In principle, I'm still bothered by the idea of checking for a rare occurrence in the most frequent update you have. But just for the sanity of the code, this would make more logical sense as an alternative to a "miss" in one_hit. "Victim has too much AC for a hit? Oh, your sword bounced off and hit someone else."
* Penalty if the opponent can see.
*/
if (victim->in_room != NULL && can_see (victim, ch))
chance = chance * 3 / 5;
What you're coding here is the chance that a wild swing with a sword will hit an innocent bystander. Why does the chance of this increase with skill? Or, whether or not your opponent can see how sloppy you are? Should these not be the other way 'round? And why the happy accident that you ONLY hit a random enemy?
In principle, I'm still bothered by the idea of checking for a rare occurrence in the most frequent update you have. But just for the sanity of the code, this would make more logical sense as an alternative to a "miss" in one_hit. "Victim has too much AC for a hit? Oh, your sword bounced off and hit someone else."
17 Nov, 2008, David Haley wrote in the 91st comment:
Sandi said:
"Victim has too much AC for a hit? Oh, your sword bounced off and hit someone else."
Tee hee. Sounds like a great feature to me. :grinning:
17 Nov, 2008, Igabod wrote in the 92nd comment:
Sharmair said:
Yea, he never put in the fix I told him for the main problem. Again, using the line numbers from
this new post, COPY lines 63 and 64 (the check this code seems to use to detect if victim is good)
at line 18 (right after the main multi_hit() call). One of my last posts had the exact code for this
(message 68 in this thread) if you don't understand this way of presenting it.
this new post, COPY lines 63 and 64 (the check this code seems to use to detect if victim is good)
at line 18 (right after the main multi_hit() call). One of my last posts had the exact code for this
(message 68 in this thread) if you don't understand this way of presenting it.
that was the fix i was looking for, must've missed that post while skimming over the posts that were davidhaley and runter's little debate about some other subject i didn't care about at the time. thank you all for the assistance. i'll be sure to update you if further testing results in problems with this.
Random Picks
In fact, what's going on here is a good example of why you want to use braces immediately after the loop/if/whatever. :smile:
and in effect, vch (or at least the loop iteration) goes out of scope, it does not matter. The point is that
as the loop is not continued after the loop pointer might have gone invalid, a vch_next holder is not needed.
It might be argued that a new coder might not be able to determine when a holder is needed and should
use one all the time. But this was brought up as a side issue (that had nothing to do with his original
problem) and was just one more thing that has muddled the whole issue.
I thought that the original problem here was that the victim might have died and possibly been extracted as a consequence. It sounded to me earlier like you were saying that the victim dying wasn't an issue, but I think based on what you just said that it's not an issue only as far as the loop counter is concerned. Is that what you meant?