Re-coding the Sigil

[Ed the Bat]

Anyone who's tried working with or around the Sigil of the One God in a project (Xaser, perhaps you share some of my grief here? ) likely knows what a complete mess of code it really is under the hood. It uses several restricted codepointers that can be completely supplanted by simpler and cleaner methods, and its native coding makes it that a new weapon can't even be made by inheriting from it -- child classes become some kind of an extension of the original.

But I'll get to the point. I think it can be improved greatly, thanks to more modern coding. I'm no genius, but I'd be more than happy to put my weight behind an effort to fix it up. The more I look at it, the more I'm convinced that the only reason it wasn't cleaned up sooner is because no one really cared enough. And I suppose I understand that, since I've never seen any other project that needed it, until mine (and Xaser's, but I didn't know about Psychic until my own project was well into its development).

Now, I turn to the community for input. I know I'm not a genius, and for the most part, raw source code goes well over my head. So I ask, does anyone feel I've missed any vital information on this issue? Am I completely misguided on how I'm thinking here? If my thoughts here are incorrect or incomplete, I should find that out sooner rather than later. On the other hand, if anyone supports this idea, I suppose I should know that too.

As a miscellaneous tidbit, here's some code I churned out during my investigations a few days ago, when trying to create a simplified-yet-identical weapon out of the Sigil. As I mentioned above, I found it impossible, as inheritance doesn't work the same at all with this weapon... I particularly think my Deselect state rewrite highlights just how needlessly complex/outdated the original Sigil's code is.

Code: Select all

ACTOR MGStrifeSigil : Sigil
{
	Tag "SIGIL"
	+FLOORCLIP
	+WEAPON.CHEATNOTWEAPON
	Weapon.SelectionOrder 4000
	Weapon.Kickback 100
	Health 1
	Inventory.PickupSound "weapons/sigilcharge"
	Inventory.PickupMessage "You picked up the SIGIL."
	Inventory.Icon "I_SGL1"
	States
	{
	Ready:
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",5,"Ready5")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",4,"Ready4")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",3,"Ready3")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",2,"Ready2")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",1,"Ready1")
		Wait
	Ready1:
		SIGH A 1 Bright A_WeaponReady
		Wait
	Ready2:
		SIGH B 1 Bright A_WeaponReady
		Wait
	Ready3:
		SIGH C 1 Bright A_WeaponReady
		Wait
	Ready4:
		SIGH D 1 Bright A_WeaponReady
		Wait
	Ready5:
		SIGH E 1 Bright A_WeaponReady
		Wait
	Deselect:
		SIGH "#" 1 Bright A_Lower
		Wait
	Select:
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",5,"Select5")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",4,"Select4")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",3,"Select3")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",2,"Select2")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",1,"Select1")
		Wait
	Select1:
		SIGH A 1 Bright A_Raise
		Wait
	Select2:
		SIGH B 1 Bright A_Raise
		Wait
	Select3:
		SIGH C 1 Bright A_Raise
		Wait
	Select4:
		SIGH D 1 Bright A_Raise
		Wait
	Select5:
		SIGH E 1 Bright A_Raise
		Wait
	Fire:
		SIGH "#" 0 A_PlaySound("weapons/sigilcharge",CHAN_WEAPON)
		SIGH "#" 18 Bright A_Light2
		SIGH "#" 3 Bright A_GunFlash
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",5,"Fire5")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",4,"Fire4")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",3,"Fire3")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",2,"Fire2")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",1,"Fire1")
	Fire1:
		SIGH A 15 A_FireSigil1
		Goto Ready
	Fire2:
		SIGH B 15 A_FireSigil2
		Goto Ready
	Fire3:
		SIGH C 15 A_FireSigil3
		Goto Ready
	Fire4:
		SIGH D 15 A_FireSigil4
		Goto Ready
	Fire5:
		SIGH E 15 A_FireSigil5
		Goto Ready
	Flash:
		SIGF A 4 BRIGHT A_Light2
		SIGF B 6 BRIGHT A_LightInverse
		SIGF C 4 BRIGHT A_Light1
		Goto LightDone
	}
}

