9 posts / 0 new
Last post
0siribix
0siribix's picture
Contributor
Offline
Last seen: 3 years 3 months ago
Joined: 01/23/2016 - 16:20
Karma: 28
Problem scripts in dev release 0.9.7.1

I looked through many of the script source files included in the latest dev release. Most of the problems I found were non-quest scripts. I didn't look at every quest script because most of them were the same and few had any issues.

Of what I've looked at, fbmwinterventionscript has the most room for improvement. I would almost want to rewrite it from scratch.

qf_mq101_0003372b, if I'm not mistaken, was a vanilla script and still contains alot of original code which, I assume, needs to stay that way. Anyway, it needs Game.GetPlayer() replaced with a playerREF property and some convenience functions expanded to full versions.

fbmwplayerwerewolfquestscript needs Game.GetPlayer() replaced with playerREF and some repetitive code replaced with loops + arrays of properties.

At a quick glance, it looks like fbmwmtwritsmanagement could be shortened to about 1/3 - 1/4 of it's current size by replacing redundant code with loops + arrays of properties. First need to know why so many if statements are commented out?

fbmwplayervampirequestscript could also use some loops + arrays of properties. IMO, it needs a check in the VampireChange() function that Actor == playerREF As Actor, but there could be a check before this function is called that I don't know about.

If VampClan == "0ClanAundae"
       Target.SetFactionRank(ClanAundae, 1)
       Target.AddSpell(AbVampireAundaeSpecials, abVerbose = False)
       VampClanGlobal.mod(1)
       Target.RemoveFromFaction(ClanBerne)
       Target.RemoveFromFaction(ClanQuarra)
       If VampClan == "0ClanBerne"
           Target.SetFactionRank(ClanBerne, 1)
           Target.AddSpell(AbVampireBerneSpecials, abVerbose = False)
           VampClanGlobal.mod(2)
           Target.RemoveFromFaction(ClanAundae)
           Target.RemoveFromFaction(ClanQuarra)
           If VampClan == "0ClanQuarra"
               Target.SetFactionRank(ClanQuarra, 1)
               Target.AddSpell(AbVampireQuarraSpecials, abVerbose = False)
               VampClanGlobal.mod(3)
               Target.RemoveFromFaction(ClanAundae)
               Target.RemoveFromFaction(ClanBerne)
           Else
               Debug.messagebox("Something is wrong, no clan defined.")
           Endif
       Endif
   Endif

This looks like bad logic. The "0ClanBerne" and "0ClanQuarra" sections are unreachable.

morrowindbardssongsscript looks pretty good, but we could put the song scenes in a properties array and replace the PlayChosenSong() function with a single line of code.  Also, with that array, the StopAllSongs() function couild use a loop.

In fbmw_qf_fbmwttsevengraces_0100f86f, we need to remove all those '== TRUE'. 'If xxx == TRUE' is logically equivalent to 'If xxx' but when you compile it it actually puts that extra comparison in there (IIRC). So 'If xxx == TRUE' compiles to '(If xxx == TRUE) == TRUE'.

fbmwbirthsigns could be converted to use several arrays of properties and probably 1/4 of its current size but if this script only runs once then that priority should be very low.

fbmwshrinedaring and fbmwshrinepride need to be rewritten. It looks to me like all the code is redundant and can be minimized easily.

fbmwtrapscript is very well written (looks like something I wrote :-p ). Just need to replace Game.GetPlayer() with playerREF

skywindutils already has playerREF property but isn't used through most of the script. A note about 'return Game.getPlayer().getDistance(target)': IIRC, I dealt with some issue with this function where if the target isn't persistent and isn't loaded, it would return a value without complaining but it would be invalid. It's been a while so I could be wrong.

fbmwtowershieldeventcallerscript has an awful lot going on in the OnHit event. It looks like it could easily call that event multiple times before it has had time to finish. I honestly don't know how that situation plays out.

fbmw_qf_fbmwbladestrainers_0100f87b, again, has a bunch of '== TRUE' that can safely be removed

If we put all those messages in fbmwvavampdreamssleepscript into a string array, you could replace the entire if/then/else block with a single statement

fbmw_arkngthandexteriorcrank needs to be rewritten. Yeah...

fbmw_qf_fbmwdamephala_0100f880 uses convenience functino 'GetRef()', easy to expand to full function but should be low priority

