[Any version] [SW] Incorrect Coolie deaths

[sinisterseed]

This is not a new bug but rather something that was reported a while ago elsewhere and I'm just making a new topic for it to prevent it from getting lost for good.

I just remembered that there was a slight behavioral inaccuracy when it comes to the death of the Coolies in SW, more exactly, that their heads do not "melt" completely when they die, but not turn into a ghost too, their heads disappearing only when raising from their dead bodies. This is a small deviation from vanilla which, as far as I can tell, does not impact the actual gameplay in any capacity, at least not in a way I can identify, so it appears to be a visual-only kind of deal, but it also might as well not be.

My investigation reveals that VoidSW has the correct, vanilla Coolie behavior - same for Redux for that matter, but I already knew this -, and I've managed to trace it down to SWP which is the port that did in fact alter their behavior and the change managed to slip into Raze back when development first emerged.

[mjr4077au]

I don't think Raze has ever included SWP code, though? I wonder if it's just some kind of hack job that's been exposed by the better backend code?

[sinisterseed]

I don't think Raze has ever included SWP code, though? I wonder if it's just some kind of hack job that's been exposed by the better backend code?

No it did, and this was there from basically day-1.

It's story time: Back when Raze originally started development VoidSW was in a very rough state and not ready for any public release - it wasn't even in a compilable state until December if I remember correctly -, so Graf had to use something else as a reference in addition to the-then largely incomplete VoidSW, and that something was SWP - Graf doesn't have Redux. This behavioral change most likely originates from the things that got borrowed from it at the time.

Why do you think VoidSW and SWP in particular were used as reference for cross-checking and filing bug reports, in addition to vanilla and me comparing it a lot with Redux ? That's not a coincidence.

[mjr4077au]

I don't think Raze has ever included SWP code, though? I wonder if it's just some kind of hack job that's been exposed by the better backend code?

No it did, and this was there from basically day-1.

It's story time: Back when Raze originally started development VoidSW was in a very rough state and not ready for any public release - it wasn't even in a compilable state until December if I remember correctly -, so Graf had to use something else as a reference in addition to the-then largely incomplete VoidSW, and that something was SWP - Graf doesn't have Redux. This behavioral change most likely originates from the things that got borrowed from it at the time.

Why do you think VoidSW and SWP in particular were used as reference for cross-checking and filing bug reports, in addition to vanilla and me comparing it a lot with Redux ? That's not a coincidence.

I'm happy to be wrong, but I can't see any evidence of SWP in Raze's commit history against the /source/sw folder. The first commit by Graf is a random change when bringing in some Blood stuff in Sept, then upstream commits from April. This indicates the sw folder is and always has been the folder in EDuke32's repository.

Capture.PNG

[sinisterseed]

No, you're wrong buddy, that code was probably not mentioned in any commit, but it is there, I just digged the source code up and compared it with SWP and VoidSW.

This is Raze's coolie.cpp death sequence:

Code: Select all

//////////////////////
//
// COOLIE DIE
//
//////////////////////

#define COOLIE_DIE_RATE 30
ANIMATOR DoCoolieWaitBirth;

STATE s_CoolieDie[] =
{
    {COOLIE_DIE + 0, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[1]},

    {COOLIE_DIE + 0, 0|SF_QUICK_CALL, SpawnCoolieExp, &s_CoolieDie[2]},

    {COOLIE_DIE + 1, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[3]},
    {COOLIE_DIE + 2, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[4]},
    {COOLIE_DIE + 3, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[5]},
    {COOLIE_DIE + 4, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[6]},
    {COOLIE_DIE + 5, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[7]},
    {COOLIE_DIE + 6, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[8]},
    {COOLIE_DIE + 7, COOLIE_DIE_RATE, DoCoolieWaitBirth, &s_CoolieDie[8]},

    {COOLIE_DIE + 7, COOLIE_DIE_RATE*5, DoActorDebris, &s_CoolieDie[10]},
    {COOLIE_DIE + 7, 0|SF_QUICK_CALL, SpawnCoolg, &s_CoolieDie[11]},
    {COOLIE_DEAD_NOHEAD, SF_QUICK_CALL, QueueFloorBlood, &s_CoolieDie[12]},
    {COOLIE_DEAD_NOHEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDie[12]}
};

STATEp sg_CoolieDie[] =
{
    s_CoolieDie
};

STATE s_CoolieDead[] =
{
    {COOLIE_DEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDead[0]},
};

STATEp sg_CoolieDead[] =
{
    s_CoolieDead
};

STATE s_CoolieDeadHead[] =
    {
    {COOLIE_DEAD_HEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDeadHead[0]}
    };

This is VoidSW's coolie.cpp death sequence:

Code: Select all

//////////////////////
//
// COOLIE DIE
//
//////////////////////

#define COOLIE_DIE_RATE 30
ANIMATOR DoCoolieWaitBirth;

