SSE Code Review + Porting a Skyrim classic mod

[Drakmyth]

Many moons ago I wrote a simple little mod for Skyrim: https://www.nexusmods.com/skyrim/mods/36073

 

I recently got interested in playing again and was hoping to update it to support SSE. While the mod works (or rather worked, I haven't ported it yet) I only sort of understood what I was doing when I wrote it and as such I only sort of understand it now. I would like to ask, if anyone would be willing to take the time, if anyone could look over how I implemented the mod and offer any suggestions for improvement, particularly where the scripts are concerned. I'm a programmer by trade, but am not completely familiar with how Papyrus interacts with Skyrim, and while I've gleaned some information regarding such from the Creation Kit wiki, I'd like to get another pair of eyes on it in case I've interpreted something wrong.

 

 

What follows are the implementation details of the mod:

 

This mod adds a spell that can unlock chests and doors. Given this was written 5 years ago over the course of a couple days, and was my only Skyrim mod, I don't entirely remember all the details, but from looking at the unpacked scripts here's is my best interpretation of how I did this.

 

The player learns the spell via a spell tome. The book is created like any other spell tome and is not particularly notable. Rather than placing it somewhere in the world, I decided to just add it to the levelled spell tome lists so it would appear as random loot and/or in a shop once the player has the appropriate Alteration level. In order to avoid conflicting with other mods by modifying the leveled list record directly, I created a quest 'drakKnockInitQuest' that runs once the first time the game loads with the mod installed. This quest starts, adds the tome to the levelled lists, then stops and never runs again.

 

The spell itself works by casting a hitscan projectile which, upon collision with an object or terrain, creates an invisible explosion. That explosion spawns an object I referred to as a "trigger". I execute a script on the trigger to start a quest 'drakKnockQuest'. This quest functions purely as a one time reference to other data. Specifically it stores a reference to the trigger provided the player is within a certain distance of it (this effectively limits how far the spell can be cast from) and it stores a reference to the nearest locked object to the trigger within a certain distance (this accounts for how accurate the player must be when casting the spell). If either of these values are not set (the distance was too far away) then I delete the trigger object and do nothing. Otherwise the script executes the unlock logic using that data and stops the quest. Finally I delete the trigger object.

 

drakKnockInitQuestScript:

 

 

Scriptname drakKnockInitQuestScript extends Quest

Book Property KnockSpellTome Auto

LeveledItem[] Property tomeLists Auto

Event OnInit()

int index = tomeLists.Length

while index

index -= 1

LeveledItem list = tomeLists[index]

list.AddForm(KnockSpellTome, 1, 1)

endwhile

EndEvent

 

 

 

drakKnockQuestScript:

 

 

Scriptname drakKnockQuestScript extends Quest

ReferenceAlias Property KnockTriggerAlias Auto

ReferenceAlias Property LockedObjAlias Auto

 

 

 

drakKnockTriggerScript:

 

 

Scriptname drakKnockTriggerScript extends ObjectReference

Quest Property qstKnock Auto

Sound Property sndSuccess Auto

EffectShader Property efsGlow Auto

Message Property msgFailure Auto

Event OnInit()

drakKnockQuestScript kqs = qstKnock as drakKnockQuestScript

kqs.Start()

ObjectReference lockedObj = kqs.LockedObjAlias.getReference()

ObjectReference knockTrigger = kqs.KnockTriggerAlias.getReference()

if lockedObj != NONE && knockTrigger != NONE

int alterationSkillLvl = Game.GetPlayer().GetActorValue("Alteration") as int

if (alterationSkillLvl > 100)

alterationSkillLvl = 100

endif

if lockedObj.getLockLevel() <= alterationSkillLvl

lockedObj.Lock(false, false)

Game.DisablePlayerControls(false, false, false, false, false, false, true, false, 0)

Game.EnablePlayerControls(false, false, false, false, false, false, true, false, 0)

efsGlow.Play(lockedObj, 1.0)

sndSuccess.Play(Game.GetPlayer())

Game.AdvanceSkill("Alteration", 15.0)

else

msgFailure.Show()

endif

kqs.Stop()

endif

self.Delete()

EndEvent

 

 

 

 

While this works, I have no idea how efficient or clean this is. I don't know if I'm polluting the users save file with objects or gobbling memory, or causing some other form of instability. When I briefly forayed into modding Fallout 3 back in the day, I seem to recall reading that scripts should be avoided if at all possible because they left references around after the mod was uninstalled. I don't believe that's true with Skyrim, but I'm not sure. I read through the Papyrus best practices thread, but didn't see much that applied to my mod outside of a couple formatting practices.

 

Any pointers or recommendations on this are greatly appreciated.

[cdcooley]

That looks reasonable to me. You could accomplish the same thing without the quest to check distance to the player and locate the nearest container (because there are matching functions that can do that) but it's not going to run any faster so there's no point in changing things.