fbmwoffensivedefensespellscript needs to be converted to use 'RegisterForSingleUpdate'. That's a big no-no that has caused lots of people lots of headaches.

fbmw_qf_fbmwicadvancement_0100f9cc needs Game.GetPlayer() replaced with playerREF

fbmw_qf_fbmwfgadvancement_0100f9ce needs Game.GetPlayer() replaced with playerREF

fbmw_qf_fbmwbmstones_0100f9ef needs '== TRUE' removed

fbmwdebugdiseasestone needs to be rewritten

fbmw_qf_fbmwtgadvancement_0100f9ca needs Game.GetPlayer() replaced with playerREF

fbmwenvemberstrigger should be rewritten. The OnActivate Event has redundant logic and the 'LetThereBeLight()' and 'LightsOut()' functions can be combined to a single toggle function

fbmwdefaultstaffscript and fbmwstaffwardscript: I don't see how these scripts could work. Pretty sure they need to be rewritten.

fbmwmagicaugmentweap probably works as-is, but could definitely be optimized

fbmwtellighttrigger could use a few optimizations but otherwise well done :)

divineinterventionscript needs to be rewritten

Taerkalith
Taerkalith's picture
Moderator
Offline
Last seen: 7 months 1 week ago
Joined: 04/10/2014 - 04:58
Karma: 1294
I see no issue with changing

I see no issue with changing Game.GetPlayer() over to the faster playerREF, so long as you don't miss assigning the property on any of them of course.

For qf_mq101_0003372b:

this is a modified vanilla script for game start-up. I'd tend to leave it be since it only fires once and I've noticed no speed issues with it.

For bards: I've got some additional tracks if you feel like doing some work on this one and want to expand/change the in-game functionality a bit. The choices don't quite line up nicely with the audio files being played right now.

The rest also need touch up and some point. I'm the primary other person who might end up butting heads with you as I've been checking quest aliases that cause non-firing quests. I can hold off on working on these until the next build to give you a clean window if you'd like. I've got enough to do elsewhere as is ;)

maxnosense
maxnosense's picture
Contributor
Offline
Last seen: 2 years 9 months ago
Joined: 02/07/2015 - 08:45
Karma: 167
Just as a general note here:

Just as a general note here:

0siribix, please contact ma about organizing this whole testing and debugging business.

We'll need to track this on trello to keep track of which scripts have been cleaned up and which haven't been. Doing this like this on the forums doesn't seem to be very promising in the long run. Good work for now, though.

0siribix
0siribix's picture
Contributor
Offline
Last seen: 3 years 3 months ago
Joined: 01/23/2016 - 16:20
Karma: 28
Understood. I mentioned in

Understood. I mentioned in chat that I had made some notes and grumpy told me to go ahead and make a post about them. I'll start looking at trello now.

grumpycat
grumpycat's picture
Moderator
Offline
Last seen: 2 months 2 weeks ago
Joined: 12/08/2012 - 06:41
Karma: 3383
Maybe as we cleanup the

Maybe as we cleanup the scripts we can rename them so that its easy to tell which ones have been changed?

At the moment they are prefixed with "fbmw", maybe we can make a new prefix of "mw_"

0siribix
0siribix's picture
Contributor
Offline
Last seen: 3 years 3 months ago
Joined: 01/23/2016 - 16:20
Karma: 28
I've got it all on trello now

I've got it all on trello now. Anway if you rename a script, you have to reattach it to every instance in the CK.

Taerkalith
Taerkalith's picture
Moderator
Offline
Last seen: 7 months 1 week ago
Joined: 04/10/2014 - 04:58
Karma: 1294
Yeah, renaming is not the way

Yeah, renaming is not the way to go. Keep track in a spreadsheet the ones that have been modified out of a total known problem list would be my suggestion.

grumpycat
grumpycat's picture
Moderator
Offline
Last seen: 2 months 2 weeks ago
Joined: 12/08/2012 - 06:41
Karma: 3383
OK, lets just make sure we

OK, lets just make sure we have comments in the top section of the scripts with authors names and version so that it can be seen if overhauled

maxnosense
maxnosense's picture
Contributor
Offline
Last seen: 2 years 9 months ago
Joined: 02/07/2015 - 08:45
Karma: 167
Yes, of course, we will take

Yes, of course, we will take care to inlcude proper documentation in the future, I plan on using gitlab for this where we can also attach a bug tracker easily.