STATE s_CoolieDie[] =
{
    {COOLIE_DIE + 0, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[1]},

    {COOLIE_DIE + 0, 0|SF_QUICK_CALL, SpawnCoolieExp, &s_CoolieDie[2]},

    {COOLIE_DIE + 1, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[3]},
    {COOLIE_DIE + 2, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[4]},
    {COOLIE_DIE + 3, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[5]},
    {COOLIE_DIE + 4, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[6]},
    {COOLIE_DIE + 5, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[7]},
    {COOLIE_DIE + 6, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[8]},
    {COOLIE_DIE + 7, COOLIE_DIE_RATE, DoCoolieWaitBirth, &s_CoolieDie[8]},

    {COOLIE_DIE + 7, COOLIE_DIE_RATE*5, DoActorDebris, &s_CoolieDie[10]},
    {COOLIE_DIE + 7, 0|SF_QUICK_CALL, SpawnCoolg, &s_CoolieDie[11]},
    {COOLIE_DEAD_NOHEAD, SF_QUICK_CALL, QueueFloorBlood, &s_CoolieDie[12]},
    {COOLIE_DEAD_NOHEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDie[12]}
};

STATEp sg_CoolieDie[] =
{
    s_CoolieDie
};

STATE s_CoolieDead[] =
{
    {COOLIE_DEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDead[0]},
};

STATEp sg_CoolieDead[] =
{
    s_CoolieDead
};

And this is SWP's coolie.c death sequence:

Code: Select all

//////////////////////
//
// COOLIE DIE
//
//////////////////////

#define COOLIE_DIE_RATE 30
ANIMATOR DoCoolieWaitBirth;

STATE s_CoolieDie[] =
    {
    {COOLIE_DIE + 0, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[1]},

    {COOLIE_DIE + 0, 0|SF_QUICK_CALL, SpawnCoolieExp, &s_CoolieDie[2]},

    {COOLIE_DIE + 1, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[3]},
    {COOLIE_DIE + 2, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[4]},
    {COOLIE_DIE + 3, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[5]},
    {COOLIE_DIE + 4, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[6]},
    {COOLIE_DIE + 5, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[7]},
    {COOLIE_DIE + 6, COOLIE_DIE_RATE, NullCoolie, &s_CoolieDie[8]},
    {COOLIE_DIE + 7, COOLIE_DIE_RATE, DoCoolieWaitBirth, &s_CoolieDie[8]},

    {COOLIE_DIE + 7, COOLIE_DIE_RATE*5, DoActorDebris, &s_CoolieDie[10]},
    {COOLIE_DIE + 7, 0|SF_QUICK_CALL, SpawnCoolg, &s_CoolieDie[11]},
    {COOLIE_DEAD_NOHEAD, SF_QUICK_CALL  , QueueFloorBlood, &s_CoolieDie[12]},
    {COOLIE_DEAD_NOHEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDie[12]}
    };

STATEp sg_CoolieDie[] =
    {
    s_CoolieDie
    };

STATE s_CoolieDead[] =
    {
    {COOLIE_DEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDead[0]},
    };

STATEp sg_CoolieDead[] =
    {
    s_CoolieDead
    };

STATE s_CoolieDeadHead[] =
    {
    {COOLIE_DEAD_HEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDeadHead[0]}
    };

So as you can see, Raze inherited that extra CoolieDeadHead enhancement from SWP at some point, but Graf probably completely missed its mention somehow - because it is even mentioned in SWP's coolie.c file at the beginning:

Code: Select all

Original Source: 1997 - Frank Maddin and Jim Norwood
Prepared for public release: 03/28/2005 - Charlie Wiederhold, 3D Realms
*/
//-------------------------------------------------------------------------
// Added coolie die with head if no ghost spawn
//

So, now we know what needs to be purged, and also exactly where, it should be just this extra state inside coolie.cpp:

Code: Select all

STATE s_CoolieDeadHead[] =
    {
    {COOLIE_DEAD_HEAD, COOLIE_DIE_RATE, DoActorDebris, &s_CoolieDeadHead[0]}
    };

Everything else should be the same, but please do double-check to be sure I'm not missing something else, to avoid breaking something by accident .

[mjr4077au]

So checking out git blame, it's always been VoidSW but Graf specifically merged 'fixes' from SWP: https://github.com/coelckers/Raze/commi ... 79d5c28354 .

I've built a binary with your proposed fix here. The proposed changes are in this branch. If it fixes the problem, I'll push the branch as a PR.

[sinisterseed]

So checking out git blame, it's always been VoidSW but Graf specifically merged 'fixes' from SWP: https://github.com/coelckers/Raze/commi ... 79d5c28354 .

Yup, that's right, now you got it buddy.

I've built a binary with your proposed fix here. The proposed changes are in this branch. If it fixes the problem, I'll push the branch as a PR.

