Skip to content
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

MMSEV RO Configs #2571

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

MMSEV RO Configs #2571

wants to merge 2 commits into from

Conversation

JohnMcLane4
Copy link

MMESV Configs, heavily oriented on the SSPX configs by Tagir-1.
Should be looked over. But its a start.

MMESV Configs, heavily oriented on the SSPX configs by Tagir-1.
Defenetly needs to be looked over and refinded. But its a start.
@JohnMcLane4
Copy link
Author

see also: KSP-RO/ROKerbalism#112
KSP-RO/RP-1#1612

@JohnMcLane4 JohnMcLane4 changed the title Create RO_MMSEV.cfg MMSEV RO Configs Jan 10, 2022
Comment on lines 62 to 76
@PART[Benjee10_MMSEV_adapter_0-9375]:FOR[RealismOverhaul]:NEEDS[Benjee10_MMSEV]
{
%RSSROConfig = True
%rescaleFactor = 1.8

@title = 2.25m to 1.6875m Adapter PT-091A
@description = This low-profile adapter provides a connection between 2.25m and 1.6875m bulkhead profiles.
@tags = planetside, exploration, technologies, station, radial, attach

@maxTemp = 773.15
@skinMaxTemp = 773.15
@crashTolerance = 10

@mass = 0.5
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would help readability IMO to extract the portions of these patches applied everywhere into a separate patch. Specifically that rescaleFactor, maxTemp, skinMaxTemp, and crashTolerance are the same in every patch. Instead:

@PART[Benjee10_MMSEV*]:FOR[RealismOverhaul]
{
	%rescaleFactor = 1.8
	@maxTemp = 773.15
	@skinMaxTemp = 773.15
	@crashTolerance = 10
}

Place this patch at the top of the file, then anything further in can deviate if needed,


@mass = 0.5
}
@PART[Benjee10_MMSEV_crewTube0]:FOR[RealismOverhaul]:NEEDS[Benjee10_MMSEV,RealFuels]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NEEDS[Benjee10_MMSEV] is unnecessary since the part won't exist without it. Remove throughout.

Either remove the RealFuels NEEDS (because RO deps RealFuels anyway) -or- just move it to the only PartModule that requires it: ModuleFuelTanks

Comment on lines +99 to +108
@MODULE[ModuleFuelTanks]
{

TANK
{
name = ElectricCharge
amount = 5000
maxAmount = 5000
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declaring the module and then immediately patching it is a strange pattern. Put these two together here [and everywhere].

I realize you used the SSPX configs as a template for these things that I'm commenting on. I recently went through and changed these patterns in the SSPX files,


@maxTemp = 773.15
@skinMaxTemp = 773.15
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add newline before EOF

@JohnMcLane4
Copy link
Author

I will take a look in two weeks, away for holidays, so no time to change this.

@NathanKell
Copy link
Member

Might you have a chance to come back to this soon?

@JohnMcLane4
Copy link
Author

Of course I forgot this existed :D
Not much time today. but in the next three days i got a workshop out of the home office. There should be time to make the changes. i try to to it then

@JohnMcLane4
Copy link
Author

Iam sorry guys, right now i cant find the time and motivation to correct and update it. To much on my plate with family stuff.
Do someone want to take this over?

I did at least the fast basic changes. For more i will need to find time...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants