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

Fix contract confidence display with leader reputation affects #2241

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

Blothorn
Copy link
Contributor

Noticed when taking Glushko as admin--standard-difficulty SR at 100% rewards showed as 48 confidence in admin, but provided the correct 50 when actually accepted and completed. It seems that the display was already using base rep. This was tested using Glushko--altitude SR gave increased reputation (11 after rounding) but 50 confidence in the new total, total confidence earned summary, MC contract summary, contract completion message, and career log.

Noticed when taking Glushko as admin--standard-difficulty SR at 100% rewards showed as 48 confidence in admin, but provided the correct 50 when actually accepted and completed. It seems that the display was already using base rep. This was tested using Glushko--altitude SR gave increased reputation (11 after rounding) but 50 confidence in the new total, total confidence earned summary, MC contract summary, contract completion message,  and career log.
@NathanKell
Copy link
Member

As you say it's correct on contract summary (which I assume you mean the completed contract tab?) so changing this back will then break completed contracts, no? The issue is that we don't actually record confidence gained from a contract (because we can't add data to KSP, just change code, with Harmony) so it's a bit of a pickle.

@NathanKell
Copy link
Member

lolnope I'm wrong, KSP is super dumb about this and doesn't store the actual awards granted on contract success, it recalculates based on current CMQs.

@Blothorn
Copy link
Contributor Author

I meant that I've tested that with the change confidence is displayed consistently and correctly (i.e. 50 despite the reputation bonus from Glushko) in all those places--I haven't checked whether the problem affects the completed contract list, but they are displaying correctly with it.

Worth note is that if I understand how the code works correctly, the change to use base reputation didn't actually change the confidence added (https://github.com/KSP-RO/RP-1/blob/master/Source/Programs/ProgramHandler.cs#L346), just various places it was displayed--I think the contract's ReputationCompletion has been base reputation all along.

@NathanKell NathanKell merged commit 4dce3dc into KSP-RO:master Sep 20, 2023
3 checks passed
@NathanKell
Copy link
Member

Yup you're 100% correct, per above KSP also Does The Dumb and just recalculates the base rep based on the current CMQ regardless of what the actual advances/awards were when the contract was accepted. So we'll just keep following in those footsteps.

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.

None yet

2 participants