In the editor, querying crew on root part from another Kerbalism cause KSP's UIPartActionController to go crazy

[gotmachine]

This is a strange bug that happen on KSP 1.11.1 when Kerbalism + CrewR&R are installed together.

To reproduce :

• In the editor, have a crewable and crewed root part
• Open the PAW on any part

This cause constant re-instantiation of the PAW by UIPartActionController .Update(), generating huge lag that result in PAWs becoming very unresponsive :

• This doesn't happen if the root part isn't crewable
• This doesn't happen if the root part isn't crewed

This seems triggered by the Kerbalism Habitat PartModule querying the part crew manifest on Update().
Not sure of the exact root cause, but removing the CrewR&R EditorModule.Update() method fixes it.

My guess is that CrewR&R somehow enter in a state where the manifestNeedsCleaning bool gets reset to true every frame, causing this expression to evaluate to true every frame :

CrewRandR/Source/Interface/EditorModule.cs

Line 105 in 3bf6f0a

if (manifestNeedsCleaning)

[linuxgurugamer]

The bool manifestNeedsCleaning is set true on the following conditions:

        GameEvents.onEditorStarted.Add(OnEditorStarted);
        GameEvents.onEditorPodPicked.Add(OnEditorPodPicked);
        GameEvents.onEditorLoad.Add(OnEditorLoad);

So your report tells me that Kerbalism is forcing KSP to trigger one (or more) of these three events.

I've responded in more detail in the thread here: https://forum.kerbalspaceprogram.com/index.php?/topic/159299-19x-110x-111x-crew-rr-crew-rest-rotation/&do=findComment&comment=3941979

[gotmachine]

Kerbalism doesn't trigger or subscribe to any of those events.
As I said, the only thing it does is getting the vessel/part crew manifest through the stock methods and reading how many Kerbals are on the part.
And also, no need to fire bullets at each other, the root issue is likely some weird behavior on the KSP side, thank you (ie, your forum post).

Since it is improbable that manifestNeedsCleaning is stuck to true or that the SceneModule.CleanManifest() is called repeateadly (but some logging in those methods would have been useful to confirm it), I suspect something getting in a wrong or undefined state following the initial crew re-assigement by CrewR&R, causing the stock KSP.UI.BaseCrewAssignmentDialog.GetManifest() method to trigger the issue :

// KSP.UI.BaseCrewAssignmentDialog
public VesselCrewManifest GetManifest(bool createClone = true)
{
    if (defaultManifest == null)
    {
        return null;
    }
    if (!listIsValid)
    {
        return listManifest;
    }
    VesselCrewManifest vesselCrewManifest = ((!createClone) ? listManifest : VesselCrewManifest.CloneOf(listManifest, blank: true));
    int num = 0;
    int i = 0;
    for (int count = scrollListCrew.Count; i < count; i++)
    {
        CrewListItem component = scrollListCrew.GetUilistItemAt(i).GetComponent<CrewListItem>();
        if (component == null)
        {
            num = 0;
            continue;
        }
        if (component != null && component.GetCrewRef() != null)
        {
            vesselCrewManifest.GetPartCrewManifest(component.pUid).AddCrewToSeat(component.GetCrewRef(), num);
        }
        num++;
    }
    return vesselCrewManifest;
}

As you can see, that method does reassign Kerbals to seats, and I suspect that due to Kerbals and their inventories being visible in the PAW since 1.11, that could be the link between the stuck PAW rebuild loop and what we are doing.

I fact, I'm able to reproduce the issue without Kerbalism and only CrewR&R installed, adding only a simple plugin that call that stock method :
TestCrewModule.zip

public class TestCrewModule : PartModule
{
    void Update()
    {
        if (HighLogic.LoadedSceneIsEditor)
        {
            var manifest = KSP.UI.CrewAssignmentDialog.Instance.GetManifest();
        }
    }
}

With that TestCrewModule added through the following MM Patch :

@PART[*]:HAS[#CrewCapacity[>0]]:FINAL
{
	MODULE
	{
		name = TestCrewModule
	}
}

[linuxgurugamer]

I'm not firing bullets, sorry if it seemed that way.
That test module will be very helpful, I'll see if I can fix it

Thanks

[linuxgurugamer]

I would offhand say that maybe the GetManifest shouldn't be called on every Update???? I don't have time to get into this now, but that seems to be an awful wait of cpu, how much can it change in 1/100th of a second, especially in the editor?

[gotmachine]

I would offhand say that maybe the GetManifest shouldn't be called on every Update ?

It hasn't been an issue so far (and I'm pretty careful about performance profiling), and that's the only way to get vessel-equivalent crew information in the editor. And crew modification handling through gamevents is a big mess, so I prefer the polling approach. This being said, if the issue persist and you can't find what the cause is (editor crew handling is a big spaghetti plate), I could look into implementing an alternative (maybe just by bypassing that stock method and getting the VesselCrewManifest object directly)

[linuxgurugamer]

Been looking into this. It's really strange, it has nothing to do with manifestNeedsCleaning being set true at all. the Update() isn't doing anything

[linuxgurugamer]

Not sure what's going on, but I've been able to isolate it to this line:

            CrewAssignmentDialog.Instance.RefreshCrewLists(originalVesselManifest, true, true);

It only gets called once, but that seems to trigger it. When I comment it out, it doesn't happen.

[linuxgurugamer]

It's not CrewR&R, It's KSP. See this code which creates the issue:
`using KSP.UI;

public class TestCrewModule : PartModule
{
bool called = false;
void Update()
{
if (HighLogic.LoadedSceneIsEditor)
{
if (!called)
{
VesselCrewManifest originalVesselManifest = CrewAssignmentDialog.Instance.GetManifest();
CrewAssignmentDialog.Instance.RefreshCrewLists(originalVesselManifest, true, true);
called = true;
}
var manifest = KSP.UI.CrewAssignmentDialog.Instance.GetManifest();
}
}
}`

I'll file a bug report

[linuxgurugamer]

https://bugs.kerbalspaceprogram.com/issues/27432

[gotmachine]

In all 3 places where RefreshCrewLists() is called in your code, if you change the third arg (bool updateUI) to false :

CrewAssignmentDialog.Instance.RefreshCrewLists(originalVesselManifest, true, false);

the issue disappear.

This is how this method is always called in the KSP source when the fourth arg (Func<PartCrewManifest, bool> displayFilter) is null, and I don't notice any issue with the UI not being updated.

For example, see the following EditorLogic method that seem to do something similar as what CrewR&R is doing :

// EditorLogic
using KSP.UI;

internal void ResetCrewAssignment(ConfigNode craftNode, bool allowAutoHire)
{
    if (!(CrewAssignmentDialog.Instance == null))
    {
        ShipConstruction.ShipManifest = HighLogic.CurrentGame.CrewRoster.DefaultCrewForVessel(craftNode, null, allowAutoHire);
        CrewAssignmentDialog.Instance.RefreshCrewLists(ShipConstruction.ShipManifest, setAsDefault: true, updateUI: false);
        GameEvents.onEditorShipCrewModified.Fire(ShipConstruction.ShipManifest);
    }
}

[linuxgurugamer]

Interesting. I'll look, but it's still a bug in KSP. I'll test this, and if it doesn't have any other issues, I'll release it as an update
Thanks

Edit: One thing, the CrewR&R code actually has it in four places.