search

ak86 / Milk-Mod-Economy

http://www.loverslab.com/files/file/1382-milk-mod-economy/
2 stars 0 forks source link

MilkQuest.psc: suspicious code in lines 1383-1388 #2

Closed ghost closed 8 years ago

ghost commented 8 years ago

The code in lines 1383-1388 in MilkQuest.psc looks suspicious. Why is the value immediately "set,read and set again" again?

StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", LactacidCnt)
if StorageUtil.GetFloatValue(akActor,"MME.MilkMaid.LactacidCount") >= 1
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", StorageUtil.GetFloatValue(akActor,"MME.MilkMaid.LactacidCount") - 1)
else
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", 0)
endif

Here's what I would do:

Optimization 1 - remove set/get cycle (I don't see a reason why it would be necessary.)

if LactacidCnt >= 1
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", LactacidCnt - 1)
else
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", 0)
endif

Optimization 2 - set the difference instead of absolute value (Purely optical change. Intended to make the intention a bit clearer and the code shorter.)

if LactacidCnt >= 1
    StorageUtil.AdjustFloatValue(akActor,"MME.MilkMaid.LactacidCount", -1)
else
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", 0)
endif

Optimization 3) - this ~may~ be able to avoid some strange floating point issues (I don't know in detail how Papyrus handles comparison between floats and integers, moving the 'Count=1' case to the else-branch neatly avoids strange cases where LactacidCount could evaluate equal to 1 (due to roundoff/conversion) but 'LactacidCount-1' does NOT equal exactly 0. Floating point arithmetic can be surprising - better not to take chances...)

if LactacidCnt > 1
    StorageUtil.AdjustFloatValue(akActor,"MME.MilkMaid.LactacidCount", -1)
else
    StorageUtil.SetFloatValue(akActor,"MME.MilkMaid.LactacidCount", 0)
endif
ak86 commented 8 years ago

okay, ill take 3rd variant