[Gez]

Okay, there is one glaring problem here:

Code: Select all

	Select:
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",5,"Select5")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",4,"Select4")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",3,"Select3")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",2,"Select2")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",1,"Select1")
		Wait
	Select1:

Let's suppose that somehow there isn't any MGSigilPiece in inventory (maybe the player toyed with console commands). What happens when it's selected? All the jumps fail, so it goes to the "Wait" part which tells it to loop on the current state (that is to say: TNT1 A 0 A_JumpIfInventory("MGSigilPiece",1,"Select1") in this case), which will fail again, and you get an infinite 0-tic loop. Also known as a freeze.

For the sake of failsafing, you should do it like this:

Code: Select all

	Select:
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",5,"Select5")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",4,"Select4")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",3,"Select3")
		TNT1 A 0 A_JumpIfInventory("MGSigilPiece",2,"Select2")
	Select1:

That way, whether the player has 1 or 0 MGSigilPieces, the weapon code will go to the Select1 state (state labels do not interrupt a state sequence).

[Ed the Bat]

Good point. I forgot people might screw around with things they shouldn't. You're right, that automatic fallthrough you put should cover that. For that matter, I should do similar with the Ready state. Looks like the Fire state should already be safe, since there's no Wait or Loop before something with an actual duration happens, right?

The potential loop in Ready was an oversight on my part, from transferring over the original Sigil's code.

Code: Select all

	Ready:
		SIGH A 0 Bright A_SelectSigilView
		Wait
		SIGH A 1 Bright A_WeaponReady
		Wait
		SIGH B 1 Bright A_WeaponReady
		Wait
		SIGH C 1 Bright A_WeaponReady
		Wait
		SIGH D 1 Bright A_WeaponReady
		Wait
		SIGH E 1 Bright A_WeaponReady
		Wait

A_SelectSigilView checks how many pieces the player has, much like what I did up above. If A_SelectSigilView were to ever return a result of 'zero pieces', then it looks like this would cause an infinite loop freeze. So, thank goodness it can't return zero, right?

[Xaser]

The Sigil is one thing I haven't yet touched in my WIP strife enhancement-ish mod, precisely because of the complexity involved, though I did go ahead and do it with Psychic for reasons I can't remember. Good to see someone's beat me to it.

[Ed the Bat]

The Sigil is one thing I haven't yet touched in my WIP strife enhancement-ish mod, precisely because of the complexity involved, though I did go ahead and do it with Psychic for reasons I can't remember. Good to see someone's beat me to it.