Oh nice, that's a full package of SWP stuff I see. No bueno, I see there are other changes to enemies such as Ninjas, a supposed "fix", but I don't know how that is supposed to impact the gameplay. I can't tell, so... any ideas?

As for your fix, I tested it out on different maps with many Coolies and it seems to have done it, the original behavior has been restored, so I am confident in this fixing the immediate issue. However, I'd like you to compare the rest of the Coolie and Coolie Ghost sources from SWP and VoidSW for code differences as well (or maybe more? Does this spread elsewhere?), the reason for this is twofold but actually just one in reality. To a) make sure there's no other behavioral differences between these 2 enemies anymore (as in, vanilla vs Raze vs VoidSW vs Redux), and b) make sure the removal didn't cause other issues which could lead to undefined behavior and similar problems.

I already went through them myself once, but I am quite cautious with changes like this and I would definitely appreciate a second pair of eyes . "Perfectly cautious, as everyone should be."

[mjr4077au]

My simple opinion on this would be that VoidSW is right and SWP is wrong. VoidSW have strived hard to keep the game demo compatible so there should be an element of accuracy there.

I don't know SW as well as some so I can't say much about any potential accuracy issues to other enemy types.

This is your fix and you're saying it's fixed the problem, I reckon it's time to break your cherry and submit your first PR off for review

[sinisterseed]

I agree, I want Raze to offer a faithful experience but with a lot of new additions and improvements too, however changes like this are something I'm definitely not too fond of since it was a change that well, changed things up for the sake of it. I think SWP simply did it because it could.

If this is all that was borrowed from SWP in terms of behavioral changes it's perfect, judging from that big commit, and you have properly fixed it since it reverts exactly what's necessary, I think I was just overly cautious if not actually paranoid. What concerns me now is what is that Ninja fix, if I read that code right it also adds a "hara kiri" feature for the Ninjas that should be locked behind a cvar, so it can be enabled or disabled on a whim. But I think this setting is actually missing its own menu entry though, I've never seen an option for this but there is one in SWP.

It also added a rotation fix of sorts for their sprites, which is something that I think is worth preserving since it's probably an actual fix this time around and not a gratuitious change. The "swgimme" cheat was also changed, apparently it gives 2 Uzis now, whereas, I guess, the original gave only one for some reason. Meh, that being a cheat I'm personally fine with keeping it that way - and considering the change, I really don't think anyone seriously minds this change.

As for making a PR, well I totally would do it, but there's a small... problem. I don't know how, nor do I know what else I need to do it. I definitely want to learn though, so if you have some spare time on your hands I'm all ears .

[Graf Zahl]

Another notable change in SWP was an alternative death for the ninja, but that one's optional. This one should be, too.

[sinisterseed]

Hm, so would prefer keeping the alternative Coolie behavior as a toggle instead boss?

I fine with that too if you want.

[mjr4077au]

@lowskill, wondering if you wanted to take point on this as a skill up? You did well to find the code issues with the Coolie. If the ninja death is behind a CVAR, you should be able to see how that's been implemented fairly easily.

I imagine the CVAR is a bool that might be used in one or two if statements in the ninja file. If the CVAR is exposed into the menu, that should be a one liner in the menudef code. Happy to take a look if you'd prefer though

[sinisterseed]

@lowskill, wondering if you wanted to take point on this as a skill up? You did well to find the code issues with the Coolie. If the ninja death is behind a CVAR, you should be able to see how that's been implemented fairly easily.

I imagine the CVAR is a bool that might be used in one or two if statements in the ninja file. If the CVAR is exposed into the menu, that should be a one liner in the menudef code. Happy to take a look if you'd prefer though

Frankly I'd still like to do it myself because I think I have a vague idea about how it should be done, it needs a menu entry to tie it to and a CVAR bool, but I'll have to see how the Ninja alternate death is done to get a better idea.

But the underlying issue still remains - I still don't know how to make a PR and set up Raze locally before I can attempt that. I tried to set up a Doom port once but I failed at that and I imagine I'll do the same here since the procedure would be roughly the same. I first forked it, then tried to clone it, but got errors about missing some dependencies, panicked, and canned the whole thing. Totally unrelated to a topic I recently opened in a different section.

So unless someone helps me out here, the PR with the fix for the Coolies ain't coming from me anytime soon, and I'll be a roadblock .

[mjr4077au]

I'm happy to help with this as a personal upskill for you. When you've been testing builds of Raze currently, how have you been doing so? Just using Continuous Integration builds?

To keep the topic on point, I'll do a build guide and see if I can get it into the Wiki. I've been meaning to do some Raze documentation for some time and I have an account to do so thanks to Rachael

@Graf, for languages.csv, is there a Google Docs link for the file similar to GZDoom? I'm aware that PRs directly manipulating the CSV file get pushed back so would want this to be done right the first time around.

[Graf Zahl]

It is private for now because it contains things that cannot be made public, like all of Ion Fury's text. Just post what is needed and I'll add it.