Attempt to fix random reputation loss

[ghost]

I had gotten it to work, tested it for a couple of hours, everything went great, and then I accidentally deleted it... But I rebuilt it (mostly from memory) tested it briefly and it seems to work. The idea behind it is to check if reputation was lost do to vessels being destroyed, and then check if a vessel was really destroyed. If so, check if we were not in control of the vessel, or if no one was in control. If either is true, then undo the reputation loss. Also, I added the "InfraredTelescope" KSP 1.3 missing part.

[Xinayder]

Can you elaborate more on how this new system works?

[ghost]

Sure, here is a breakdown of the separate changes:

I've added 3 new game event hooks, "onCrewKilled", "onVesselTerminated", and "OnReputationChanged" to the ScenarioWorker class.

I've also added "repLoss" bool, "repLossHasReason" bool, "repEventTimer" float, "REP_LOSS_COOLDOWN" const float, "currentRep" float, and "repLost" float.

"repLoss" when true means that the player has lost rep do to "VesselLoss" transactions.

"repLossHasReason" when true means that something happened to warrant the need to undo rep loss. Such as a vessel other then your own crashed. In hindsight "repLossHasReason" should have been named "repLossHasNoReason".

"repEventTimer" is the last time either "repLoss", or "repLossHasReason" was set to true.

"REP_LOSS_COOLDOWN" is the fudge time to make sure that if "repLoss" and "repLossHasReason" are not fired exactly the same time, the system will still undo incorrect rep loss.

"currentRep" is the rep the player had at the last rep change. This is used to find out how much the rep has changed.

"repLost" is the cumulative loss of rep do to "VesselLoss" transactions. This is reset when ever rep loss is undone or allowed.

In the "onCrewKilled" hook I check for a vessel ref, if there is no ref then the "repLossHasReason" is set to true, if not already true, if there is a ref then check if we're not in control or if no one is in control. If so then "repLossHasReason" is set to true, if not already true.

In the "onVesselTerminated" hook I check if there were any Kerbals inside the vessel, and if so check if we are in control of the vessel, or if no one was in control. If so then "repLossHasReason" is set to true, if not already true.

In the "OnReputationChanged" hook I just check if the transaction reason is "VesselLoss". If it is then I set "repLoss" to true, if not already true, and I add the rep change to the "repLost" variable. Also regardless of the transaction reason, update the "currentRep" variable.

And last but not least, in "private void Update()" the system checks if "repLossHasReason" is true, and if so then it checks if "repLoss" is true. If it is then the system undoes the rep loss. If "repLoss" is false then check the "repEventTimer" to see if the "REP_LOSS_COOLDOWN" time has expired, if so then clear "repLost" and set "repLossHasReason" to false. back to the "repLossHasReason" is true check, if it is not then check if "repLoss" is true. If is then check the "repEventTimer" to see if the "REP_LOSS_COOLDOWN" time has expired, if so then clear "repLost" and set "repLoss" to false.

[ghost]

Ok, so I just had a better idea then this patch. I'm gona post the idea here (but not the code, at least not yet) because it's an already open pull request from me, and because this new idea can still use the old idea. Back to the point, since rep loss is caused by players in the future thinking that players with Kerbals in the past have crashed and died. What if we were to remove the Kerbals from the vessel data when sending it to other players. And then when no player is controlling a vessel, send the vessel data with the Kerbals. This would fix the rep loss bug everywhere except when players are flying Kerbals directly. For that we could use the old idea.

[Xinayder]

What about missions and things like it?

[ghost]

DMP would still sync Kerbals, just not when they are a part of a vessel. The reason I suggested this new idea is because while playing KSP with the first idea, I noticed that tourist contracts could not be completed. This was because KSP/DMP still thought that the Kerbals were dying in the future and that caused me to fail the contract. The only way I could think of to fix that problem was to make sure that KSP/DMP would never even know that their was a Kerbal aboard the vessel and thus could not kill them. The new idea might have some drawbacks later down the road, but it should fix the problem at it's root.

[Xinayder]

I disagree that it's a good idea. If we stop syncing kerbals (sending them to the other clients), we are essentially breaking DMP. I think a better idea would be to add vessel/kerbal privacy, and add some sort of confirmation: if a vessel was changed in the future and we're in the past and it explodes/loses some kerbals, we ignore this update.

Sent from my Motorola XT1095 using FastHub

[ghost]

Hmm... OK, to start with, two things. One: I do agree that your idea is better. Not syncing Kerbals when aboard vessel is kinda a crude fix. (which is why I thought it would create problems down the road) Two: I just want to make sure you understand my findings/understanding of the root problem. That is to say, that its not that vessels are changed in the future. Instead its that players in a future subspace see players in past subspaces crashing all the time. Simply because KSP doesn't assume that the players in the past made it down with a safe landing. That said, I'm not sure that your idea would work if my findings are correct. Because its not just one moment that the future player thinks that the vessel controlled in the past is crashing, its every update from DMP that KSP assumes that the vessel crashed. However, we might be able to fix the problem by making some sort of safe zone for vessels in different subspaces. However, there is a downside to that idea, that is that the further in time the future player is, the bigger the safe zone would need to be. The root problem could be fixed by completely overriding KSP's "onCrewKilled", but to the best of my knowledge there is no way to do that.

[Xinayder]

Well, I think the problem is that we have the same kerbals being assigned to two different players. What I also noticed is that sometimes the kerbals in a vessel are replaced by other kerbals.

Privacy would "fix" this problem, but it would still occur when one of your vessels bug out and crash due to DMP freaking out.

[ghost]

Well, I don't know how one would go about building a Kerbal privacy system. I assume that you would need to add a player id tag to Kerbals when sending them to the server. On the other hand, the two commits in this pull request undo random rep loss after it happens. So that should at least help patch the problem for the moment.

[81ninja]

How does this compare to #361? I haven't been able to test that attempt thoroughly, but back then I merged it and haven't noticed any reputation loss ever since.

In a nutshell, that fix just reimburses any reputation loss due to the death of kerbals that aren't being or haven't been controlled by the player, during that session. It's not perfect, but makes the game playable.

[ghost]

I just updated this pull request to the latest commits from godarklight : master. I have been using this pull request when playing KSP, and I have not loss any reputation randomly. 81ninja as for how this compares, it does basically the same thing only it does it when the rep is lost instead of when you recover the craft.