-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add RealFuels support #4
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR. I am not using RF, did you test this and nwhat does it do? Why mass * 1000? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would still be missing the RealBattery module for the tanks, a SC only tank would make no sense, since SC can't flow to another part. How will the RealBattery module be added?
I replied to your post on the KSP forums but I'll also leave this here so you have a record as part of this PR: Real Fuels lets the player control what resources are assigned to a 'tank' part. (tank can be a loose term; in this case it would be like a battery) The thing that determines what a given tank can hold is the TANK_DEFINITION. The patch in that pull request lets any tank that could have electric charge (basically adding batteries to the part) also add StoredCharge. As you note in the PR comments, by itself it seems to do nothing. The player could configure tank parts with StoredCharge but if, as you say, your mod / resource definition won't allow for StoredCharge to actually flow into the part then it's probably pretty useless. Nothing else in there adds your mods modules to the part. (the kinds of parts that this would affect in RF would be plane fuselages, service modules and electric propulsion tank parts (Xenon tanks that also contain batteries for ion engines) About the mass / utilization... that looks really screwy because the end result is that one StoredCharge would require a 'tank' mass (really battery mass) of 2.89 metric tons due to the math involved. Mass is the tank mass for one volume unit and utilization is how volume unit of the resource will fit in one tank volume unit... EC had a utilization of 1000 and mass of 0.00289 so... yeah, it's going to end up as 2.89 tons per volume unit of SC.... I am not familiar with your mod and what kind of power density your batteries (capacitors?) have so I could be wrong but that seems screwy to me. |
I somewhat messed up the testing, not sure what happened there. I just added the missing module but did not fix the incorrect mass for now. Unfortunately it seems that there is more to to than just taping the parts together. RealBattery uses the part mass for calculations (RealBattery.cs#L344) and there is a null-ref when no StoredCharge is present (RealBattery.cs#L437). The presence of StoredCharge should also be involved when displaying the battery stats and in the "LoadMaster"-mechanism. Thats a bigger change than I thought, unfortunately I do neither have the time nor the development environment at the moment. So feel free to close this PR or leave it open for the future and/or someone else. |
No description provided.