Fixed gain/loss of resource in transfers

PapaJoesSoup / ShipManifest

Kerbal Space Program Addon. Manages Crew, Science, & Resources on a given vessel.

Other

32 stars 25 forks source link

Fixed gain/loss of resource in transfers #48

Closed arivaldh closed 6 years ago

arivaldh commented 6 years ago

Fixed gain/loss or resource in multitank to multitank transfers. Also:

better comments!
variable name changes in order to reflect their purpose
creation of intermediate list in order not to duplicate skip conditions (if they change in only one loop where doomed)

arivaldh commented 6 years ago

Please merge or discuss why it's not being merged.

arivaldh commented 6 years ago

I have removed the use of Enumerator since: "The foreach statement of the C# language (for each in C++, For Each in Visual Basic) hides the complexity of enumerators. Therefore, using foreach is recommended, instead of directly manipulating the enumerator." https://msdn.microsoft.com/en-us/library/x854yt9s(v=vs.110).aspx

And I personally don't like them, since they give us nothing more than foreach. And foreach is cleaner. I have tested the change for multiple Solar Panels per Part.

PapaJoesSoup commented 6 years ago

That is true when using C# with the Microsoft garbage collector. However, the Mono garbage collector Used in Unity does not play well with Foreach. Therefore you end up with a significant number of orphaned objects, reducing your available memory. So to combat this the use of enumerators avoids the additional object generation in Mono, and reduces the garbage footprint. I have tested and profiled this behavior in multiple Mods, including BDA. it was a huge improvement to garbage collection.

As for the branches, I do all my work in progress against Dev. Master is always the release version. I keep Dev up to date with my local wip often, so there should not be any significant uncommitted work. Yes, please move the PRs to the Dev branch I would appreciate that, thanks!

arivaldh commented 6 years ago

Ok. Then I'll restore use of Enumerator. And try to find the issue with Antennas that's bugging my campaign.

arivaldh commented 6 years ago

Changed merge to: from: arivaldh:master to: PapaJoesSoup:Dev

arivaldh commented 6 years ago

I have fixed Antenna Control when RemoteTech is present. For now I'll be switching to different tasks.

arivaldh commented 6 years ago

Anything going on? Do you need anything more?