Skip to content

Rework unlockable PartUpgrade tips#2759

Merged
siimav merged 6 commits into
KSP-RO:masterfrom
periodically-makes-puns:more-tips
May 19, 2026
Merged

Rework unlockable PartUpgrade tips#2759
siimav merged 6 commits into
KSP-RO:masterfrom
periodically-makes-puns:more-tips

Conversation

@periodically-makes-puns

@periodically-makes-puns periodically-makes-puns commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Currently there are 6 PartUpgrade lines that can only be purchased in R&D, and are therefore frequently missed by new players. Even experienced players can forget to purchase a corresponding upgrade when it is unlocked.
These are:

  • Communications tech
  • Solar panel tech
  • Minimum structure density upgrades (referred to as Fairing upgrades)
  • Data storage
  • MLI layers
  • The X-2 cockpit upgrade

We currently only warn about communications tech, and in order to purchase it players must leave the VAB and enter R&D.

For antennas and solar panels, the following behaviour now applies:

  • We do not warn about unpurchased tech levels.
  • When a part is placed, the new default TL is now the best researched TL (that is not necessarily purchased).
  • Unpurchased TL issues can be fixed during vessel integration.

For the other 4 cases, the following behaviour now applies:

  • When a PAW is opened containing the relevant module AND there is an upgrade in the order that has not been purchased AND the user has not chosen to fully ignore the alert for this upgrade:
    • A popup will appear, notifying the user that a new upgrade is available. They will be prompted to either purchase it immediately, acknowledge the notification (which can show again), or disable the notification (which will disable it forever for that upgrade and all before it, but not any future ones).

The "relevant modules" for the 4 tech lines described above are as follows:

  • Fairing upgrades: ProceduralFairingSide
  • Data storage: ModuleProceduralAvionics
  • MLI layers: ModuleFuelTanks
  • X-2: ModuleUnpressurizedCockpit

Implementation notes:

  • The Balloon tank MLI upgrade is not in the MLI tech order. I do not know of a clean way to distinguish Balloon tanks from non-Balloon tanks. [Edit: The Balloon tank MLI upgrade is now included.]
  • It may still be worthwhile to notify the user when a new comms tech is available, for the sole purpose of reminding the user that the Tracking Station needs to be upgraded to support it.
  • If a part would cause multiple alerts to be set off at the same time, only one shows. As far as I can tell this occurs in one of two situations: [Edit: There should no longer be any simultaneous alerts.]
    • A procedural avionics core is opened when there is an MLI upgrade and data storage upgrade available. This could probably be fixed by the same fix that distinguishes Balloon tanks.
    • A X-1 cockpit is opened when there is an MLI upgrade and X-2 cockpit upgrade available.
  • The solar panel PartUpgrade names are currently hardcoded in a bunch of places. I don't like this.

This pull request requires KSP-RO/ROLibrary#30 and KSP-RO/RealAntennas#74 to handle the comms and solar panel cases, but theoretically works standalone without them.

@njits23

njits23 commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

A procedural avionics core is opened when there is an MLI upgrade and data storage upgrade available. This could probably be fixed by the same fix that distinguishes Balloon tanks.

As you cannot put anything other than batteries inside a proc avionics core, why not just disable the MLI upgrade alert on them outright?

@periodically-makes-puns

Copy link
Copy Markdown
Contributor Author

... Oh I just noticed a major error with persistence. Converting to draft while I work this out.

Also I think with how the callbacks are ordered, the Data Storage alert will always fire first? (My implementation right now isn't smart enough to handle a separate smarter check for a specific upgrade type, it's intentionally generic.)

@periodically-makes-puns periodically-makes-puns marked this pull request as draft April 26, 2026 16:25
@periodically-makes-puns

Copy link
Copy Markdown
Contributor Author

Okay cool, fixed it. Ignored upgrades now persist per-savegame instead of per-save, which is better since players may reload prior to when they purchased/ignored an upgrade.

