Skip to content

Tooling viewer: bucket categorization + Refit button#2840

Open
chambm wants to merge 4 commits into
KSP-RO:masterfrom
chambm:feat/tooling-refit
Open

Tooling viewer: bucket categorization + Refit button#2840
chambm wants to merge 4 commits into
KSP-RO:masterfrom
chambm:feat/tooling-refit

Conversation

@chambm

@chambm chambm commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Two-commit change to the Toolings tab in the editor:

  1. Group tank tooling types into category buttons keyed by construction (Conventional / Isogrid / Balloon / Fuselage / Service Module / Shielded / Cryogenic / Other) and pressure class (HP variants get their own button where they exist). Buttons sorted alphabetically by title. Each leaf row tags the underlying material(s) supplied it. Lets a player search for an existing tooled diameter for a tank type without picking material first. ProceduralAvionicsWindow now also shows the avionics tooling abbreviation ("N3") next to the config name so it cross-references the grouped Avionics-N entries.

Before:
Screenshot 2026-06-03 170800

After:
Screenshot 2026-06-05 161536

Screenshot 2026-06-05 162333
  1. Refit button on each tooled (d, L) row that writes the dims to whichever part has its PAW open in the editor, switching the part's RF tank type when the row only supplies materials the part isn't currently on. Disabled when the PAW target's tank type belongs to a different bucket than the one on screen -- refit only crosses materials within the same construction family. When the user opens a PAW on a tank, the viewer auto-switches to that tank's bucket (from the main bucket list or from inside another bucket page). Manual bucket picks suppress the auto-switch until the PAW target changes.
Screenshot 2026-06-07 222638

Test plan

  • Tooling tab in editor -- categories render in alphabetical order, HP variants appear as separate buttons where data exists.
  • Open a PAW on each tank class (Conventional/Isogrid/Balloon/Fuselage/ServiceModule/Cryogenic) -- viewer jumps to the matching bucket from both the main list and from inside another bucket.
  • In a bucket, click Refit on a row -- part resizes to (d, L), and RF tank type switches if the row's materials don't include the current type.
  • PAW open on a tank in a non-matching bucket -- Refit greys out with the "switch to its bucket" tooltip.
  • No PAW open -- Refit greys out with "Open a part's PAW" tooltip.
  • Avionics-N bucket -- ProceduralAvionicsWindow shows the N-level abbreviation next to the config name.

chambm added 2 commits June 7, 2026 22:46
The Toolings tab now collapses tank tooling types into category buttons
keyed by construction (Conventional / Isogrid / Balloon / Fuselage /
Service Module / Shielded / Cryogenic / Other) and pressure class (HP
variants get their own button where they exist). Buttons are sorted
alphabetically by title. Each leaf row tags which underlying material(s)
supplied it -- Tank-* keys get the construction prefix trimmed (and the
-HP suffix dropped since the bucket title already says it), SM-* keys
pass through unchanged, and other dashed keys (Avionics-N3) drop the
prefix to just the suffix. Type-tab scroll widened to fit the longer
badges. Lets the player search for an existing tooled diameter for a
tank type without picking material first.

ProceduralAvionicsWindow shows the avionics tooling abbreviation
(e.g. "N3") next to the config name so it cross-references the
grouped Avionics-N entries in the tooling viewer.
Each tooled (diameter, length) row gets a Refit button that writes the
dims to whatever part has its PAW open, and switches the part's RF tank
type if the row's bucket only carries materials the part isn't on.
Disabled when the PAW target's tank type belongs to a different bucket
than the one currently shown.

When the player opens a PAW on a tank, the viewer jumps into that tank's
bucket -- including from inside another bucket page, not just from the
main bucket list. A manual bucket pick suppresses the auto-switch until
the PAW target changes.
@chambm chambm force-pushed the feat/tooling-refit branch from 65da377 to 9d6cce6 Compare June 8, 2026 02:47
@siimav

siimav commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

What is that mysterious SteelAl in your images?

@siimav siimav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

General notes:

  • Use proper /// method comments
  • Do not use var unless the type is apparent from the right side of the assignment.
  • Do not mix fields between methods

IDK what to think of the "Refit" feature. Sounds like there's so many corner cases that can force the part into an invalid state. For instance it doesn't appear to handle avionics parameters at all so the outcome will be a weird mix. Same for PP shapes. PickRfType will silently swallow a correct type not being found and will happily use the first one available.