I was actually reviewing the Sigil mockup you created for Psychic, and I think you're onto something there. In fact, don't be offended if I use your work as a base for finishing a DECORATE ground-up rebuild of a Sigil. If I can recreate the entire thing without having to inherit from the base class (which, as mentioned, doesn't quite work right...), I'll be able to smooth out a few things in my own project. Granted, I still think the Sigil could use some cleanup in the engine, even if I already got what I wanted.

[Xaser]

No worries at all about the code borrowing -- have at it! It'd be great to see this be used as a resource to make Strife modders' lives easier. Though on that note, maybe I should recruit you for my own project since that's sorta what it's aspiring to do...

Also, you're quick at replying to things. Either you lurk more quickly than I or actually make use of the "subscribe" feature.

[Ed the Bat]

I would be honored to be involved in something! I wouldn't go as far as to say my project is... good, per se, but I've put a lot of work into it, learned a lot while doing so, and found a surprising number of opportunities to put something I've done into practice for something else.

As for my quickness, I do subscribe to things. Real-time email alerts! But even if I didn't, I would probably be coming back to check rather frequently, as I often get bored and hope for any kind of new content to interest me.

[Ed the Bat]

I'm in the process of fixing my pseudo-Sigil. I've added the fixes Gez mentioned, and with a little help from Xaser's Psychic as a reference, I've muddled my way through the source code enough to translate a couple of the firing mechanisms to DECORATE (with some ACS for the self-damaging aspect). Right now I'm looking at rewriting A_FireSigil3 using generic code, and I want to verify I'm understanding something correctly...

Code: Select all

//============================================================================
//
// A_FireSigil3
//
//============================================================================

DEFINE_ACTION_FUNCTION(AActor, A_FireSigil3)
{
	AActor *spot;
	player_t *player = self->player;
	int i;

	if (player == NULL || player->ReadyWeapon == NULL)
		return;

	P_DamageMobj (self, self, NULL, 3*4, 0, DMG_NO_ARMOR);
	S_Sound (self, CHAN_WEAPON, "weapons/sigilcharge", 1, ATTN_NORM);

	self->angle -= ANGLE_90;
	for (i = 0; i < 20; ++i)
	{
		self->angle += ANGLE_180/20;
		spot = P_SpawnSubMissile (self, PClass::FindClass("SpectralLightningBall1"), self);
		if (spot != NULL)
		{
			spot->z = self->z + 32*FRACUNIT;
		}
	}
	self->angle -= (ANGLE_180/20)*10;
}

It looks to me like the angle is set to -90, but then incremented by 9 immediately, before anything is shot. So, would that equate to something like this?

Code: Select all

	Fire3:
		SIGH "#" 0 A_PlaySound("weapons/sigilcharge",CHAN_WEAPON)
		SIGH "#" 0 ACS_NamedExecuteAlways("MGStrifeSigilDamage",0,12,0,0)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-81,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-72,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-63,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-54,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-45,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-36,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-27,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-18,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",-9,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",0,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",9,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",18,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",27,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",36,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",45,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",54,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",63,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",72,0,0,32)
		SIGH "#" 0 A_FireCustomMissile("SpectralLightningBall1",81,0,0,32)
		SIGH "#" 15 A_FireCustomMissile("SpectralLightningBall1",90,0,0,32)
		//SIGH "#" 15 A_FireSigil3
		Goto Ready

It starts at -81, ends at 90, and fires a total of 20, like the code would seem to indicate. It just seemed a little odd to me, starting at -81 (thus making the arc not perfectly symmetrical), so I wanted to double-check that I didn't mess something up.

[Ed the Bat]

Moving on from that last one, I've been looking at the 1st and 4th firing modes for the Sigil, and these are a bit more complex.

Code: Select all

//============================================================================
//
// A_FireSigil1
//
//============================================================================

DEFINE_ACTION_FUNCTION(AActor, A_FireSigil1)
{
	AActor *spot;
	player_t *player = self->player;
	AActor *linetarget;

	if (player == NULL || player->ReadyWeapon == NULL)
		return;

	P_DamageMobj (self, self, NULL, 1*4, 0, DMG_NO_ARMOR);
	S_Sound (self, CHAN_WEAPON, "weapons/sigilcharge", 1, ATTN_NORM);

	P_BulletSlope (self, &linetarget);
	if (linetarget != NULL)
	{
		spot = Spawn("SpectralLightningSpot", linetarget->x, linetarget->y, linetarget->floorz, ALLOW_REPLACE);
		if (spot != NULL)
		{
			spot->tracer = linetarget;
		}
	}
	else
	{
		spot = Spawn("SpectralLightningSpot", self->x, self->y, self->z, ALLOW_REPLACE);
		if (spot != NULL)
		{
			spot->velx += 28 * finecosine[self->angle >> ANGLETOFINESHIFT];
			spot->vely += 28 * finesine[self->angle >> ANGLETOFINESHIFT];
		}
	}
	if (spot != NULL)
	{
		spot->SetFriendPlayer(player);
		spot->target = self;
	}
}

//============================================================================
//
// A_FireSigil4
//
//============================================================================

