Check over my plugin?

[Benjamin Kerman]

Hello modding community!

I have been helping with a mod by @DarknessHasLost and getting some great advice from @linuxgurugamer, and was hoping the community could look over the plugin that I wrote with Linux's help. 

https://github.com/Darknesshas1/DK-BHTanks/blob/master/DK-BHTanks/GameData/BlackHoleTanks/Plugins/BHStorageCfg.cs

If anyone has advice, just comment. 

Thanks!

[HebaruSan]

What does it do?

[Crzyrndm]

A short explanation of purpose would have been somewhat useful, and generally it helps if the code for review compiles to begin with (assigning from a void function being one among many that would prevent that).

Folder structure
I would put only the compiled .dll inside GameData, not the source file (.cs). GameData to me is more like the "Release" folder which only contains the necessary components.
NOTE: ^^ Purely my personal opinion

Data Structure
You've artificially limited your plugin to two fuel types and aren't grouping information about each fuel type together.

using System.Collections.Generic;
public class FuelType
{
  public string Name;
  public float MaxQuantity;
  public float LastQuantity;
  // other members...
}
// 0, 1, ... N types
List<FuelType> fuels;

Again, a little personal style coming through on the use of the List<>, but it prevents errors where you make a change for item1 but not item2 and sure beats having to copy it all out later if you do need another type. It does take a little more effort to load (override OnLoad to get the fuels in...)
NOTE: reading the comments up top, definitely need a collection...

Possible bug
"Just a little" suspicious of this logic: https://github.com/Darknesshas1/DK-BHTanks/blob/master/DK-BHTanks/GameData/BlackHoleTanks/Plugins/BHStorageCfg.cs#L80-L83
Even if it's not a bug because of some side effect in GetMaxResourceAmount (definition where?), that looks likely to catch you out at some point in the future

Bug
https://github.com/Darknesshas1/DK-BHTanks/blob/master/DK-BHTanks/GameData/BlackHoleTanks/Plugins/BHStorageCfg.cs#L138
1) You're calling catchup on the lower warp rates (<= 5x multiplier), not the higher rates
2) You probably want to be checking TimeWarp.WarpMode instead of the multiplier (there are mods which fiddle with the rates, the >5 already is a logical error with the lowest time warp rate getting paired with phys warp, and is a magic number that doesn't express your intent).
3) Pulling all available EC in a single frame
4) if (Ec <= BHECCost || Ec == 0.0f), avoid repeating yourself to save yourself headaches later (add brackets or newlines to suit taste if wanted)
5) If you're not going to do anything with it, don't create the variable, and definitely don't just list it in an empty statement (if it does compile, it still makes me question whether something got screwed up in a formatting shenanigan)

[Benjamin Kerman]

@HebaruSan: The plugin is meant to shut off fuel access when EC is not provided. 

@Crzyrndm: Thanks for your help! I'll look over the bugs you pointed out. I also added an IF loop for having either one or two fuel tanks, if you want to look at that. Still reading up on the syntax of C#, so please forgive any stupid questions!

[Benjamin Kerman]

Hello again! 

I belive I have finished the plugin! Could you guys take a look at it one more time? 

[HebaruSan]

On 4/10/2017 at 11:07 AM, Benjamin Kerman said:

@HebaruSan: The plugin is meant to shut off fuel access when EC is not provided. 

Thanks for the explanation, but I meant that question as a remark on the code: It should be easier to tell what it does from looking at the file. One way to accomplish that would be to change that comment at the top to say, "The plugin is meant to shut off fuel access when EC is not provided." 

[Benjamin Kerman]

Ok  Any suggestions as far as the code itself? Will it do what I want it to do?