-
Notifications
You must be signed in to change notification settings - Fork 277
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 HG-3 and HG-3-SL #1609
Add HG-3 and HG-3-SL #1609
Conversation
Add HG-3 and HG-3-SL
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.
when doing a +part you must use BEFORE not FOR
@@ -167,6 +167,107 @@ | |||
} | |||
engineType = J2 | |||
} | |||
// HG-3 | |||
+PART[SXTLT80]:FOR[RealismOverhaul] |
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.
should be changed to +PART[SXTLT80]:BEFORE[RealismOverhaul]
engineType = HG3 | ||
} | ||
// HG-3-SL | ||
+PART[SXTLT80]:FOR[RealismOverhaul] |
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.
change to +PART[SXTLT80]:BEFORE[RealismOverhaul]
Incorporated raidernick's change requests.
RN Change 1
Changes made |
BTW, the TweakScale stuff for the engine parts is not required anymore, as we have a global patch to take care of that: https://github.com/KSP-RO/RealismOverhaul/blob/master/GameData/RealismOverhaul/RO_Engines.cfg#L17-L25 |
Acknowledged, removed it to ensure there was no interference with the global patch. |
@name=HG-3 | ||
%RSSROConfig = True | ||
{ | ||
} |
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.
Module was deleted in 90d1cb6 but you forgot the brackets
@name=HG-3-SL | ||
%RSSROConfig = True | ||
{ | ||
} |
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.
same here
Thank you for pointing that out. |
Realized that considering the shared heritage of the J-2 and HG-3, it didn't make sense that the HG-3 wouldn't be able to throttle as deeply.
Well, now that I know that engine configs are a thing, would it be a good idea for me to add the HG-3 and HG-3-SL to the J-2 series engine configs? The HG-3 and HG-3-SL were derived from the J-2, and have higher performance. |
No. They're different engines, not upgrades.
That'd be like putting HG-3 and RS-25 configs on the same part.
…On Apr 27, 2017 3:26 PM, "Ash19256" ***@***.***> wrote:
Well, now that I know that engine configs are a thing, would it be a good
idea for me to add the HG-3 and HG-3-SL to the J-2 series engine configs?
The HG-3 and HG-3-SL were derived from the J-2, and have higher performance.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1609 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE5-IrG6VqDCP7JRM-huEwrGToTJPl1rks5r0RYDgaJpZM4NJ9_N>
.
|
Add HG-3 and HG-3-SL as selectable configurations.
Created HG-3 engine and HG-3 Config. |
name = ModuleEngineConfigs | ||
type = ModuleEngines | ||
configuration = J-2 | ||
@configuration:NEEDS[RP-0] = J-2-200klbf |
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.
configuration needs changing to HG-3
and the @configuration:NEEDS
line can probably go
Would it make sense to give the upper stage variant extra ignitions? considering it was considered for use on the Saturn MLV's modified S-IVB stage. |
Correcting the configuration and removing a nonsensical requirement.
Add temporary HG-3 plume effect.
Alright, so that should fix the Test Flight thing. Also, added a temporary Real Plume config courtesy of Cosmonaut Crash on YouTube. I'll just get this all compiled and off to him for more testing prior to an official merging of this pull request. |
@PhineasFreak is this good now? It has a plume config (albeit not necessarily the most fitting one for the vacuum variant), and the TestFlight stuff should be working. |
@Ash19256 the ModuleEnginesRF fields are not needed anymore (they are patched automatically) but otherwise it is good! |
Removing unneeded ModuleEnginesRF patches.
Fixed that. Looks like this is ready for merging otherwise. |
Does this already config the SHIP version? https://github.com/Saabstory88/RO_Extended |
Uh.. no. I'll get on that right away. |
Adding SHIP compatibility.
Adding changes to the SHIP HG-3s
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.
You could migrate these two to be part of the global engine config. That way you won't have to worry about copying and pasting these lines between different parts.
Right, that should add SHIP compatibility to this pull request. I'll see what I can do about testing it, but otherwise it should be fine. |
@Ash19256 disregard my initial comment, it did not work as i expected to. I was talking about these two lines: |
Oh, okay. I wasn't sure which lines you were talking about. Those are still in the SHIP/Engines.cfg file. |
Yea, it was a bit of a brainfart... |
Out of curiosity, is there a tutorial or something for making a RP-0 PR for something like this? Is it something that I should only do after this PR gets approved/merged or what? Also, is there a How To guide type thing someone could link me to if it's okay to make a pull request to add this into the RP-0 tech tree at this time? EDIT: Or is the fact that RP-0 flags them as "Non RP-0" a fail on my part, because the configs are missing something that they should have? |
Realized that because the SHIP HG-3 parts are already existing parts, the @name field is probably rather irrelevant.
RP-0 needs to be made aware, yeah--best bet is to talk to @pap1723 about how to integrate them into the new tree, since the methodology has changed significantly from the current release build's way. |
So should I just wait for @pap1723 to make an appearance? |
I have appeared! @Ash19256 What that means is that the parts were not priced and place on the Tech Tree. So RP-0 will automatically flag them as Non-RP0. Do you have real-world pricing on these parts? |
@pap1723 Unfortunately no, I've only been able to find 3 sources, none of which list a unit cost. Considering the HG-3 and it's seal level variant were never actually built IRL (it was, as far as the sources I've found have indicated, an on-paper project only, with no prototypes built), that's not really surprising. Considering the time period and other engines available at the time, I'd imagine that it'd be probably somewhere in the same price range (probably around 5-10% more expensive) than the J-2S, which my sources indicate it's a contemporary of. Take into account the higher chamber pressure making manufacturing more expensive, and making it cost more than the roughly similarly sized J-2S makes sense to me. But ultimately, the price would be a best guesstimate, simply because no IRL price for the engines can be found. |
If memory serves the HG-3 was, like, halfway between J-2 and RS-25*, so I'd suggest a price somewhere between them rather than so close to J-2, maybe? *RS-25 was developed using a lot of the techniques used for HG-3, IIRC. |
Yeah, that sounds vaguely right. Although, based on it's capabilities, I'd lean more towards the J-2 side of things than the RS-25 side of things, mostly because the engine only produces around 27% more thrust for the same fuel consumption. That being said, it's ultimately up to @pap1723 and the others, so I'll go along with whatever they decide the price should be. I'll just need to know what to change in the configs to make them not get flagged as non RP-0. |
Credit to Cosmonaut Crash on YouTube for these updates.
Right, with this recent update this PR should be compatible with the SHIP mod. All that's needed now is to figure out RP-0 compatibility. |
Great! RP-0 stuff is done on RP-0 side so that's all set! :) |
Adding two historical engines that IRL were stepping stones between the J-2 and the RS-25 aka Space Shuttle Main Engine.