search

shadowmage45 / SSTULabs

Dev repository for testing/unfinished KSP parts/plugins/etc.
Other
62 stars 41 forks source link

SolarModule : EC output is wrongly multiplied by TimeWarp.fixedDeltaTime instead of divided #794

Closed gotmachine closed 4 years ago

gotmachine commented 5 years ago

https://github.com/shadowmage45/SSTULabs/blob/d6b9982af91ddbfc7c3bdcdbb2b803add229a3d2/Plugin/SSTUTools/SSTUTools/ModelDefinition/SolarModule.cs#L226

shadowmage45 commented 5 years ago

I believe this is correct. The configs specify EC in EC/s. The part consumes it in EC/physics tick. FIxedDeltaTime is the portion of a second happening in a physics tick, as a scalar.

Thus, to convert from one seconds worth of EC to one physics-tick worth, you multiply: One Second EC Value * Portion of a Second =EC value for portion of a second.

Unless I've forgotten something fundamental about math?

gotmachine commented 5 years ago

You are converting from EC/s to EC/tick a few lines before : https://github.com/shadowmage45/SSTULabs/blob/d6b9982af91ddbfc7c3bdcdbb2b803add229a3d2/Plugin/SSTUTools/SSTUTools/ModelDefinition/SolarModule.cs#L219 But obviously at line 226 you intend to do the reverse conversion, EC/tick to EC/s, that would require doing totalOutput / TimeWarp.fixedDeltaTime, not totalOutput * TimeWarp.fixedDeltaTime

And also : TimeWarp.fixedDeltaTime isn't a scalar, it's the amount of ingame seconds per physic tick. Can, and will be > 1.0 at high timewarp speeds.

shadowmage45 commented 5 years ago

You are converting from EC/s to EC/tick a few lines before :

I got ya. Indeed, there does appear to be a double multiplication in there.

I'll have to take a look at that code and figure out what it was supposed to be doing, as obviously its a bit incorrect.

TimeWarp.fixedDeltaTime isn't a scalar, it's the amount of ingame seconds per physic tick. Can, and will be > 1.0 at high timewarp speeds.

Yes, its a scalar, that represents the number of seconds per physics tick. If it is > 1, that just means that there is more than one second per physics tick (scalars can be >1). Not meaning to argue, I think we are both meaning the same thing.

Thankfully this error: https://github.com/shadowmage45/SSTULabs/blob/d6b9982af91ddbfc7c3bdcdbb2b803add229a3d2/Plugin/SSTUTools/SSTUTools/ModelDefinition/SolarModule.cs#L226 doesn't directly impact the EC output, only the cached/exposed value. Yes, it is a problem, but hopefully the panels will continue to function until it can be fixed.