Halstrom Posted June 19, 2014 Author Posted June 19, 2014 Hmm basically if you don't understand how some code works screwing with it is bound to cause trouble, especially if it's been done by someone "ingenious" A lot of my early coding could be termed "ingenious" and it leaves me scratching my head on how it works Basically though that short snippet then means nothing to anyone then, you need to post the whole script and tell us which bit isn't working and why you came to the conclusion that that line is an issue.
ArgusSCCT Posted June 19, 2014 Posted June 19, 2014 Hmm basically if you don't understand how some code works screwing with it is bound to cause trouble, especially if it's been done by someone "ingenious" A lot of my early coding could be termed "ingenious" and it leaves me scratching my head on how it works Basically though that short snippet then means nothing to anyone then, you need to post the whole script and tell us which bit isn't working and why you came to the conclusion that that line is an issue. I know, reading code from other people and figuring out what it does, is really hard, I already had to do that with some earlier scripts from this mod. I got rid of those and made my own implementations, they were too messy to try to fix. I think I may have said something wrong, the entirety of this script does work, including that line. I just asked how to change that or make it shorter, because I'm still not completely sure on how to work with FNV's scripting language. The code does work, I'm just removing irrelevancies and fixing messy statements, like that conditional, in order to make it easier for me or anyone else, who might come to update that mod in the future. Thankfully, this script was small, there are some others that are huge, and contain those sorts of problems. For instance, this was one of the ways a variable was being declared: short variable If (variable -- 0) set variable to something EndIf Now I'm pretty sure that variable is null, so there's no point in doing that anyway. This mod has some big scripts that have those sorts of blunders, but on the whole, this particular script with that "If" was working fine, I just corrected a few things.
Odessa Posted June 19, 2014 Posted June 19, 2014 Are you sure the script doesn't run multiple times, hence only the first time it will be '== 0'. Or, if its a quest script or some object scripts, that var could be set externally. As a general point, saying 'I'm pretty sure this will be that so theres no point doing this' is a bad a ideology, you should include error handling in your code. Remember any script you write is part of the greater game and unpredicted things will often happen. (I will spontaneously orgasm if someone manages to add Python-like try...exceptions to NVSE5.) To make that long if line more readable, I would suggest using a UDF, so instead of : if cond1 && cond2 && cond3 && cond4 && cond5 ; whatever endif ; use if eval call MyReadablyNamedConditionUDF ;whatever endif ; ------ in a seperate object script scn MyReadablyNamedConditonUDF Begin Function { } if eval !(Cond1) SetFunctionValue 0 ; False elseif eval !(Cond2) SetFunctionValue 0 ; False elseif eval !(Cond3) SetFunctionValue 0 ; False elseif eval !(Cond4) SetFunctionValue 0 ; False elseif eval !(Cond5) SetFunctionValue 0 ; False else SetFunctionValue 1 ; True endif return End
ArgusSCCT Posted June 19, 2014 Posted June 19, 2014 I'll see what I can do with it. It's a pretty simple script. Anyway, if you want to check out how it looks (I've not fixed that troublesome line yet), here it is: scn PPAInstalledAPACapAdapterScript ref rContainer ref rArmor int iActive int iCount int iDoOnce int iSwapCounter begin GameMode Let iCount := 0 Let iCount := 0 Let rContainer := GetContainer If (PPADisableToggle.rTarget == rContainer) return EndIf Let rArmor := rContainer.getequippedobject 2 If (ListGetFormIndex PPAPowerReduced rArmor > -1 && listgetformindex PPAPowerNuclear rArmor > -1 && listgetformindex PPAAPA rArmor > -1 && rContainer.getequipped PPAAPAMFExp == 0 && rContainer.getdead == 0) Let iDoOnce := 0 ElseIf iDoOnce == 0 If iSwapCounter < 3 Let iSwapCounter := sSwapCounter + 1 Else Let iDoOnce := 1 rContainer.additem PPAMAPACapAdapter 1 1 rContainer.unequipitem PPAAPACapAdapter 0 1 removeme EndIf EndIf end It is a script used in Powered Power Armor to add one of the Power Armor mods' effects onto the player.
Odessa Posted June 19, 2014 Posted June 19, 2014 Looks broken. Some general comments: - Object script GameMode blocks run once per frame (so 30+ times per second) - In the GECK, all variables are initialized to 0. - When you do 'RemoveMe' it destroys the object and the script (NOTE: it may continue running for a few frames because the GECK can be ornery) - Object scripts are instance based- if you have multiple objects with the same script, the scripts are unique (as in, they don't share variables) - If you use 'if eval' instead of just 'if' you get an error in the game console if it fails, otherwise it silently bones the script. You can also do 'if eval !(Cond)' instead of 'if Cond == 0', which I personally prefer. Since you mentioned you wanted it to be easier to read- personally I always use global constants: True: 1, False: 0, None: 0 (for refs) and NotFound: -1 (for checking lists) Here is what I note in your script: 1) GetContainer may not return an actor, it could be a box or something, but some later functions should only be used on actors. Consider adding an "if eval !(rContainer.IsActor): return". Also, you check if rContainer is dead, you might also want to check GetDisabled too. 2) If (ListGetFormIndex PPAPowerReduced rArmor > -1 && listgetformindex PPAPowerNuclear rArmor > -1 && listgetformindex PPAAPA rArmor > -1 && rContainer.getequipped PPAAPAMFExp == 0 && rContainer.getdead == 0) Let iDoOnce := 0 ElseIf iDoOnce == 0 ; ** Else block: so never runs whilst above 'if' is true. But it does run if above was never true, since iDoOnce inits to 0 anyway If iSwapCounter < 3 Let iSwapCounter := sSwapCounter + 1 Else Let iDoOnce := 1 rContainer.additem PPAMAPACapAdapter 1 1 rContainer.unequipitem PPAAPACapAdapter 0 1 removeme ; * This destroys the instance of this script and the item it is attached to. However, it may run for few more frames before that takes effect, and the if statment might reset iDoOnce to 0 then this block repeat. EndIf EndIf And, you could check those formlists like this: array_var aFLs array_var aEntry ref rFL let aFLS := Ar_List FirstFL, SecondFL, ThirdFL...... foreach aEntry <- aFLs let rFL := aEntry["value"] if eval ((ListGetFormIndex rFL, rArmor) > NotFound) ; (I like to define a global constant NotFound == -1, makes things easier to read) .... break ; I don't need to check any more endif loop To stop an object script running every frame add: float fTimer ; * MUST BE FLOAT Begin GameMode if fTimer < 5 ; I want to run at max, once every 5 seconds let fTimer += GetSecondsPassed ; this returns seconds since last script run, not since first- so GetSecondsPassed ~= 1 / frames per second return endif let fTimer := 0 ...
ArgusSCCT Posted June 20, 2014 Posted June 20, 2014 Just one more question because there are some scripts in this mod that are just utter madness. For example, this insanity right here: scn PPAInstalledTROScript ref rContainer ref rArmor short sActive short sWasPressed short sSwitched short sCount short sDoOnce short sSpeedBoost short sSpeedDelta short sSwapCounter begin gamemode IF rContainer == 0 set rContainer to getcontainer ELSE IF PPADisableToggle.sActivate && PPADisableToggle.rTarget == rContainer return ENDif IF sSwitched IF player.getactorvalue ignorecrippledlimbs player.setactorvalue ignorecrippledlimbs 0 IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif player.setactorvalue ignorecrippledlimbs 1 ELSE IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif ENDif set sSwitched to 0 ENDif set rArmor to rContainer.getequippedobject 2 IF (isPowerArmor rArmor && rContainer.getequipped PPAServoDrag == 0 && rContainer.getequipped PPAServoQuiet == 0) && (listgetformindex PPAPowerReduced rArmor == -1 || rContainer.getequipped PPAAPAMFExp || rContainer.getequipped PPAT51bMFExp || rContainer.getequipped PPAAPACapAdapter || rContainer.getequipped PPAT51bCapAdapter) && rContainer.getdead == 0 set sDoOnce to 0 IF sActive == 0 IF (PPACharge > 0 && PPAThermalOverride == 1) set sActive to 1 setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red rContainer.modav speedmult PPATROBoost set sSpeedBoost to PPATROBoost set sSwitched to 1 ENDif ELSE IF (PPACharge <= 0 || PPAThermalOverride == 0) set sActive to 0 setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ELSEif PPATROBoost != sSpeedBoost set sSpeedDelta to PPATROBoost - sSpeedBoost rContainer.modav speedmult sSpeedDelta set sSpeedBoost to PPATROBoost set sSwitched to 1 ENDif ENDif IF (getkeypress 0 == PPAKeyTRO || getkeypress 1 == PPAKeyTRO || getkeypress 2 == PPAKeyTRO) IF sWasPressed == 0 IF PPAThermalOverride set PPAThermalOverride to 0 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain ENDif ELSE set PPAThermalOverride to 1 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red ENDif ENDif IF (PPAPAInit.fAlphaTimer1 <= 0 || PPAPAInit.sIndicator1 == 16) && PPAPAInit.sIndicator2 != 16 && PPAPAInit.sIndicator3 != 16 SetUIString "HudMainMenu\_PPAInitImage1File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage1AlphaMult" 1 set PPAPAInit.fAlphaTimer1 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator1 to 16 ELSEif (PPAPAInit.fAlphaTimer2 <= 0 || PPAPAInit.sIndicator2 == 16) && PPAPAInit.sIndicator3 != 16 SetUIString "HudMainMenu\_PPAInitImage2File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage2AlphaMult" 1 set PPAPAInit.fAlphaTimer2 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator2 to 16 ELSE SetUIString "HudMainMenu\_PPAInitImage3File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage3AlphaMult" 1 set PPAPAInit.fAlphaTimer3 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator3 to 16 ENDif set sWasPressed to 1 ENDif ELSE set sWasPressed to 0 ENDif ELSEif sDoOnce == 0 IF sSwapCounter < 48 set sSwapCounter to sSwapCounter + 1 ELSE set sDoOnce to 1 IF sActive setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ENDif rContainer.additem PPAMTRO 1 1 rContainer.unequipitem PPATRO 0 1 removeme ENDif ENDif ENDif end This another one of those "installed" scripts, which adds a Power Armor mod and its effects onto the player character. Either way, I don't even know where to begin deciphering how it works. I'd just rewrite it all if I knew exactly how to accomplish what it does. There's also the fact that some of these scripts are almost completely the same, I'm wondering if shouldn't just get rid of them all, and make a single script to add all the mods whose scripts are completely the same. I could also just can, this thing, and make my own implementation with the original mod's resources, instead of fixing these scripts.
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 Should I use NVAC? I installed it to let my GECK open GNR Cell but I think that all that NVAC does is prevent game\geck from crashing, leaving user with the error. So in terms of modding should I dispose of NVAC or one can mod with him?
Odessa Posted June 20, 2014 Posted June 20, 2014 Just one more question because there are some scripts in this mod that are just utter madness. For example, this insanity right here: ... This another one of those "installed" scripts, which adds a Power Armor mod and its effects onto the player character. Either way, I don't even know where to begin deciphering how it works. I'd just rewrite it all if I knew exactly how to accomplish what it does. There's also the fact that some of these scripts are almost completely the same, I'm wondering if shouldn't just get rid of them all, and make a single script to add all the mods whose scripts are completely the same. I could also just can, this thing, and make my own implementation with the original mod's resources, instead of fixing these scripts. This may help you to begin to decipher it, I will say, it looks like shoddy script writing to me. (uhh.. no offense to whoever wrote it): scn PPAInstalledTROScript ref rContainer ref rArmor short sActive short sWasPressed short sSwitched short sCount short sDoOnce short sSpeedBoost short sSpeedDelta short sSwapCounter ; * since this has getcontainer, it must be an object script, because that func is only valid inside one ; * Therefore, this gamemode block will run every single frame of the game, unless a menu is up. ; * the vars all start at zero before the first run and are then persistent regarding updates. ; * On 'RemoveMe' the script is destroyed however. begin gamemode IF rContainer == 0 ; True the first time the script runs set rContainer to getcontainer ; * this will return 0 if the object is not in a container ELSE ; * this block runs the frame after getcontainer returns not None IF PPADisableToggle.sActivate && PPADisableToggle.rTarget == rContainer return ENDif IF sSwitched ; * I think dealing 1 damage and then healing it is a method to force the engine to update something IF player.getactorvalue ignorecrippledlimbs player.setactorvalue ignorecrippledlimbs 0 IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif player.setactorvalue ignorecrippledlimbs 1 ELSE IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif ENDif set sSwitched to 0 ENDif set rArmor to rContainer.getequippedobject 2 ; * '2' is a magic number meaning 'upper body' eg: equipped armor/clothes IF (isPowerArmor rArmor && rContainer.getequipped PPAServoDrag == 0 && rContainer.getequipped PPAServoQuiet == 0) && (listgetformindex PPAPowerReduced rArmor == -1 || rContainer.getequipped PPAAPAMFExp || rContainer.getequipped PPAT51bMFExp || rContainer.getequipped PPAAPACapAdapter || rContainer.getequipped PPAT51bCapAdapter) && rContainer.getdead == 0 set sDoOnce to 0 ; * I think naming any variable that gets reset DoOnce is poor naming chance, since it isn't done once, DoUpdate would make more sense IF sActive == 0 IF (PPACharge > 0 && PPAThermalOverride == 1) ; * these look like global variables, probably using a quest var would have been better, since they aren't globally relevant, they are only PPA relevant set sActive to 1 setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red rContainer.modav speedmult PPATROBoost set sSpeedBoost to PPATROBoost ; * looks like anothing dubious global set sSwitched to 1 ENDif ELSE IF (PPACharge <= 0 || PPAThermalOverride == 0) set sActive to 0 setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ELSEif PPATROBoost != sSpeedBoost set sSpeedDelta to PPATROBoost - sSpeedBoost rContainer.modav speedmult sSpeedDelta set sSpeedBoost to PPATROBoost set sSwitched to 1 ENDif ENDif IF (getkeypress 0 == PPAKeyTRO || getkeypress 1 == PPAKeyTRO || getkeypress 2 == PPAKeyTRO) ; * I believe the number passed to 'getkeypress' function is magic, and corresponds to directX scan codes, you will have to look them up. It will only return true if the key is pressed during the frame the script runs. probably. IF sWasPressed == 0 IF PPAThermalOverride set PPAThermalOverride to 0 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain ENDif ELSE set PPAThermalOverride to 1 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red ENDif ENDif IF (PPAPAInit.fAlphaTimer1 <= 0 || PPAPAInit.sIndicator1 == 16) && PPAPAInit.sIndicator2 != 16 && PPAPAInit.sIndicator3 != 16 ; * looks like a hud mod is involved and this does stuff to it SetUIString "HudMainMenu\_PPAInitImage1File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage1AlphaMult" 1 set PPAPAInit.fAlphaTimer1 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator1 to 16 ELSEif (PPAPAInit.fAlphaTimer2 <= 0 || PPAPAInit.sIndicator2 == 16) && PPAPAInit.sIndicator3 != 16 SetUIString "HudMainMenu\_PPAInitImage2File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage2AlphaMult" 1 set PPAPAInit.fAlphaTimer2 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator2 to 16 ELSE SetUIString "HudMainMenu\_PPAInitImage3File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage3AlphaMult" 1 set PPAPAInit.fAlphaTimer3 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator3 to 16 ENDif set sWasPressed to 1 ENDif ELSE set sWasPressed to 0 ENDif ELSEif sDoOnce == 0 IF sSwapCounter < 48 ; * this is a timer, the counter increments by 1 for next 48 script iteraration (so, once per frame) to create a delay on the next part. Seems like the delay would be better at top of script with a return for efficiency, at a glance set sSwapCounter to sSwapCounter + 1 ELSE set sDoOnce to 1 IF sActive setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ENDif rContainer.additem PPAMTRO 1 1 rContainer.unequipitem PPATRO 0 1 removeme ; * destroys script, but it may run for a few more frames. This is dangerous, it would be better to add a "int Completed", then "let Completed := True" after the RemoveMe, then at the very top of the script, "if Completed: return", to make sure it doesn't run again when its supposed to be dead. ENDif ENDif ENDif end Another note: they are using shorts. In the GECK short === int (under the hood both are stored as floats and floored). I would suggest never using shorts, they are completely pointless. (Actually, this is nearly always true in real programming too, if you want to know why shorts exist, read a computer science history book.). Also, I think if the variable is intended as a boolean, prefacing it with 'i' or 's' is just confusing, either use 'b' or nothing.
nyaalich Posted June 20, 2014 Posted June 20, 2014 removeme ; * destroys script, but it may run for a few more frames. This is dangerous, it would be better to add a "int Completed", then "let Completed := True" after the RemoveMe, then at the very top of the script, "if Completed: return", to make sure it doesn't run again when its supposed to be dead. I'm uncertain of the context of this script (how it's being called). But this concerns me as that I'm currently just using "removeme" for any token stuff. Okay or not okay for tokens?
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 So, about ArgusSCTT script - why don't do "begin OnEquip" block(like PowerArmor in NW starts disguise quest) and start a new quest with quest delay of some value(I see that there is a "keypressed" condition - I don't know what quest delay would be appropriate). That way: 1)we minimize script running in gamemode block (all references, keys and variables are now set in OnEquip block) and all script in overral. 2) No need to worry about setting token delay - Now delay is set by quest 3)quests are more reliable So. anyone knows minimum appropriate quest delay for getkeypressed condition?
Guest Posted June 20, 2014 Posted June 20, 2014 So. anyone knows minimum appropriate quest delay for getkeypressed condition? I stick with .1/.2 usually, it's pretty smooth and not that heavy to be handled
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 So. anyone knows minimum appropriate quest delay for getkeypressed condition? I stick with .1/.2 usually, it's pretty smooth and not that heavy to be handled I expected it to be smaller. 0.1 should do fine then. Somebody should be really quick with buttons to press the key between checks.
ArgusSCCT Posted June 20, 2014 Posted June 20, 2014 This may help you to begin to decipher it, I will say, it looks like shoddy script writing to me. (uhh.. no offense to whoever wrote it): scn PPAInstalledTROScript ref rContainer ref rArmor short sActive short sWasPressed short sSwitched short sCount short sDoOnce short sSpeedBoost short sSpeedDelta short sSwapCounter ; * since this has getcontainer, it must be an object script, because that func is only valid inside one ; * Therefore, this gamemode block will run every single frame of the game, unless a menu is up. ; * the vars all start at zero before the first run and are then persistent regarding updates. ; * On 'RemoveMe' the script is destroyed however. begin gamemode IF rContainer == 0 ; True the first time the script runs set rContainer to getcontainer ; * this will return 0 if the object is not in a container ELSE ; * this block runs the frame after getcontainer returns not None IF PPADisableToggle.sActivate && PPADisableToggle.rTarget == rContainer return ENDif IF sSwitched ; * I think dealing 1 damage and then healing it is a method to force the engine to update something IF player.getactorvalue ignorecrippledlimbs player.setactorvalue ignorecrippledlimbs 0 IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif player.setactorvalue ignorecrippledlimbs 1 ELSE IF player.getav leftmobilitycondition player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ELSE player.damageav leftmobilitycondition 1 player.restoreav leftmobilitycondition 1 ENDif ENDif set sSwitched to 0 ENDif set rArmor to rContainer.getequippedobject 2 ; * '2' is a magic number meaning 'upper body' eg: equipped armor/clothes IF (isPowerArmor rArmor && rContainer.getequipped PPAServoDrag == 0 && rContainer.getequipped PPAServoQuiet == 0) && (listgetformindex PPAPowerReduced rArmor == -1 || rContainer.getequipped PPAAPAMFExp || rContainer.getequipped PPAT51bMFExp || rContainer.getequipped PPAAPACapAdapter || rContainer.getequipped PPAT51bCapAdapter) && rContainer.getdead == 0 set sDoOnce to 0 ; * I think naming any variable that gets reset DoOnce is poor naming chance, since it isn't done once, DoUpdate would make more sense IF sActive == 0 IF (PPACharge > 0 && PPAThermalOverride == 1) ; * these look like global variables, probably using a quest var would have been better, since they aren't globally relevant, they are only PPA relevant set sActive to 1 setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red rContainer.modav speedmult PPATROBoost set sSpeedBoost to PPATROBoost ; * looks like anothing dubious global set sSwitched to 1 ENDif ELSE IF (PPACharge <= 0 || PPAThermalOverride == 0) set sActive to 0 setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ELSEif PPATROBoost != sSpeedBoost set sSpeedDelta to PPATROBoost - sSpeedBoost rContainer.modav speedmult sSpeedDelta set sSpeedBoost to PPATROBoost set sSwitched to 1 ENDif ENDif IF (getkeypress 0 == PPAKeyTRO || getkeypress 1 == PPAKeyTRO || getkeypress 2 == PPAKeyTRO) ; * I believe the number passed to 'getkeypress' function is magic, and corresponds to directX scan codes, you will have to look them up. It will only return true if the key is pressed during the frame the script runs. probably. IF sWasPressed == 0 IF PPAThermalOverride set PPAThermalOverride to 0 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain ENDif ELSE set PPAThermalOverride to 1 IF PPACharge setUIFloat "HUDMainMenu\_PPAImage1Alert" 1 ; red ENDif ENDif IF (PPAPAInit.fAlphaTimer1 <= 0 || PPAPAInit.sIndicator1 == 16) && PPAPAInit.sIndicator2 != 16 && PPAPAInit.sIndicator3 != 16 ; * looks like a hud mod is involved and this does stuff to it SetUIString "HudMainMenu\_PPAInitImage1File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage1AlphaMult" 1 set PPAPAInit.fAlphaTimer1 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator1 to 16 ELSEif (PPAPAInit.fAlphaTimer2 <= 0 || PPAPAInit.sIndicator2 == 16) && PPAPAInit.sIndicator3 != 16 SetUIString "HudMainMenu\_PPAInitImage2File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage2AlphaMult" 1 set PPAPAInit.fAlphaTimer2 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator2 to 16 ELSE SetUIString "HudMainMenu\_PPAInitImage3File" "PPA\Icons\PPA_Pic_TRO.dds" SetUIFloat "HudMainMenu\_PPAInitImage3AlphaMult" 1 set PPAPAInit.fAlphaTimer3 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator3 to 16 ENDif set sWasPressed to 1 ENDif ELSE set sWasPressed to 0 ENDif ELSEif sDoOnce == 0 IF sSwapCounter < 48 ; * this is a timer, the counter increments by 1 for next 48 script iteraration (so, once per frame) to create a delay on the next part. Seems like the delay would be better at top of script with a return for efficiency, at a glance set sSwapCounter to sSwapCounter + 1 ELSE set sDoOnce to 1 IF sActive setUIFloat "HUDMainMenu\_PPAImage1Alert" 0 ; hudmain set sSpeedBoost to sSpeedBoost * -1 rContainer.modav speedmult sSpeedBoost set sSpeedBoost to 0 set sSwitched to 1 ENDif rContainer.additem PPAMTRO 1 1 rContainer.unequipitem PPATRO 0 1 removeme ; * destroys script, but it may run for a few more frames. This is dangerous, it would be better to add a "int Completed", then "let Completed := True" after the RemoveMe, then at the very top of the script, "if Completed: return", to make sure it doesn't run again when its supposed to be dead. ENDif ENDif ENDif end Another note: they are using shorts. In the GECK short === int (under the hood both are stored as floats and floored). I would suggest never using shorts, they are completely pointless. (Actually, this is nearly always true in real programming too, if you want to know why shorts exist, read a computer science history book.). Also, I think if the variable is intended as a boolean, prefacing it with 'i' or 's' is just confusing, either use 'b' or nothing. Yeah I know about shorts, I never use them, not with GECK or in any other place for that matter, I got rid of them in other scripts. The main issue as I see it with these scripts is that they have a lot of nested ifs and elseifs, makes it super hard to see what's going on and this is not the only script like this, I doubt this one is the worst of them. Honestly, I'm thinking of finding out what it is that they do and rewriting them, I can't work with that mess. I don't know, every time I get more into the mod, it seems like a rewrite would be the best thing to do. It just has so many unusual ways of doing things. So, about ArgusSCTT script - why don't do "begin OnEquip" block(like PowerArmor in NW starts disguise quest) and start a new quest with quest delay of some value(I see that there is a "keypressed" condition - I don't know what quest delay would be appropriate). That way: 1)we minimize script running in gamemode block (all references, keys and variables are now set in OnEquip block) and all script in overral. 2) No need to worry about setting token delay - Now delay is set by quest 3)quests are more reliable So. anyone knows minimum appropriate quest delay for getkeypressed condition? I've thought about that. Maybe to create a script that's starts doing stuff once the player equips power armor and has some mods in his/her inventory. The other thing would be to allow the player to add and remove the mods as he/she pleases, instead of having the mod add them automatically. It'd get rid of those quest scripts.
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 So, is this mod already released? I don't quite understand what is all this about. begin OnEquip only executes once so if player acquires mods he will have to reequip armor again which seems logical if you ask me. You can easily add modds conditions to OnEquip block.
jaam Posted June 20, 2014 Posted June 20, 2014 removeme ; * destroys script, but it may run for a few more frames. This is dangerous, it would be better to add a "int Completed", then "let Completed := True" after the RemoveMe, then at the very top of the script, "if Completed: return", to make sure it doesn't run again when its supposed to be dead. I'm uncertain of the context of this script (how it's being called). But this concerns me as that I'm currently just using "removeme" for any token stuff. Okay or not okay for tokens? This way of guarding the script should be applied wherever removeMe is used yes. EDIT or disable or dispel... Pretty much every time you remove something from the game, you should guard the script in case things gets done a little bit too slow.
ArgusSCCT Posted June 20, 2014 Posted June 20, 2014 So, is this mod already released? I don't quite understand what is all this about. begin OnEquip only executes once so if player acquires mods he will have to reequip armor again which seems logical if you ask me. You can easily add modds conditions to OnEquip block. Yeah its an old mod, which has not been updated for a while. It never worked quite right, and it always had problems when interacting with older mods. A friend of mine got permission to update it, he doesn't know about scripting so I offered to do it. The problem is this: the mod is old, there are crazy scripts like the one I mentioned before. I'm still not sure on whether I should just forget about the old scripts and make new ones that look and behave better or try to fix them, which might end up being harder in the long run. I don't know how to get rid of all those nested conditionals.
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 if you specify your problem, we can help you. You can unnest conditions easily You have if condition1 ==1 && condition2 ==1 && condition3 ==1 ;do stuff endif You can turn it into if condition1 ==1 if condition2 ==1 if contidion3 ==1 ;do stuff endif endif endif Difference is - if condition 1 ==0 - in first case game checks all conditions anyway and that takes CPU power. In second case game checks only condition1 and untill it is not eval to 1, engine won't check condition2(it will spend some very very tiny small CPU power to skip it but that's okay).
ArgusSCCT Posted June 20, 2014 Posted June 20, 2014 if you specify your problem, we can help you. You can unnest conditions easily You have if condition1 ==1 && condition2 ==1 && condition3 ==1 ;do stuff endif You can turn it into if condition1 ==1 if condition2 ==1 if contidion3 ==1 ;do stuff endif endif endif Difference is - if condition 1 ==0 - in first case game checks all conditions anyway and that takes CPU power. In second case game checks only condition1 and untill it is not eval to 1, engine won't check condition2(it will spend some very very tiny small CPU power to skip it but that's okay). I think I've been explaining myself wrong. There is no problem, at least not with the scripts, they do what they have to do, but just barely if you ask me and with a lot of issues in the middle if you ask me. The problem is that they're a huge mess and I don't know where to start with regards to flattening a script with that many conditionals everywhere. The example I posted before is what I'm referring to, its a huge mess, I just don't have a clue how to make it smaller, cleaner and more understandable. That is basically my issue, I want to change to scripts to look understandable and functional, not like a crazy mess. If (condition) If (condition) do stuff ElseIf do other stuff Else If (condition) do stuff Else If (condition) I don't want that sort of mess, and I'm sure that most of that stuff can be avoided by using loops, or merging it into one conditional instead of having 4 of them within each other, each with its elseif and elses. I think it's very clear why having that mess, is not desirable to me. This is my first time dealing with someone else's huge code, which is why I'm having a hard time trying to figure out how to fix things. Basically my question is this: Is it worth it to keep a mess like this? scn PPAInstalledNinjaScript ref rContainer short sActive short sCount short sDoOnce short sTROCounter float fTSubUpper float fTSubLower float fDT short sSwapCounter begin gamemode getsecondspassed set sCount to 0 IF rContainer == 0 set rContainer to getcontainer ELSE IF PPADisableToggle.sActivate && PPADisableToggle.rTarget == rContainer return ENDif IF rContainer == player && player.getequipped PPATRO && (player.getequipped PPALCLower || player.getequipped PPALCUpper) set sDoOnce to 0 IF PPAThermalOverride IF sTROCounter == 0 ELSEif sTROCounter < 40 && player.getitemcount PPANitro set sTROCounter to 0 IF (PPAPAInit.fAlphaTimer1 <= 0 || PPAPAInit.sIndicator1 == 6) && PPAPAInit.sIndicator2 != 6 && PPAPAInit.sIndicator3 != 6 SetUIString "HudMainMenu\_PPAInitImage1File" "PPA\Icons\PPA_Pic_Ninja.dds" SetUIFloat "HudMainMenu\_PPAInitImage1AlphaMult" 1 set PPAPAInit.fAlphaTimer1 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator1 to 6 ELSEif (PPAPAInit.fAlphaTimer2 <= 0 || PPAPAInit.sIndicator2 == 6) && PPAPAInit.sIndicator3 != 6 SetUIString "HudMainMenu\_PPAInitImage2File" "PPA\Icons\PPA_Pic_Ninja.dds" SetUIFloat "HudMainMenu\_PPAInitImage2AlphaMult" 1 set PPAPAInit.fAlphaTimer2 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator2 to 6 ELSE SetUIString "HudMainMenu\_PPAInitImage3File" "PPA\Icons\PPA_Pic_Ninja.dds" SetUIFloat "HudMainMenu\_PPAInitImage3AlphaMult" 1 set PPAPAInit.fAlphaTimer3 to PPAPAInit.fAlphaTime set PPAPAInit.sIndicator3 to 6 ENDif player.removeitem PPANitro 1 1 player.additem PPANitroD 1 1 playsound PPANinjaSteam IF player.getequipped PPALCLower && player.getequipped PPALCUpper set fTSubLower to 120 set fTSubUpper to 120 ELSEif player.getequipped PPALCLower set fTSubLower to 240 ELSE ; LCUpper set fTSubUpper to 240 ENDif ELSE set sTROCounter to 0 ENDif ELSE set sTROCounter to sTROCounter + 1 ENDif IF fTSubLower > 0 set fDT to getsecondspassed * (20 + 2 * fTSubLower) set fTSubLower to fTSubLower - fDT set PPATLower to PPATLower - fDT ENDif IF fTSubUpper > 0 set fDT to getsecondspassed * (20 + 2 * fTSubUpper) set fTSubUpper to fTSubUpper - fDT set PPATUpper to PPATUpper - fDT ENDif IF player.getitemcount PPANitro IF fTSubUpper < 0 IF fTSubLower <= 0 playsound WPNFlamerReload set fTSubLower to 0 set fTSubUpper to 0 ENDif ELSEif fTSubLower < 0 IF fTSubUpper <= 0 playsound WPNFlamerReload set fTSubLower to 0 set fTSubUpper to 0 ENDif ENDif ENDif ELSEif sDoOnce == 0 IF sSwapCounter < 18 set sSwapCounter to sSwapCounter + 1 ELSE set sDoOnce to 1 rContainer.additem PPAMNinja 1 1 rContainer.unequipitem PPANinja 0 1 removeme ENDif ENDif ENDif end Is it better to just forget about fixing something like that and making it all over again, regardless of needing to test everything all over again?
Guest tomm434 Posted June 20, 2014 Posted June 20, 2014 I think it's your choice. Reading someones other script is always hard, sometimes when I see my old scripts, I'll spent some time debugging before I can tart editing it.
ArgusSCCT Posted June 20, 2014 Posted June 20, 2014 I think it's your choice. Reading someones other script is always hard, sometimes when I see my old scripts, I'll spent some time debugging before I can tart editing it. Ultimately it is, but other more experienced and older programmers I know have told me its better to keep the old stuff. I personally don't know if that works in this case, I can't figure out what does what and which things go where. Plus, I don't know how I'd replicate what this person made in my own code.
Halstrom Posted June 20, 2014 Author Posted June 20, 2014 If you want to do things the previous programmer wasn't doing and add a few new tricks to the mod then sometimes a rewrite is a better option. Maybe NVSE wasn't even around when it was written if it's realy old, I probably could be a lot simpler and more powerful too with NX Variable usage.
Odessa Posted June 20, 2014 Posted June 20, 2014 Rewriting any pre NVSE4 script using NVSE4 is never a bad idea in my opinion. In the bad old days even the foxiest modders were lowering themselves to things they'd be ashamed of now. Also, although the scripts might seem to work most the time, they are definitely not water tight, and it might be just as fast to rewrite them as while making sense of them. You probably don't need to rewrite all of them at once.
ArgusSCCT Posted June 21, 2014 Posted June 21, 2014 Rewrite it is, and I believe part of the reason those scripts are what they are is because they are pre NVSE4, this is an old mod, I think it was released back in 2010-2011.
ArgusSCCT Posted June 21, 2014 Posted June 21, 2014 Just one other thing, I realized why these scripts use GameMode but I want to switch that, there are like 10 Power Armor mod scripts, and if a person is running all ten that is bound to cause issues. I assume they use game mode because those scripts have to run as long as the person is using power armor, in order to maintain the effects of the Power Armor Mods. This needs to change, the only thing I'm not sure of is if it can be accomplished through OnEquip, since this will only work when they're equipped.
Guest Posted June 21, 2014 Posted June 21, 2014 It depends by what they are doing. A GameMode can't be replaced, if it really needs some action is continuously running. But a GameMode can be staged, the best deal would be sticking with the same script, attached to the item, but delaying it with a timer like Odessa wrote some posts ago. If they apply debuff it could be done by a spell which is very reliable for that, but putting some other specific script in a ScriptEffectUpdate wouldn't make sense since you would be in the same exact situation as in the current script. I saw a part of the script was changing the hud, so that part can stick on a Quest Script (delay 0.1 is fine for huds) and OnEquip you only trigger some variable value that activate the Quest Script Block. It all depends by all the functions your armors' scripts execute.
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now