DEFINE_ACTION_FUNCTION(AActor, A_FireSigil4)
{
	AActor *spot;
	player_t *player = self->player;
	AActor *linetarget;

	if (player == NULL || player->ReadyWeapon == NULL)
		return;

	P_DamageMobj (self, self, NULL, 4*4, 0, DMG_NO_ARMOR);
	S_Sound (self, CHAN_WEAPON, "weapons/sigilcharge", 1, ATTN_NORM);

	P_BulletSlope (self, &linetarget);
	if (linetarget != NULL)
	{
		spot = P_SpawnPlayerMissile (self, 0,0,0, PClass::FindClass("SpectralLightningBigV1"), self->angle, &linetarget);
		if (spot != NULL)
		{
			spot->tracer = linetarget;
		}
	}
	else
	{
		spot = P_SpawnPlayerMissile (self, PClass::FindClass("SpectralLightningBigV1"));
		if (spot != NULL)
		{
			spot->velx += FixedMul (spot->Speed, finecosine[self->angle >> ANGLETOFINESHIFT]);
			spot->vely += FixedMul (spot->Speed, finesine[self->angle >> ANGLETOFINESHIFT]);
		}
	}
}

Thankfully, it looks like the two share similar structures, so cracking one would nearly get me everything I need to know for both of them. But, once again, the internal functions of the source code are a great deal over my head. As I stare at this, though, something in the back of my mind says A_JumpIfTargetInLOS could be useful, but my gut says there's still more pieces to the puzzle. You folks have any advice or ideas?

[Fishytza]

Hi guys! I've managed to make... something, if you're interested.

[Ed the Bat]

Solid stuff, my Fishy friend! Would you be offended if I borrowed some of your work to patch up what I've got missing from mine?

Also, in reference to the one you have, I have an idea for a quicker hack for picking up the Sigil in Strife. If you like it, have at it. Or if there's something missing from it, stay with the ones you came up with.

Code: Select all

if(CheckWeapon("Sigil"))			//If the Sigil tries to appear in your hands...
{
	TakeInventory("Sigil",1);		//Pull it out of your inventory...
	GiveInventory("NNSigilPiece",1);	//Then get another non-native piece in its place...
	SetWeapon("NNSigil");		//And put the non-native Sigil in your hands
}

[Fishytza]

Would you be offended if I borrowed some of your work to patch up what I've got missing from mine?

Not at all! Take whatever you need.

[Ed the Bat]

Thank you very much! This was a huge jump forward for me.

And, just as a curiosity, have you happened to look at the other weapons in Strife? Working on the Sigil got me interested in some of the more nuanced facets of the other guns. In fact, if not for the sprite-shifting of A_FireMauler2Pre, I found that I could entirely recode that weapon without restricted code pointers.

[Fishytza]

Code: Select all

if(CheckWeapon("Sigil"))         //If the Sigil tries to appear in your hands...
{
   TakeInventory("Sigil",1);      //Pull it out of your inventory...
   GiveInventory("NNSigilPiece",1);   //Then get another non-native piece in its place...
   SetWeapon("NNSigil");      //And put the non-native Sigil in your hands
}

Amazing that 'CheckWeapon' returns true *just before* the actual weapon is selected, which makes this useful in a another weapon's 'Deselect' state.
There's just one problem. If the player has "switch on pickup" turned off how exactly is the NNSigil supposed to update, then?

And, just as a curiosity, have you happened to look at the other weapons in Strife?

Not yet. Two reasons why I did the Sigil was to see if I could, and secondly to make people's lives a little easier and rid them of the can't-mod-the-Sigil-cuz-it's-too-hardcoded excuse.

[Ed the Bat]

If the player has "switch on pickup" turned off...

Aha, good point. I hadn't thought of that. I guess I'll keep working, then.
EDIT: Maybe something to this tune?

Code: Select all

if(GetCVar(neverswitchonpickup)==true&&CheckInventory("Sigil"))//If autoswitch is off...
	SetWeapon("Sigil");					//Force the player to try to select it if he has it
//The rest is the same as before
if(CheckWeapon("Sigil"))				//If the Sigil tries to appear in your hands...
{
	TakeInventory("Sigil",1);			//Pull it out of your inventory...
	GiveInventory("NNSigilPiece",1);	//Then get another non-native piece in its place...
	SetWeapon("NNSigil");				//And put the non-native Sigil in your hands
}