Comment on lines +303 to +309
bool hp = toolingType.EndsWith("-HP");
if (toolingType.StartsWith("Tank-Iso-")) return hp ? TankIsogridHP : TankIsogrid;
if (toolingType.StartsWith("Tank-Balloon-") || toolingType == "Balloon")
return hp ? TankBalloonHP : TankBalloon;
if (toolingType == "Fuselage") return hp ? TankFuselageHP : TankFuselage;
if (toolingType.StartsWith("Tank-Sep-")) return hp ? TankConventionalHP : TankConventional;
return TankOther; // legacy RF types we don't know how to place: Default, ElectricPropulsion, etc.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IDK why your clanker thought this is good formatting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree it could be improved, but I'm not sure which part you're objecting to. That (only) half of the return statements are manually indented? That all but one are single line ifs? Looking at it again, I will make predicates for each tank type (IsBalloonTank, IsShieldedTank, IsCryoTank) so each line can be a single line. At that point it could change to a switch statement or expression.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not clue what it's even aligned on. Just half of the code dangling pretty much nowhere.

Comment thread Source/RP0/UI/ToolingGUI.cs Outdated
Comment on lines +273 to +284
{ TankConventional, "Conventional Tanks" },
{ TankConventionalHP, "HP Conventional Tanks" },
{ TankIsogrid, "Isogrid Tanks" },
{ TankIsogridHP, "HP Isogrid Tanks" },
{ TankBalloon, "Balloon Tanks" },
{ TankBalloonHP, "HP Balloon Tanks" },
{ TankFuselage, "Fuselage" },
{ TankFuselageHP, "HP Fuselage" },
{ TankServiceModule, "Service Modules" },
{ TankShieldedKey, "Shielded Tanks" },
{ TankCryogenic, "Cryogenic Tanks" },
{ TankOther, "Other Tanks" },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

HP and non-HP bucket pairs sort non-adjacently GetGroupedToolingTypes() sorts by GetGroupTitle(). "Conventional Tanks" and "HP Conventional Tanks" sort with several other buckets between them (alphabetically H follows C, I, etc.). Users looking for HP variants would expect them next to their base family. Consider naming them "Conventional Tanks (HP)" so they sort adjacent to the base.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's intentional. I think users first decide "Do I need HP or non-HP?" and then they want to see if they have any tanks already tooled for the dimensions they need. They're likely to be flexible about whether it's balloon/conventional/isogrid and even more flexible about the material of the tank.

If tank type (balloon/conventional/isogrid) was a slider instead of part of the type, I'd probably just have two buckets: HP and non-HP (perhaps as a user option for users who aren't so flexible about the tank type, e.g. to keep that isogrid integration bonus). The Refit button would then change tank type as well as material. That would be a considerably more convenient than the current situation where you have to take the rocket apart and put it back together to change the tank type. Outside this PR's scope, of course.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, but it is weird that the HP types sort together but the non-HP types do not. Add explicit "Non-HP" prefix so they sort together? That would look clunky but it would maintain my intention and alphabetical listing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How about Tank (HP Conventional) Tank (HP Fuselage) Tank (HP Isogrid) Tank (HP Shielded) Tank (Non-HP Balloon) Tank (Non-HP Conventional) Tank (Non-HP Fuselage) Tank (Non-HP Isogrid) Tank (Non-HP Shielded)

Better than a clunky Non-HP prefix at the beginning, with the added advantage that all the Tanks sort together.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image

Comment thread Source/RP0/UI/ToolingGUI.cs Outdated
Comment thread Source/RP0/UI/ToolingGUI.cs Outdated
Comment thread Source/RP0/UI/ToolingGUI.cs Outdated
Comment thread Source/RP0/UI/ToolingPartResizer.cs Outdated
Comment thread Source/RP0/UI/ToolingPartResizer.cs Outdated
Comment thread Source/RP0/UI/ToolingPartResizer.cs Outdated
@chambm

chambm commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

IDK what to think of the "Refit" feature. Sounds like there's so many corner cases that can force the part into an invalid state. For instance it doesn't appear to handle avionics parameters at all so the outcome will be a weird mix. Same for PP shapes. PickRfType will silently swallow a correct type not being found and will happily use the first one available.