Also, I added a filter delegate to the constructor for UpgradeLine, so I can add more logic for the MLI check. The MLI warning now only fires for Conventional and Isogrid tanks, and there is now a Balloon tank MLI warning for Balloon tanks (if that upgrade is unlocked).

@periodically-makes-puns periodically-makes-puns marked this pull request as ready for review April 26, 2026 20:03
@siimav

siimav commented May 1, 2026

Copy link
Copy Markdown
Contributor
  1. Dialog can be spammed
    The old code did _hasNewPurchasableRATL = false immediately before showing the dialog, so it could only fire once per
    session. The new OnPartActionUIShown has no equivalent guard.
    PopupDialog.SpawnPopupDialog is non-blocking. Every time the player opens a relevant part's PAW, a new popup is
    spawned on top of any existing one.

  2. Upgrade-after-purchase not re-evaluated
    After the player clicks "Unlock", bestUpgrade is updated but UpdateUpgradeInfo() is never called. If the purchase
    causes the next tier to become immediately available, that tip won't fire until the next scene load (because the PAW
    subscription remains but bestUnpurchasedUpgrade is now == bestUpgrade and never refreshed). This is a minor edge case but inconsistent with the intent.

  3. Unused variable invertCMQOp (also flagged by SonarCloud)

  4. using System.Linq instead of using UniLinq
    The rest of the codebase (e.g., GameplayTips.cs) uses UniLinq. RnDUpgradeTips.cs imports System.Linq. This should be UniLinq.

  5. ECMHelper additions are currently dead code
    GetEcmNameFromModuleRealAntenna() and GetEcmNameFromModuleROSolar() are added to ECMHelper.cs but never called
    anywhere in this PR. If they're stubs for future RA/solar upgrade lines, they should either be left out of this PR or
    referenced by TODO comments. As-is they'll generate "unused method" warnings.

  6. Namespace mismatch
    RnDUpgradeTips.cs lives in Source/RP0/ScenarioModules/ but declares namespace RP0.Addons. Other files in that folder
    use namespace RP0. Worth aligning unless there's intentional separation.

  7. Some images would be nice to illustrate the new UX. I'm too lazy to go scouring discord.

@periodically-makes-puns

Copy link
Copy Markdown
Contributor Author

ECMHelper additions are used inside the function that gets ECM names for the unlock cost calculation, so unpurchased comms/solar TLs can be fixed during validation.

Everything else should be fixed:

  • _firedAlready now forbids the pop-up from showing again for the duration of the scene. Okay should stop it from firing for the duration of the scene, Never remind me again stops it from firing for that upgrade forever.
  • The best unpurchased tier should not change for the duration of the scene, unless a tech is somehow researched in the VAB/SPH scene. Regardless, we now call UpdateUpgradeInfo (which should properly write the new upgrade level to the persistent Dictionary).

https://github.com/user-attachments/assets/1d4cd731-e047-4379-b078-d52259ed8e58
[setup ends around 1:15. note that unlock cost doesn't actually change when I'm sliding the comms TL slider around because I forgot about the antenna in the X-1 cockpit]

image image image image image image

@pap1723

pap1723 commented May 1, 2026

Copy link
Copy Markdown
Contributor
image

This is a REALLY nice feature!

@Clayell

Clayell commented May 2, 2026

Copy link
Copy Markdown
Contributor

Would be best to scrub the "This is not a part you can use..." text for the VAB.

@periodically-makes-puns

Copy link
Copy Markdown
Contributor Author

Would it be better to programmatically remove that text in the popup, or should I remove it entirely from the description in the tech tree?

@Clayell

Clayell commented May 2, 2026

Copy link
Copy Markdown
Contributor

The former. Although, if you could find a way to automatically add the text when viewing the part upgrade in the tech tree, that would be even better.

@periodically-makes-puns

Copy link
Copy Markdown
Contributor Author
image image

@siimav siimav merged commit 4a4010c into KSP-RO:master May 19, 2026
4 checks passed
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.

5 participants