I'm not sure what you mean about avionics parameters. The Refit button is only for fuel tanks right now. It would be nice to apply it to avionics parts too but like you say, the controllable mass and EC parameters would have to be handled additionally. Those don't apply for fuel tanks though, right?

Different PP shapes do present a bit of an issue when they have more than the basic "diameter/length" or "diameter/vscale" sliders. But since RP-1 tooling doesn't distinguish between them (AFAIK?), it seems reasonable to just have Refit handle those diameter and length parameters and leave the other parameters alone. Certainly this handles the bulk of cases with cylindrical/cone/cyl-deriv/cone-deriv. I do need to test that it's working as I expect with other shapes though, I think I only tested switching between cylinders of each tank type and material (tank types being different parts, as mentioned above).

@chambm

chambm commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

What is that mysterious SteelAl in your images?

  • Tank-Balloon-SteelAl → title "Balloon Tank"
  • Tank-Balloon-SteelAlCu → "Refined Balloon Tank"
  • Tank-Balloon-SteelAlLi → "Modern Balloon Tank"

Shall I change it to "Balloon", "Refined", and "Modern"? Or "Al", "AlCu", "AlLi"?

- Tank bucket titles -> Tank (HP/Non-HP X) so families sort adjacently
- Cache grouped tooling list by ToolingDatabase.Generation
- GetGroupedToolingTypes returns List; camelCase locals; predicate helpers
- Move grouping consts out from between methods; /// comments; drop var
- ToolingPartResizer: explicit private modifiers; log swallowed exceptions
- PickRfType refuses when no available material matches instead of forcing
  first source; Refit button disables that row with an explanatory tooltip
@chambm

chambm commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

IDK what to think of the "Refit" feature. Sounds like there's so many corner cases that can force the part into an invalid state. For instance it doesn't appear to handle avionics parameters at all so the outcome will be a weird mix. Same for PP shapes. PickRfType will silently swallow a correct type not being found and will happily use the first one available.

I'm not sure what you mean about avionics parameters. The Refit button is only for fuel tanks right now. It would be nice to apply it to avionics parts too but like you say, the controllable mass and EC parameters would have to be handled additionally. Those don't apply for fuel tanks though, right?

Different PP shapes do present a bit of an issue when they have more than the basic "diameter/length" or "diameter/vscale" sliders. But since RP-1 tooling doesn't distinguish between them (AFAIK?), it seems reasonable to just have Refit handle those diameter and length parameters and leave the other parameters alone. Certainly this handles the bulk of cases with cylindrical/cone/cyl-deriv/cone-deriv. I do need to test that it's working as I expect with other shapes though, I think I only tested switching between cylinders of each tank type and material (tank types being different parts, as mentioned above).

Definitely found some issues here with different shapes and symmetry, will address them soon. Thanks for the review.

- PickRfType: read typesAvailable as List<TankDefinition> and gate on the
  type's upgrade being unlocked; the old string cast was always null, so every
  cross-material refit was disabled.
- ApplyProcShape: write the diameter field the active shape actually uses
  (outerDiameter for hollow, top/bottom for cone) and skip trusses, so hollow
  tanks land tooled instead of silently missing.
- Drive dimension changes through the procedural module's own field-changed
  handler (the PAW slider path) so attach nodes and attached parts reposition,
  and let it walk the symmetry group itself instead of re-applying per part.
- Apply the type switch across symmetry counterparts; post one message; show
  no success message when the shape can't be refit.
- ROTank: in lengthWidth mode drive currentLength (currentVScale is inert).
- Refresh the part's PAW after refit so the slider display matches.
@chambm

chambm commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

OK I've tested pretty thoroughly with the main tank types (modular, another modular, procedural), and with changing shapes with procedural tanks. Refit works well with all of them. It's easier to test with changing shapes with the PR I just put in to preserve diameter/length when changing procedural shape. That way you can see that Refit even applies to cones (treated as cylinders), hollow/filleted variants, and polygon, Refit on truss is currently disabled although I could revisit that and just have it handle top/bottom diameter and length the same way it does for other shapes with both top and bottom diameter.

@siimav

siimav commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Shall I change it to "Balloon", "Refined", and "Modern"? Or "Al", "AlCu", "AlLi"?

Well we have to follow what is shown on the tank part itself or it's gonna get hella confusing.

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.

2 participants